Skip to content

Commit

Permalink
[macsec]: Set MTU for MACsec (sonic-net#2398)
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Co-authored-by: Junhua Zhai <[email protected]>
  • Loading branch information
@Pterosaur @jimmyzhai
Pterosaur and jimmyzhai committed Sep 23, 2022
1 parent 88371f7 commit 70e739f
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 16 deletions.
4 changes: 4 additions & 0 deletions orchagent/macsecorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,8 @@ bool MACsecOrch::createMACsecPort(
phy);
});

m_port_orch->setMACsecEnabledState(port_id, true);

if (phy)
{
if (!setPFCForward(port_id, true))
Expand Down Expand Up @@ -1542,6 +1544,8 @@ bool MACsecOrch::deleteMACsecPort(
result &= false;
}

m_port_orch->setMACsecEnabledState(port_id, false);

if (phy)
{
if (!setPFCForward(port_id, false))
Expand Down
6 changes: 3 additions & 3 deletions orchagent/p4orch/tests/fake_portorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,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;
}
Expand Down Expand Up @@ -561,12 +561,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;
}
Expand Down
103 changes: 94 additions & 9 deletions orchagent/portsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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<sai_uint32_t*>(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;
}
Expand Down Expand Up @@ -3574,7 +3618,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;
Expand Down Expand Up @@ -4641,6 +4685,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.
*/
Expand Down Expand Up @@ -7017,6 +7067,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));
Expand Down Expand Up @@ -7519,6 +7571,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)
{
Expand Down Expand Up @@ -7633,4 +7718,4 @@ void PortsOrch::doTask(swss::SelectableTimer &timer)
{
m_port_state_poller->stop();
}
}
}
13 changes: 9 additions & 4 deletions orchagent/portsorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<Table> m_counterTable;
unique_ptr<Table> m_counterLagTable;
Expand Down Expand Up @@ -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);
Expand All @@ -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<sai_uint32_t>& speed_list);
bool getPortAdvSpeeds(const Port& port, bool remote, string& adv_speeds);
Expand Down Expand Up @@ -403,8 +408,8 @@ class PortsOrch : public Orch, public Subject
void voqSyncAddLagMember(Port &lag, Port &port);
void voqSyncDelLagMember(Port &lag, Port &port);
unique_ptr<LagIdAllocator> m_lagIdAllocator;
set<sai_object_id_t> m_macsecEnabledPorts;

std::unordered_set<std::string> generateCounterStats(const string& type, bool gearbox = false);

};
#endif /* SWSS_PORTSORCH_H */

0 comments on commit 70e739f

Please sign in to comment.