From cfcf3d8788e04a0500aeb30d45a906db27c67b10 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Tue, 16 Aug 2022 09:18:59 +0800 Subject: [PATCH] [macsec]: Set MTU for MACsec (#2398) What I did Set extra MTU for MACsec enabled port. Why I did it MACsec frame will expend the packet with MACsec SecTAG, Otherwise if a packet length equals the MTU which will be dropped by SAI port. Signed-off-by: Ze Gan Co-authored-by: Junhua Zhai --- orchagent/macsecorch.cpp | 4 + orchagent/p4orch/tests/fake_portorch.cpp | 6 +- orchagent/portsorch.cpp | 103 +++++++++++++++++++++-- orchagent/portsorch.h | 13 ++- 4 files changed, 110 insertions(+), 16 deletions(-) diff --git a/orchagent/macsecorch.cpp b/orchagent/macsecorch.cpp index 67ba904043d3..9a5e48f8832f 100644 --- a/orchagent/macsecorch.cpp +++ b/orchagent/macsecorch.cpp @@ -1275,6 +1275,8 @@ bool MACsecOrch::createMACsecPort( phy); }); + m_port_orch->setMACsecEnabledState(port_id, true); + if (phy) { if (!setPFCForward(port_id, true)) @@ -1542,6 +1544,8 @@ bool MACsecOrch::deleteMACsecPort( result &= false; } + m_port_orch->setMACsecEnabledState(port_id, false); + if (phy) { if (!setPFCForward(port_id, false)) diff --git a/orchagent/p4orch/tests/fake_portorch.cpp b/orchagent/p4orch/tests/fake_portorch.cpp index 0762dd035748..a21294d8dd2e 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; } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 26219d4d35ba..f46c23be4f10 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1168,27 +1168,62 @@ 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::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(); 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; - sai_status_t status = sai_port_api->set_port_attribute(id, &attr); + if (isMACsecPort(port.m_port_id)) + { + attr.value.u32 += MAX_MACSEC_SECTAG_SIZE; + } + + 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); + + 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; } @@ -2144,7 +2179,7 @@ void PortsOrch::initPortSupportedFecModes(const std::string& alias, sai_object_i /* * 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; @@ -2162,7 +2197,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; @@ -2216,6 +2251,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; } @@ -3565,7 +3609,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; @@ -4632,6 +4676,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. */ @@ -7008,6 +7058,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)); @@ -7510,6 +7562,39 @@ 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(); + + 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); + } + else + { + m_macsecEnabledPorts.erase(port_id); + } + + if (p.m_mtu) + { + setPortMtu(p, p.m_mtu); + } +} + +bool PortsOrch::isMACsecPort(sai_object_id_t port_id) const +{ + SWSS_LOG_ENTER(); + + return m_macsecEnabledPorts.find(port_id) != m_macsecEnabledPorts.end(); +} + /* Refresh the per-port Auto-Negotiation operational states */ void PortsOrch::refreshPortStateAutoNeg(const Port &port) { @@ -7624,4 +7709,4 @@ void PortsOrch::doTask(swss::SelectableTimer &timer) { m_port_state_poller->stop(); } -} +} \ No newline at end of file diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 28e576e9064f..d801bcd87ef4 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" @@ -175,6 +176,9 @@ 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; + private: unique_ptr m_counterTable; unique_ptr
m_counterLagTable; @@ -310,7 +314,8 @@ 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 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); bool getPortPvid(Port &port, sai_uint32_t &pvid); @@ -328,8 +333,8 @@ class PortsOrch : public Orch, public Subject void initPortSupportedFecModes(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); bool getPortAdvSpeeds(const Port& port, bool remote, std::vector& speed_list); bool getPortAdvSpeeds(const Port& port, bool remote, string& adv_speeds); @@ -403,8 +408,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 */