From 16cae2169c8b36f6edbc5828d0b84d94481bdbad Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Tue, 26 Jul 2022 07:59:17 +0000 Subject: [PATCH 1/4] Set mtu for MACsec Signed-off-by: Ze Gan --- orchagent/macsecorch.cpp | 19 ++++++++++++++ orchagent/macsecorch.h | 1 + orchagent/portsorch.cpp | 54 ++++++++++++++++++++++++++++++++++++++++ orchagent/portsorch.h | 9 +++++-- 4 files changed, 81 insertions(+), 2 deletions(-) diff --git a/orchagent/macsecorch.cpp b/orchagent/macsecorch.cpp index 67ba904043..0572d0965f 100644 --- a/orchagent/macsecorch.cpp +++ b/orchagent/macsecorch.cpp @@ -1275,6 +1275,18 @@ bool MACsecOrch::createMACsecPort( phy); }); + if (!m_port_orch->getPortMtu(port.m_port_id, macsec_port.m_original_mtu)) + { + SWSS_LOG_WARN("Cannot get Port MTU at the port %s", port_name.c_str()); + return false; + } + m_port_orch->setMACsecEnabledState(port.m_port_id, true); + if (!m_port_orch->setPortMtu(port.m_port_id, macsec_port.m_original_mtu)) + { + SWSS_LOG_WARN("Cannot set MTU to %u at the MACsec enabled port %s", macsec_port.m_original_mtu, port_name.c_str()); + return false; + } + if (phy) { if (!setPFCForward(port_id, true)) @@ -1542,6 +1554,13 @@ bool MACsecOrch::deleteMACsecPort( result &= false; } + m_port_orch->setMACsecEnabledState(port.m_port_id, false); + if (!m_port_orch->setPortMtu(port.m_port_id, macsec_port.m_original_mtu)) + { + SWSS_LOG_WARN("Cannot set MTU to %u at the port %s", macsec_port.m_original_mtu, port_name.c_str()); + return false; + } + if (phy) { if (!setPFCForward(port_id, false)) diff --git a/orchagent/macsecorch.h b/orchagent/macsecorch.h index 9c6e2be636..d8628a838f 100644 --- a/orchagent/macsecorch.h +++ b/orchagent/macsecorch.h @@ -109,6 +109,7 @@ class MACsecOrch : public Orch bool m_sci_in_sectag; bool m_enable; uint32_t m_original_ipg; + uint32_t m_original_mtu; }; struct MACsecObject { diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index e9a3afdc1e..943894cc98 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1135,6 +1135,34 @@ bool PortsOrch::getPortAdminStatus(sai_object_id_t id, bool &up) return true; } +bool PortsOrch::getPortMtu(sai_object_id_t id, sai_uint32_t &mtu) +{ + SWSS_LOG_ENTER(); + + sai_attribute_t attr; + attr.id = SAI_PORT_ATTR_MTU; + + sai_status_t status = sai_port_api->get_port_attribute(id, 1, &attr); + + if (status != SAI_STATUS_SUCCESS) + { + task_process_status handle_status = handleSaiGetStatus(SAI_API_PORT, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + + mtu = attr.value.u32 - (uint32_t)(sizeof(struct ether_header) + FCS_LEN + VLAN_TAG_LEN); + + if (isMACsecPort(id)) + { + mtu -= MAX_MACSEC_SECTAG_SIZE; + } + + return true; +} + bool PortsOrch::setPortMtu(sai_object_id_t id, sai_uint32_t mtu) { SWSS_LOG_ENTER(); @@ -1144,6 +1172,11 @@ bool PortsOrch::setPortMtu(sai_object_id_t id, sai_uint32_t mtu) /* mtu + 14 + 4 + 4 = 22 bytes */ attr.value.u32 = (uint32_t)(mtu + sizeof(struct ether_header) + FCS_LEN + VLAN_TAG_LEN); + if (isMACsecPort(id)) + { + attr.value.u32 += MAX_MACSEC_SECTAG_SIZE; + } + sai_status_t status = sai_port_api->set_port_attribute(id, &attr); if (status != SAI_STATUS_SUCCESS) { @@ -7154,3 +7187,24 @@ bool PortsOrch::decrFdbCount(const std::string& alias, int count) } return true; } + +void PortsOrch::setMACsecEnabledState(sai_object_id_t port_id, bool enabled) +{ + SWSS_LOG_ENTER(); + + if (enabled) + { + m_macsecEnabledPorts.insert(port_id); + } + else + { + m_macsecEnabledPorts.erase(port_id); + } +} + +bool PortsOrch::isMACsecPort(sai_object_id_t port_id) const +{ + SWSS_LOG_ENTER(); + + return m_macsecEnabledPorts.find(port_id) != m_macsecEnabledPorts.end(); +} diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 0fd3552e19..44ca2ead24 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -18,6 +18,7 @@ #define FCS_LEN 4 #define VLAN_TAG_LEN 4 +#define MAX_MACSEC_SECTAG_SIZE (32) #define PORT_STAT_COUNTER_FLEX_COUNTER_GROUP "PORT_STAT_COUNTER" #define PORT_RATE_COUNTER_FLEX_COUNTER_GROUP "PORT_RATE_COUNTER" #define PORT_BUFFER_DROP_STAT_FLEX_COUNTER_GROUP "PORT_BUFFER_DROP_STAT" @@ -174,6 +175,11 @@ class PortsOrch : public Orch, public Subject bool decrFdbCount(const string& alias, int count); + void setMACsecEnabledState(sai_object_id_t port_id, bool enabled); + bool isMACsecPort(sai_object_id_t port_id) const; + bool getPortMtu(sai_object_id_t id, sai_uint32_t &mtu); + bool setPortMtu(sai_object_id_t id, sai_uint32_t mtu); + private: unique_ptr m_counterTable; unique_ptr
m_counterLagTable; @@ -301,7 +307,6 @@ class PortsOrch : public Orch, public Subject bool setPortAdminStatus(Port &port, bool up); bool getPortAdminStatus(sai_object_id_t id, bool& up); - bool setPortMtu(sai_object_id_t id, sai_uint32_t mtu); bool setPortTpid(sai_object_id_t id, sai_uint16_t tpid); bool setPortPvid (Port &port, sai_uint32_t pvid); bool getPortPvid(Port &port, sai_uint32_t &pvid); @@ -374,8 +379,8 @@ class PortsOrch : public Orch, public Subject void voqSyncAddLagMember(Port &lag, Port &port); void voqSyncDelLagMember(Port &lag, Port &port); unique_ptr m_lagIdAllocator; + set m_macsecEnabledPorts; std::unordered_set generateCounterStats(const string& type, bool gearbox = false); - }; #endif /* SWSS_PORTSORCH_H */ From b8f618c83fc3dfdd2806f428fe1a7adc263c4851 Mon Sep 17 00:00:00 2001 From: Junhua Zhai Date: Wed, 27 Jul 2022 09:15:06 +0000 Subject: [PATCH 2/4] Support port mtu update w/o gearbox --- orchagent/macsecorch.cpp | 19 ++--------- orchagent/macsecorch.h | 1 - orchagent/portsorch.cpp | 69 +++++++++++++++++++--------------------- orchagent/portsorch.h | 9 +++--- 4 files changed, 38 insertions(+), 60 deletions(-) diff --git a/orchagent/macsecorch.cpp b/orchagent/macsecorch.cpp index 0572d0965f..9a5e48f883 100644 --- a/orchagent/macsecorch.cpp +++ b/orchagent/macsecorch.cpp @@ -1275,17 +1275,7 @@ bool MACsecOrch::createMACsecPort( phy); }); - if (!m_port_orch->getPortMtu(port.m_port_id, macsec_port.m_original_mtu)) - { - SWSS_LOG_WARN("Cannot get Port MTU at the port %s", port_name.c_str()); - return false; - } - m_port_orch->setMACsecEnabledState(port.m_port_id, true); - if (!m_port_orch->setPortMtu(port.m_port_id, macsec_port.m_original_mtu)) - { - SWSS_LOG_WARN("Cannot set MTU to %u at the MACsec enabled port %s", macsec_port.m_original_mtu, port_name.c_str()); - return false; - } + m_port_orch->setMACsecEnabledState(port_id, true); if (phy) { @@ -1554,12 +1544,7 @@ bool MACsecOrch::deleteMACsecPort( result &= false; } - m_port_orch->setMACsecEnabledState(port.m_port_id, false); - if (!m_port_orch->setPortMtu(port.m_port_id, macsec_port.m_original_mtu)) - { - SWSS_LOG_WARN("Cannot set MTU to %u at the port %s", macsec_port.m_original_mtu, port_name.c_str()); - return false; - } + m_port_orch->setMACsecEnabledState(port_id, false); if (phy) { diff --git a/orchagent/macsecorch.h b/orchagent/macsecorch.h index d8628a838f..9c6e2be636 100644 --- a/orchagent/macsecorch.h +++ b/orchagent/macsecorch.h @@ -109,7 +109,6 @@ class MACsecOrch : public Orch bool m_sci_in_sectag; bool m_enable; uint32_t m_original_ipg; - uint32_t m_original_mtu; }; struct MACsecObject { diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 943894cc98..7444decc67 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1135,60 +1135,35 @@ bool PortsOrch::getPortAdminStatus(sai_object_id_t id, bool &up) return true; } -bool PortsOrch::getPortMtu(sai_object_id_t id, sai_uint32_t &mtu) -{ - SWSS_LOG_ENTER(); - - sai_attribute_t attr; - attr.id = SAI_PORT_ATTR_MTU; - - sai_status_t status = sai_port_api->get_port_attribute(id, 1, &attr); - - if (status != SAI_STATUS_SUCCESS) - { - task_process_status handle_status = handleSaiGetStatus(SAI_API_PORT, status); - if (handle_status != task_success) - { - return parseHandleSaiStatusFailure(handle_status); - } - } - - mtu = attr.value.u32 - (uint32_t)(sizeof(struct ether_header) + FCS_LEN + VLAN_TAG_LEN); - - if (isMACsecPort(id)) - { - mtu -= MAX_MACSEC_SECTAG_SIZE; - } - - return true; -} - -bool PortsOrch::setPortMtu(sai_object_id_t id, sai_uint32_t mtu) +bool PortsOrch::setPortMtu(const Port& port, sai_uint32_t mtu) { SWSS_LOG_ENTER(); sai_attribute_t attr; attr.id = SAI_PORT_ATTR_MTU; /* mtu + 14 + 4 + 4 = 22 bytes */ - attr.value.u32 = (uint32_t)(mtu + sizeof(struct ether_header) + FCS_LEN + VLAN_TAG_LEN); + mtu += (uint32_t)(sizeof(struct ether_header) + FCS_LEN + VLAN_TAG_LEN); + attr.value.u32 = mtu; - if (isMACsecPort(id)) + if (isMACsecPort(port.m_port_id)) { attr.value.u32 += MAX_MACSEC_SECTAG_SIZE; } - sai_status_t status = sai_port_api->set_port_attribute(id, &attr); + sai_status_t status = sai_port_api->set_port_attribute(port.m_port_id, &attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to set MTU %u to port pid:%" PRIx64 ", rv:%d", - attr.value.u32, id, status); + attr.value.u32, port.m_port_id, status); task_process_status handle_status = handleSaiSetStatus(SAI_API_PORT, status); if (handle_status != task_success) { return parseHandleSaiStatusFailure(handle_status); } } - SWSS_LOG_INFO("Set MTU %u to port pid:%" PRIx64, attr.value.u32, id); + + setGearboxPortsAttr(port, SAI_PORT_ATTR_MTU, &mtu); + SWSS_LOG_INFO("Set MTU %u to port pid:%" PRIx64, attr.value.u32, port.m_port_id); return true; } @@ -2021,7 +1996,7 @@ void PortsOrch::initPortSupportedSpeeds(const std::string& alias, sai_object_id_ /* * If Gearbox is enabled and this is a Gearbox port then set the attributes accordingly. */ -bool PortsOrch::setGearboxPortsAttr(Port &port, sai_port_attr_t id, void *value) +bool PortsOrch::setGearboxPortsAttr(const Port &port, sai_port_attr_t id, void *value) { bool status = false; @@ -2039,7 +2014,7 @@ bool PortsOrch::setGearboxPortsAttr(Port &port, sai_port_attr_t id, void *value) * If Gearbox is enabled and this is a Gearbox port then set the specific lane attribute. * Note: the appl_db is also updated (Gearbox config_db tables are TBA). */ -bool PortsOrch::setGearboxPortAttr(Port &port, dest_port_type_t port_type, sai_port_attr_t id, void *value) +bool PortsOrch::setGearboxPortAttr(const Port &port, dest_port_type_t port_type, sai_port_attr_t id, void *value) { sai_status_t status = SAI_STATUS_SUCCESS; sai_object_id_t dest_port_id; @@ -2093,6 +2068,15 @@ bool PortsOrch::setGearboxPortAttr(Port &port, dest_port_type_t port_type, sai_p } SWSS_LOG_NOTICE("BOX: Set %s lane %s %d", port.m_alias.c_str(), speed_attr.c_str(), speed); break; + case SAI_PORT_ATTR_MTU: + attr.id = id; + attr.value.u32 = *static_cast(value); + if (LINE_PORT_TYPE == port_type && isMACsecPort(dest_port_id)) + { + attr.value.u32 += MAX_MACSEC_SECTAG_SIZE; + } + SWSS_LOG_NOTICE("BOX: Set %s MTU %d", port.m_alias.c_str(), attr.value.u32); + break; default: return false; } @@ -3302,7 +3286,7 @@ void PortsOrch::doPortTask(Consumer &consumer) if (mtu != 0 && mtu != p.m_mtu) { - if (setPortMtu(p.m_port_id, mtu)) + if (setPortMtu(p, mtu)) { p.m_mtu = mtu; m_portList[alias] = p; @@ -6686,6 +6670,8 @@ bool PortsOrch::initGearboxPort(Port &port) SWSS_LOG_NOTICE("BOX: Connected Gearbox ports; system-side:0x%" PRIx64 " to line-side:0x%" PRIx64, systemPort, linePort); m_gearboxPortListLaneMap[port.m_port_id] = make_tuple(systemPort, linePort); port.m_line_side_id = linePort; + saiOidToAlias[systemPort] = port.m_alias; + saiOidToAlias[linePort] = port.m_alias; /* Add gearbox system/line port name map to counter table */ FieldValueTuple tuple(port.m_alias + "_system", sai_serialize_object_id(systemPort)); @@ -7192,6 +7178,13 @@ void PortsOrch::setMACsecEnabledState(sai_object_id_t port_id, bool enabled) { SWSS_LOG_ENTER(); + Port p; + if (!getPort(port_id, p)) + { + SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, port_id); + return; + } + if (enabled) { m_macsecEnabledPorts.insert(port_id); @@ -7200,6 +7193,8 @@ void PortsOrch::setMACsecEnabledState(sai_object_id_t port_id, bool enabled) { m_macsecEnabledPorts.erase(port_id); } + + setPortMtu(p, p.m_mtu); } bool PortsOrch::isMACsecPort(sai_object_id_t port_id) const diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 44ca2ead24..6be1239b36 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -18,7 +18,7 @@ #define FCS_LEN 4 #define VLAN_TAG_LEN 4 -#define MAX_MACSEC_SECTAG_SIZE (32) +#define MAX_MACSEC_SECTAG_SIZE 32 #define PORT_STAT_COUNTER_FLEX_COUNTER_GROUP "PORT_STAT_COUNTER" #define PORT_RATE_COUNTER_FLEX_COUNTER_GROUP "PORT_RATE_COUNTER" #define PORT_BUFFER_DROP_STAT_FLEX_COUNTER_GROUP "PORT_BUFFER_DROP_STAT" @@ -177,8 +177,6 @@ class PortsOrch : public Orch, public Subject void setMACsecEnabledState(sai_object_id_t port_id, bool enabled); bool isMACsecPort(sai_object_id_t port_id) const; - bool getPortMtu(sai_object_id_t id, sai_uint32_t &mtu); - bool setPortMtu(sai_object_id_t id, sai_uint32_t mtu); private: unique_ptr
m_counterTable; @@ -307,6 +305,7 @@ class PortsOrch : public Orch, public Subject bool setPortAdminStatus(Port &port, bool up); bool getPortAdminStatus(sai_object_id_t id, bool& up); + bool setPortMtu(const Port& port, sai_uint32_t mtu); bool setPortTpid(sai_object_id_t id, sai_uint16_t tpid); bool setPortPvid (Port &port, sai_uint32_t pvid); bool getPortPvid(Port &port, sai_uint32_t &pvid); @@ -321,8 +320,8 @@ class PortsOrch : public Orch, public Subject void initPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id); task_process_status setPortSpeed(Port &port, sai_uint32_t speed); bool getPortSpeed(sai_object_id_t id, sai_uint32_t &speed); - bool setGearboxPortsAttr(Port &port, sai_port_attr_t id, void *value); - bool setGearboxPortAttr(Port &port, dest_port_type_t port_type, sai_port_attr_t id, void *value); + bool setGearboxPortsAttr(const Port &port, sai_port_attr_t id, void *value); + bool setGearboxPortAttr(const Port &port, dest_port_type_t port_type, sai_port_attr_t id, void *value); task_process_status setPortAdvSpeeds(sai_object_id_t port_id, std::vector& speed_list); From e55a065960b2c2185d54f0dac62a83aa68742209 Mon Sep 17 00:00:00 2001 From: Junhua Zhai Date: Wed, 27 Jul 2022 09:47:51 +0000 Subject: [PATCH 3/4] Update fake_portorch.cpp --- orchagent/p4orch/tests/fake_portorch.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/orchagent/p4orch/tests/fake_portorch.cpp b/orchagent/p4orch/tests/fake_portorch.cpp index 51ff450312..a961ef6bb4 100644 --- a/orchagent/p4orch/tests/fake_portorch.cpp +++ b/orchagent/p4orch/tests/fake_portorch.cpp @@ -498,7 +498,7 @@ bool PortsOrch::getPortAdminStatus(sai_object_id_t id, bool &up) return true; } -bool PortsOrch::setPortMtu(sai_object_id_t id, sai_uint32_t mtu) +bool PortsOrch::setPortMtu(const Port &port, sai_uint32_t mtu) { return true; } @@ -562,12 +562,12 @@ bool PortsOrch::getPortSpeed(sai_object_id_t port_id, sai_uint32_t &speed) return true; } -bool PortsOrch::setGearboxPortsAttr(Port &port, sai_port_attr_t id, void *value) +bool PortsOrch::setGearboxPortsAttr(const Port &port, sai_port_attr_t id, void *value) { return true; } -bool PortsOrch::setGearboxPortAttr(Port &port, dest_port_type_t port_type, sai_port_attr_t id, void *value) +bool PortsOrch::setGearboxPortAttr(const Port &port, dest_port_type_t port_type, sai_port_attr_t id, void *value) { return true; } @@ -689,4 +689,4 @@ void PortsOrch::voqSyncDelLagMember(Port &lag, Port &port) std::unordered_set PortsOrch::generateCounterStats(const string &type, bool gearbox) { return {}; -} \ No newline at end of file +} From 9ec471adc95b9cea1ffcdc6686a875f5ed70f9fe Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 3 Aug 2022 02:32:22 +0000 Subject: [PATCH 4/4] Fix issue if MTU wasn't set Signed-off-by: Ze Gan --- orchagent/portsorch.cpp | 40 ++++++++++++++++++++++++++++++++++++++-- orchagent/portsorch.h | 1 + 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 4f684e88e9..1daf56ad9b 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1161,6 +1161,30 @@ bool PortsOrch::getPortAdminStatus(sai_object_id_t id, bool &up) return true; } +bool PortsOrch::getPortMtu(const Port& port, sai_uint32_t &mtu) +{ + SWSS_LOG_ENTER(); + + sai_attribute_t attr; + attr.id = SAI_PORT_ATTR_MTU; + + sai_status_t status = sai_port_api->get_port_attribute(port.m_port_id, 1, &attr); + + if (status != SAI_STATUS_SUCCESS) + { + return false; + } + + mtu = attr.value.u32 - (uint32_t)(sizeof(struct ether_header) + FCS_LEN + VLAN_TAG_LEN); + + if (isMACsecPort(port.m_port_id)) + { + mtu -= MAX_MACSEC_SECTAG_SIZE; + } + + return true; +} + bool PortsOrch::setPortMtu(const Port& port, sai_uint32_t mtu) { SWSS_LOG_ENTER(); @@ -1188,7 +1212,10 @@ bool PortsOrch::setPortMtu(const Port& port, sai_uint32_t mtu) } } - setGearboxPortsAttr(port, SAI_PORT_ATTR_MTU, &mtu); + if (m_gearboxEnabled) + { + setGearboxPortsAttr(port, SAI_PORT_ATTR_MTU, &mtu); + } SWSS_LOG_INFO("Set MTU %u to port pid:%" PRIx64, attr.value.u32, port.m_port_id); return true; } @@ -4551,6 +4578,12 @@ bool PortsOrch::initializePort(Port &port) return false; } + /* initialize port mtu */ + if (!getPortMtu(port, port.m_mtu)) + { + SWSS_LOG_ERROR("Failed to get initial port mtu %d", port.m_mtu); + } + /* * always initialize Port SAI_HOSTIF_ATTR_OPER_STATUS based on oper_status value in appDB. */ @@ -7451,7 +7484,10 @@ void PortsOrch::setMACsecEnabledState(sai_object_id_t port_id, bool enabled) m_macsecEnabledPorts.erase(port_id); } - setPortMtu(p, p.m_mtu); + if (p.m_mtu) + { + setPortMtu(p, p.m_mtu); + } } bool PortsOrch::isMACsecPort(sai_object_id_t port_id) const diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 8708efdb9d..4880112958 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -311,6 +311,7 @@ class PortsOrch : public Orch, public Subject bool setPortAdminStatus(Port &port, bool up); bool getPortAdminStatus(sai_object_id_t id, bool& up); + bool getPortMtu(const Port& port, sai_uint32_t &mtu); bool setPortMtu(const Port& port, sai_uint32_t mtu); bool setPortTpid(sai_object_id_t id, sai_uint16_t tpid); bool setPortPvid (Port &port, sai_uint32_t pvid);