Skip to content

Commit 9b0f773

Browse files
authored
[vslib]: Fixbug in cleanup MACsec device (sonic-net#1059)
The previous regex can only match one device so that the original MACsec devices cannot been cleanup by config reload.
1 parent cdf9427 commit 9b0f773

File tree

4 files changed

+61
-10
lines changed

4 files changed

+61
-10
lines changed

unittest/vslib/TestMACsecManager.cpp

+50-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
#include "MACsecAttr.h"
22
#include "MACsecManager.h"
33

4+
#include <swss/logger.h>
5+
46
#include <gtest/gtest.h>
57

6-
#include <vector>
8+
#include <set>
79

810
using namespace saivs;
911

@@ -48,3 +50,50 @@ TEST(MACsecManager, update_macsec_sa_pn)
4850
attr.m_sak = "";
4951
manager.update_macsec_sa_pn(attr, 2);
5052
}
53+
54+
class MockMACsecManager_CleanupMACsecDevice : public MACsecManager
55+
{
56+
public:
57+
mutable std::set<std::string> m_rest_devices;
58+
protected:
59+
virtual bool exec(
60+
_In_ const std::string &command,
61+
_Out_ std::string &output) const
62+
{
63+
SWSS_LOG_ENTER();
64+
65+
if (command == "/sbin/ip macsec show")
66+
{
67+
output = "\
68+
2774: macsec0: protect on validate strict sc off sa off encrypt on send_sci on end_station off scb off replay on window 0\n\
69+
cipher suite: GCM-AES-128, using ICV length 16\n\
70+
TXSC: fe5400409b920001 on SA 0\n\
71+
2775: macsec1: protect on validate strict sc off sa off encrypt on send_sci on end_station off scb off replay on window 0\n\
72+
cipher suite: GCM-AES-128, using ICV length 16\n\
73+
TXSC: fe5400409b920001 on SA 0\n\
74+
2776: macsec2: protect on validate strict sc off sa off encrypt on send_sci on end_station off scb off replay on window 0\n\
75+
cipher suite: GCM-AES-128, using ICV length 16\n\
76+
TXSC: fe5400409b920001 on SA 0";
77+
m_rest_devices.insert("macsec0");
78+
m_rest_devices.insert("macsec1");
79+
m_rest_devices.insert("macsec2");
80+
return true;
81+
}
82+
else if (command.rfind("/sbin/ip link del ") == 0)
83+
{
84+
std::string name = command.substr(strlen("/sbin/ip link del "));
85+
m_rest_devices.erase(name);
86+
return true;
87+
}
88+
return MACsecManager::exec(command, output);
89+
}
90+
};
91+
92+
TEST(MACsecManager, cleanup_macsec_device)
93+
{
94+
MockMACsecManager_CleanupMACsecDevice manager;
95+
96+
manager.cleanup_macsec_device();
97+
98+
EXPECT_EQ(manager.m_rest_devices.size(), 0);
99+
}

vslib/MACsecManager.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ MACsecManager::MACsecManager()
2121
{
2222
SWSS_LOG_ENTER();
2323

24-
cleanup_macsec_device();
24+
// empty
2525
}
2626

2727
MACsecManager::~MACsecManager()
2828
{
2929
SWSS_LOG_ENTER();
3030

31-
cleanup_macsec_device();
31+
// empty
3232
}
3333

3434
bool MACsecManager::create_macsec_port(
@@ -895,7 +895,7 @@ void MACsecManager::cleanup_macsec_device() const
895895
// cipher suite: GCM-AES-128, using ICV length 16
896896
// TXSC: fe5400409b920001 on SA 0
897897
// Use pattern : '^\d+:\s*(\w+):' to extract all MACsec interface names
898-
const std::regex pattern("^\\d+:\\s*(\\w+):");
898+
const std::regex pattern("\\d+:\\s*(\\w+):");
899899
std::smatch matches;
900900
std::string::const_iterator searchPos(macsecInfos.cbegin());
901901

vslib/MACsecManager.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ namespace saivs
4444
_In_ const MACsecAttr &attr,
4545
_Out_ sai_uint64_t &pn) const;
4646

47-
private:
47+
void cleanup_macsec_device() const;
48+
49+
protected:
4850

4951
bool create_macsec_egress_sc(
5052
_In_ const MACsecAttr &attr);
@@ -122,12 +124,10 @@ namespace saivs
122124
_In_ sai_int32_t direction,
123125
_In_ const std::string &sci) const;
124126

125-
void cleanup_macsec_device() const;
126-
127127
std::string shellquote(
128128
_In_ const std::string &str) const;
129129

130-
bool exec(
130+
virtual bool exec(
131131
_In_ const std::string &command,
132132
_Out_ std::string &output) const;
133133

vslib/SwitchStateBase.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ SwitchStateBase::SwitchStateBase(
2121
{
2222
SWSS_LOG_ENTER();
2323

24-
// empty
24+
m_macsecManager.cleanup_macsec_device();
2525
}
2626

2727
SwitchStateBase::SwitchStateBase(
@@ -34,6 +34,8 @@ SwitchStateBase::SwitchStateBase(
3434
{
3535
SWSS_LOG_ENTER();
3636

37+
m_macsecManager.cleanup_macsec_device();
38+
3739
if (warmBootState)
3840
{
3941
for (auto& kvp: warmBootState->m_objectHash)
@@ -57,7 +59,7 @@ SwitchStateBase::~SwitchStateBase()
5759
{
5860
SWSS_LOG_ENTER();
5961

60-
// empty
62+
m_macsecManager.cleanup_macsec_device();
6163
}
6264

6365
sai_status_t SwitchStateBase::create(

0 commit comments

Comments
 (0)