Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macsec]: Set MTU for MACsec #2398

Merged
merged 5 commits into from
Aug 16, 2022
Merged

[macsec]: Set MTU for MACsec #2398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16cae21
Set mtu for MACsec
Pterosaur Jul 26, 2022
b8f618c
Support port mtu update w/o gearbox
jimmyzhai Jul 27, 2022
e55a065
Update fake_portorch.cpp
jimmyzhai Jul 27, 2022
e3ce7ca
Merge branch 'master' into fix_mtu_macsec
Pterosaur Jul 30, 2022
9ec471a
Fix issue if MTU wasn't set
Pterosaur Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions orchagent/macsecorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

judyjoseph marked this conversation as resolved.
Show resolved Hide resolved
if (phy)
{
if (!setPFCForward(port_id, true))
Expand Down Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions orchagent/macsecorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
54 changes: 54 additions & 0 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely needed for non-gearbox based devices.
As discussed, do we need this additional 32 bytes to be added for the gbsyncd based devices as well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we also needed it for line_side_port for gearbox. @jimmyzhai will add more on this PR for adapting gearbox mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have updated the fix to update port (w/o gearbox) MTU for MACsec.

{
attr.value.u32 += MAX_MACSEC_SECTAG_SIZE;
}

sai_status_t status = sai_port_api->set_port_attribute(id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
Expand Down Expand Up @@ -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();
}
9 changes: 7 additions & 2 deletions orchagent/portsorch.h
View file
Open in desktop
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 @@ -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<Table> m_counterTable;
unique_ptr<Table> m_counterLagTable;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -374,8 +379,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 */