From 4e4123d7d8962a6ee8cf88860f69c5ad5f6208d1 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Thu, 21 Mar 2024 00:52:54 +0000 Subject: [PATCH] [EVPN]Fix fpmsyncd crash when EVPN type5 is received with bgp fib suppression enabled --- fpmsyncd/routesync.cpp | 8 ++- fpmsyncd/routesync.h | 2 +- tests/mock_tests/fake_producerstatetable.cpp | 7 +- tests/mock_tests/fpmsyncd/test_routesync.cpp | 69 ++++++++++++++++++-- 4 files changed, 78 insertions(+), 8 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index caf6210084..0f6ee41188 100644 --- 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 index fd18b9d25a..eb07eb8f15 100644 --- 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 index 6221556f63..33fab17ecf 100644 --- 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 index debfa16d21..6a57963d89 100644 --- 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 #include +#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 m_db = make_shared("APPL_DB", 0); + shared_ptr m_pipeline = make_shared(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,43 @@ TEST_F(FpmSyncdResponseTest, WarmRestart) m_routeSync.onWarmStartEnd(applStateDb); } + +TEST_F(FpmSyncdResponseTest, testEvpn) +{ + struct nlmsghdr *nlh = (struct nlmsghdr *) malloc(NLMSG_SPACE(MAX_PAYLOAD)); + shared_ptr m_app_db; + m_app_db = make_shared("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; + 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 keys; + vector 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"); + +}