Skip to content

Commit

Permalink
[vslib]: Fixbug in cleanup MACsec device (#1059)
Browse files Browse the repository at this point in the history
The previous regex can only match one device so that the original MACsec devices cannot been cleanup by config reload.
  • Loading branch information
Pterosaur authored Jun 13, 2022
1 parent cdf9427 commit 9b0f773
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 10 deletions.
51 changes: 50 additions & 1 deletion unittest/vslib/TestMACsecManager.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#include "MACsecAttr.h"
#include "MACsecManager.h"

#include <swss/logger.h>

#include <gtest/gtest.h>

#include <vector>
#include <set>

using namespace saivs;

Expand Down Expand Up @@ -48,3 +50,50 @@ TEST(MACsecManager, update_macsec_sa_pn)
attr.m_sak = "";
manager.update_macsec_sa_pn(attr, 2);
}

class MockMACsecManager_CleanupMACsecDevice : public MACsecManager
{
public:
mutable std::set<std::string> m_rest_devices;
protected:
virtual bool exec(
_In_ const std::string &command,
_Out_ std::string &output) const
{
SWSS_LOG_ENTER();

if (command == "/sbin/ip macsec show")
{
output = "\
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\
cipher suite: GCM-AES-128, using ICV length 16\n\
TXSC: fe5400409b920001 on SA 0\n\
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\
cipher suite: GCM-AES-128, using ICV length 16\n\
TXSC: fe5400409b920001 on SA 0\n\
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\
cipher suite: GCM-AES-128, using ICV length 16\n\
TXSC: fe5400409b920001 on SA 0";
m_rest_devices.insert("macsec0");
m_rest_devices.insert("macsec1");
m_rest_devices.insert("macsec2");
return true;
}
else if (command.rfind("/sbin/ip link del ") == 0)
{
std::string name = command.substr(strlen("/sbin/ip link del "));
m_rest_devices.erase(name);
return true;
}
return MACsecManager::exec(command, output);
}
};

TEST(MACsecManager, cleanup_macsec_device)
{
MockMACsecManager_CleanupMACsecDevice manager;

manager.cleanup_macsec_device();

EXPECT_EQ(manager.m_rest_devices.size(), 0);
}
6 changes: 3 additions & 3 deletions vslib/MACsecManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ MACsecManager::MACsecManager()
{
SWSS_LOG_ENTER();

cleanup_macsec_device();
// empty
}

MACsecManager::~MACsecManager()
{
SWSS_LOG_ENTER();

cleanup_macsec_device();
// empty
}

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

Expand Down
8 changes: 4 additions & 4 deletions vslib/MACsecManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ namespace saivs
_In_ const MACsecAttr &attr,
_Out_ sai_uint64_t &pn) const;

private:
void cleanup_macsec_device() const;

protected:

bool create_macsec_egress_sc(
_In_ const MACsecAttr &attr);
Expand Down Expand Up @@ -122,12 +124,10 @@ namespace saivs
_In_ sai_int32_t direction,
_In_ const std::string &sci) const;

void cleanup_macsec_device() const;

std::string shellquote(
_In_ const std::string &str) const;

bool exec(
virtual bool exec(
_In_ const std::string &command,
_Out_ std::string &output) const;

Expand Down
6 changes: 4 additions & 2 deletions vslib/SwitchStateBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ SwitchStateBase::SwitchStateBase(
{
SWSS_LOG_ENTER();

// empty
m_macsecManager.cleanup_macsec_device();
}

SwitchStateBase::SwitchStateBase(
Expand All @@ -34,6 +34,8 @@ SwitchStateBase::SwitchStateBase(
{
SWSS_LOG_ENTER();

m_macsecManager.cleanup_macsec_device();

if (warmBootState)
{
for (auto& kvp: warmBootState->m_objectHash)
Expand All @@ -57,7 +59,7 @@ SwitchStateBase::~SwitchStateBase()
{
SWSS_LOG_ENTER();

// empty
m_macsecManager.cleanup_macsec_device();
}

sai_status_t SwitchStateBase::create(
Expand Down

0 comments on commit 9b0f773

Please sign in to comment.