From 9b0f77384ef31c3dfee2a622f45df53c42b34baf Mon Sep 17 00:00:00 2001
From: Ze Gan <ganze718@gmail.com>
Date: Mon, 13 Jun 2022 07:57:23 -0700
Subject: [PATCH] [vslib]: Fixbug in cleanup MACsec device (#1059)

The previous regex can only match one device so that the original MACsec devices cannot been cleanup by config reload.
---
 unittest/vslib/TestMACsecManager.cpp | 51 +++++++++++++++++++++++++++-
 vslib/MACsecManager.cpp              |  6 ++--
 vslib/MACsecManager.h                |  8 ++---
 vslib/SwitchStateBase.cpp            |  6 ++--
 4 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/unittest/vslib/TestMACsecManager.cpp b/unittest/vslib/TestMACsecManager.cpp
index 049f3fecb4b9..bc1468b4517f 100644
--- a/unittest/vslib/TestMACsecManager.cpp
+++ b/unittest/vslib/TestMACsecManager.cpp
@@ -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;
 
@@ -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);
+}
diff --git a/vslib/MACsecManager.cpp b/vslib/MACsecManager.cpp
index 389f9f293069..c9b06e3920ef 100644
--- a/vslib/MACsecManager.cpp
+++ b/vslib/MACsecManager.cpp
@@ -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(
@@ -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());
 
diff --git a/vslib/MACsecManager.h b/vslib/MACsecManager.h
index 8825edef80cd..8305f21c4d8b 100644
--- a/vslib/MACsecManager.h
+++ b/vslib/MACsecManager.h
@@ -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);
@@ -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;
 
diff --git a/vslib/SwitchStateBase.cpp b/vslib/SwitchStateBase.cpp
index 1986c08d4f3e..0116c0f169ab 100644
--- a/vslib/SwitchStateBase.cpp
+++ b/vslib/SwitchStateBase.cpp
@@ -21,7 +21,7 @@ SwitchStateBase::SwitchStateBase(
 {
     SWSS_LOG_ENTER();
 
-    // empty
+    m_macsecManager.cleanup_macsec_device();
 }
 
 SwitchStateBase::SwitchStateBase(
@@ -34,6 +34,8 @@ SwitchStateBase::SwitchStateBase(
 {
     SWSS_LOG_ENTER();
 
+    m_macsecManager.cleanup_macsec_device();
+
     if (warmBootState)
     {
         for (auto& kvp: warmBootState->m_objectHash)
@@ -57,7 +59,7 @@ SwitchStateBase::~SwitchStateBase()
 {
     SWSS_LOG_ENTER();
 
-    // empty
+    m_macsecManager.cleanup_macsec_device();
 }
 
 sai_status_t SwitchStateBase::create(