[EVPN]Fix fpmsyncd crash when EVPN type5 is received with bgp fib sup…

…pression enabled (#3101) * [EVPN]Fix fpmsyncd crash when EVPN type5 is received with bgp fib suppression enabled
sonic-net · Apr 5, 2024 · cbc263a · cbc263a
1 parent 4af6116
commit cbc263a
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 8 deletions.
diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp
@@ -465,6 +465,7 @@ void RouteSync::onEvpnRouteMsg(struct nlmsghdr *h, int len)
                 inet_ntop(rtm->rtm_family, dstaddr, buf, MAX_ADDR_SIZE), dst_len);
     }
 
+    auto proto_str = getProtocolString(rtm->rtm_protocol);
     SWSS_LOG_INFO("Receive route message dest ip prefix: %s Op:%s", 
                     destipprefix,
                     nlmsg_type == RTM_NEWROUTE ? "add":"del");
@@ -550,17 +551,20 @@ void RouteSync::onEvpnRouteMsg(struct nlmsghdr *h, int len)
     FieldValueTuple intf("ifname", intf_list);
     FieldValueTuple vni("vni_label", vni_list);
     FieldValueTuple mac("router_mac", mac_list);
+    FieldValueTuple proto("protocol", proto_str);
 
     fvVector.push_back(nh);
     fvVector.push_back(intf);
     fvVector.push_back(vni);
     fvVector.push_back(mac);
+    fvVector.push_back(proto);
 
     if (!warmRestartInProgress)
     {
         m_routeTable.set(destipprefix, fvVector);
-        SWSS_LOG_DEBUG("RouteTable set msg: %s vtep:%s vni:%s mac:%s intf:%s",
-                       destipprefix, nexthops.c_str(), vni_list.c_str(), mac_list.c_str(), intf_list.c_str());
+        SWSS_LOG_DEBUG("RouteTable set msg: %s vtep:%s vni:%s mac:%s intf:%s protocol:%s",
+                       destipprefix, nexthops.c_str(), vni_list.c_str(), mac_list.c_str(), intf_list.c_str(),
+                       proto_str.c_str());
     }
 
     /*

diff --git a/fpmsyncd/routesync.h b/fpmsyncd/routesync.h
@@ -115,7 +115,7 @@ class RouteSync : public NetMsg
                           string& mac_list, string& intf_list,
                           string rmac, string vlan_id);
 
-    bool getEvpnNextHop(struct nlmsghdr *h, int received_bytes, struct rtattr *tb[],
+    virtual bool getEvpnNextHop(struct nlmsghdr *h, int received_bytes, struct rtattr *tb[],
                         string& nexthops, string& vni_list, string& mac_list,
                         string& intf_list);
 

diff --git a/tests/mock_tests/fake_producerstatetable.cpp b/tests/mock_tests/fake_producerstatetable.cpp
@@ -4,8 +4,13 @@ using namespace std;
 
 namespace swss
 {
+
 ProducerStateTable::ProducerStateTable(RedisPipeline *pipeline, const string &tableName, bool buffered)
-    : TableBase(tableName, SonicDBConfig::getSeparator(pipeline->getDBConnector())), TableName_KeySet(tableName) {}
+    : TableBase(tableName, SonicDBConfig::getSeparator(pipeline->getDBConnector())), TableName_KeySet(tableName), m_buffered(buffered)
+    , m_pipeowned(false)
+    , m_tempViewActive(false)
+    , m_pipe(pipeline) {}
 
 ProducerStateTable::~ProducerStateTable() {}
+
 }
diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp
@@ -1,12 +1,32 @@
-#include "fpmsyncd/routesync.h"
+#include "redisutility.h"
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
+#include "mock_table.h"
+#define private public
+#include "fpmsyncd/routesync.h"
+#undef private
 
 using namespace swss;
+#define MAX_PAYLOAD 1024
 
 using ::testing::_;
 
+class MockRouteSync : public RouteSync
+{
+public:
+    MockRouteSync(RedisPipeline *m_pipeline) : RouteSync(m_pipeline)
+    {
+    }
+
+    ~MockRouteSync()
+    {
+    }
+    MOCK_METHOD(bool, getEvpnNextHop, (nlmsghdr *, int,
+                               rtattr *[], std::string&,
+                               std::string& , std::string&,
+                               std::string&), (override));
+};
 class MockFpm : public FpmInterface
 {
 public:
@@ -42,10 +62,11 @@ class FpmSyncdResponseTest : public ::testing::Test
     {
     }
 
-    DBConnector m_db{"APPL_DB", 0};
-    RedisPipeline m_pipeline{&m_db, 1};
-    RouteSync m_routeSync{&m_pipeline};
+    shared_ptr<swss::DBConnector> m_db = make_shared<swss::DBConnector>("APPL_DB", 0);
+    shared_ptr<RedisPipeline> m_pipeline = make_shared<RedisPipeline>(m_db.get());
+    RouteSync m_routeSync{m_pipeline.get()};
     MockFpm m_mockFpm{&m_routeSync};
+    MockRouteSync m_mockRouteSync{m_pipeline.get()};
 };
 
 TEST_F(FpmSyncdResponseTest, RouteResponseFeedbackV4)
@@ -170,3 +191,44 @@ TEST_F(FpmSyncdResponseTest, WarmRestart)
 
     m_routeSync.onWarmStartEnd(applStateDb);
 }
+
+TEST_F(FpmSyncdResponseTest, testEvpn)
+{
+    struct nlmsghdr *nlh = (struct nlmsghdr *) malloc(NLMSG_SPACE(MAX_PAYLOAD));
+    shared_ptr<swss::DBConnector> m_app_db;
+    m_app_db = make_shared<swss::DBConnector>("APPL_DB", 0);
+    Table app_route_table(m_app_db.get(), APP_ROUTE_TABLE_NAME);
+
+    memset(nlh, 0, NLMSG_SPACE(MAX_PAYLOAD));
+    nlh->nlmsg_type = RTM_NEWROUTE;
+    struct rtmsg rtm;
+    rtm.rtm_family = AF_INET;
+    rtm.rtm_protocol = 200;
+    rtm.rtm_type = RTN_UNICAST;
+    rtm.rtm_table = 0;
+    rtm.rtm_dst_len = 32;
+    nlh->nlmsg_len = NLMSG_SPACE(MAX_PAYLOAD);
+    memcpy(NLMSG_DATA(nlh), &rtm, sizeof(rtm));
+
+    EXPECT_CALL(m_mockRouteSync, getEvpnNextHop(_, _, _, _, _, _, _)).Times(testing::AtLeast(1)).WillOnce([&](
+                               struct nlmsghdr *h, int received_bytes,
+                               struct rtattr *tb[], std::string& nexthops,
+                               std::string& vni_list, std::string& mac_list,
+                               std::string& intf_list)-> bool {
+        vni_list="100";
+        mac_list="aa:aa:aa:aa:aa:aa";
+        intf_list="Ethernet0";
+        nexthops = "1.1.1.1";
+        return true;
+    });
+    m_mockRouteSync.onMsgRaw(nlh);
+    vector<string> keys;
+    vector<FieldValueTuple> fieldValues;
+    app_route_table.getKeys(keys);
+    ASSERT_EQ(keys.size(), 1);
+
+    app_route_table.get(keys[0], fieldValues);
+    auto value = swss::fvsGetValue(fieldValues, "protocol", true);
+    ASSERT_EQ(value.get(), "0xc8");
+
+}