Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EVPN]Fix fpmsyncd crash when EVPN type5 is received with bgp fib suppression enabled #3101

Merged
merged 4 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions fpmsyncd/routesync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as i see, this is only for evpn routes and not for regular routes, right? and no impact to warmboot etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. The flow will happen only for EVPN. No impact to warmboot.


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());
}

/*
Expand Down
2 changes: 1 addition & 1 deletion fpmsyncd/routesync.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
7 changes: 6 additions & 1 deletion tests/mock_tests/fake_producerstatetable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

}
70 changes: 66 additions & 4 deletions tests/mock_tests/fpmsyncd/test_routesync.cpp
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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");

}
Loading