Skip to content

Commit 28aa309

Browse files
stepanblyschakstepanblyschak
authored
[fpm] Fix FpmLink to read all netlink messages from FPM message (#2492)
In case of using dplane_fpm_nl zebra plugin we receive RTM_DELROUTE followed by RTM_NEWROUTE in a single FPM message when route attributes change (i.e nexthops change). Current implementation can only read the first one and ignores the rest. What I did I fixed FPM implementation to read multiple nl messages in a single FPM message. Why I did it Trying to move towards using dplane_fpm_nl. How I verified it UT and using dplane_fpm_nl zebra plugin. Details if related
1 parent da56bd6 commit 28aa309

5 files changed

Lines changed: 134 additions & 34 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ swssconfig/swssplayer
7474
tlm_teamd/tlm_teamd
7575
teamsyncd/teamsyncd
7676
tests/tests
77+
tests/mock_tests/tests_fpmsyncd
7778

7879

7980
# Test Files #

‎fpmsyncd/fpmlink.cpp‎

Lines changed: 50 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -210,52 +210,70 @@ uint64_t FpmLink::readData()
210210
hdr = reinterpret_cast<fpm_msg_hdr_t *>(static_cast<void *>(m_messageBuffer + start));
211211
left = m_pos - start;
212212
if (left < FPM_MSG_HDR_LEN)
213+
{
213214
break;
215+
}
216+
214217
/* fpm_msg_len includes header size */
215218
msg_len = fpm_msg_len(hdr);
216219
if (left < msg_len)
220+
{
217221
break;
222+
}
218223

219224
if (!fpm_msg_ok(hdr, left))
220-
throw system_error(make_error_code(errc::bad_message), "Malformed FPM message received");
221-
222-
if (hdr->msg_type == FPM_MSG_TYPE_NETLINK)
223225
{
224-
bool isRaw = false;
225-
226-
nlmsghdr *nl_hdr = (nlmsghdr *)fpm_msg_data(hdr);
227-
228-
/*
229-
* EVPN Type5 Add Routes need to be process in Raw mode as they contain
230-
* RMAC, VLAN and L3VNI information.
231-
* Where as all other route will be using rtnl api to extract information
232-
* from the netlink msg.
233-
* */
234-
isRaw = isRawProcessing(nl_hdr);
235-
236-
nl_msg *msg = nlmsg_convert(nl_hdr);
237-
if (msg == NULL)
238-
{
239-
throw system_error(make_error_code(errc::bad_message), "Unable to convert nlmsg");
240-
}
226+
throw system_error(make_error_code(errc::bad_message), "Malformed FPM message received");
227+
}
241228

242-
nlmsg_set_proto(msg, NETLINK_ROUTE);
229+
processFpmMessage(hdr);
243230

244-
if (isRaw)
245-
{
246-
/* EVPN Type5 Add route processing */
247-
processRawMsg(nl_hdr);
248-
}
249-
else
250-
{
251-
NetDispatcher::getInstance().onNetlinkMessage(msg);
252-
}
253-
nlmsg_free(msg);
254-
}
255231
start += msg_len;
256232
}
257233

258234
memmove(m_messageBuffer, m_messageBuffer + start, m_pos - start);
259235
m_pos = m_pos - (uint32_t)start;
260236
return 0;
261237
}
238+
239+
void FpmLink::processFpmMessage(fpm_msg_hdr_t* hdr)
240+
{
241+
size_t msg_len = fpm_msg_len(hdr);
242+
243+
if (hdr->msg_type != FPM_MSG_TYPE_NETLINK)
244+
{
245+
return;
246+
}
247+
nlmsghdr *nl_hdr = (nlmsghdr *)fpm_msg_data(hdr);
248+
249+
/* Read all netlink messages inside FPM message */
250+
for (; NLMSG_OK (nl_hdr, msg_len); nl_hdr = NLMSG_NEXT(nl_hdr, msg_len))
251+
{
252+
/*
253+
* EVPN Type5 Add Routes need to be process in Raw mode as they contain
254+
* RMAC, VLAN and L3VNI information.
255+
* Where as all other route will be using rtnl api to extract information
256+
* from the netlink msg.
257+
*/
258+
bool isRaw = isRawProcessing(nl_hdr);
259+
260+
nl_msg *msg = nlmsg_convert(nl_hdr);
261+
if (msg == NULL)
262+
{
263+
throw system_error(make_error_code(errc::bad_message), "Unable to convert nlmsg");
264+
}
265+
266+
nlmsg_set_proto(msg, NETLINK_ROUTE);
267+
268+
if (isRaw)
269+
{
270+
/* EVPN Type5 Add route processing */
271+
processRawMsg(nl_hdr);
272+
}
273+
else
274+
{
275+
NetDispatcher::getInstance().onNetlinkMessage(msg);
276+
}
277+
nlmsg_free(msg);
278+
}
279+
}

‎fpmsyncd/fpmlink.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ class FpmLink : public Selectable {
3939
m_routesync->onMsgRaw(h);
4040
};
4141

42+
void processFpmMessage(fpm_msg_hdr_t* hdr);
43+
4244
private:
4345
RouteSync *m_routesync;
4446
unsigned int m_bufSize;

‎tests/mock_tests/Makefile.am‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ P4_ORCH_DIR = $(top_srcdir)/orchagent/p4orch
44

55
CFLAGS_SAI = -I /usr/include/sai
66

7-
TESTS = tests tests_intfmgrd tests_portsyncd
7+
TESTS = tests tests_intfmgrd tests_portsyncd tests_fpmsyncd
88

9-
noinst_PROGRAMS = tests tests_intfmgrd tests_portsyncd
9+
noinst_PROGRAMS = tests tests_intfmgrd tests_portsyncd tests_fpmsyncd
1010

1111
LDADD_SAI = -lsaimeta -lsaimetadata -lsaivs -lsairedis
1212

@@ -166,3 +166,14 @@ tests_intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST
166166
tests_intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_intfmgrd_INCLUDES)
167167
tests_intfmgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
168168
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread
169+
170+
## fpmsyncd unit tests
171+
172+
tests_fpmsyncd_SOURCES = fpmsyncd/test_fpmlink.cpp \
173+
$(top_srcdir)/fpmsyncd/fpmlink.cpp
174+
175+
tests_fpmsyncd_INCLUDES = $(tests_INCLUDES) -I$(top_srcdir)/tests_fpmsyncd -I$(top_srcdir)/lib -I$(top_srcdir)/warmrestart
176+
tests_fpmsyncd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
177+
tests_fpmsyncd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_fpmsyncd_INCLUDES)
178+
tests_fpmsyncd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
179+
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread -lgmock -lgmock_main

‎tests/mock_tests/fpmsyncd/test_fpmlink.cpp‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
#include "fpmsyncd/fpmlink.h"
2+
3+
#include <swss/netdispatcher.h>
4+
5+
#include <gtest/gtest.h>
6+
#include <gmock/gmock.h>
7+
8+
using namespace swss;
9+
10+
using ::testing::_;
11+
12+
class MockMsgHandler : public NetMsg
13+
{
14+
public:
15+
MOCK_METHOD2(onMsg, void(int, nl_object*));
16+
};
17+
18+
class FpmLinkTest : public ::testing::Test
19+
{
20+
public:
21+
void SetUp() override
22+
{
23+
NetDispatcher::getInstance().registerMessageHandler(RTM_NEWROUTE, &m_mock);
24+
NetDispatcher::getInstance().registerMessageHandler(RTM_DELROUTE, &m_mock);
25+
}
26+
27+
void TearDown() override
28+
{
29+
NetDispatcher::getInstance().unregisterMessageHandler(RTM_NEWROUTE);
30+
NetDispatcher::getInstance().unregisterMessageHandler(RTM_DELROUTE);
31+
}
32+
33+
FpmLink m_fpm{nullptr};
34+
MockMsgHandler m_mock;
35+
};
36+
37+
TEST_F(FpmLinkTest, SingleNlMessageInFpmMessage)
38+
{
39+
// Single FPM message containing single RTM_NEWROUTE
40+
alignas(fpm_msg_hdr_t) unsigned char fpmMsgBuffer[] = {
41+
0x01, 0x01, 0x00, 0x40, 0x3C, 0x00, 0x00, 0x00, 0x18, 0x00, 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0xE0,
42+
0x12, 0x6F, 0xC4, 0x02, 0x18, 0x00, 0x00, 0xFE, 0x02, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00,
43+
0x01, 0x00, 0x01, 0x01, 0x01, 0x00, 0x08, 0x00, 0x06, 0x00, 0x14, 0x00, 0x00, 0x00, 0x08, 0x00, 0x05,
44+
0x00, 0xAC, 0x1E, 0x38, 0xA6, 0x08, 0x00, 0x04, 0x00, 0x06, 0x00, 0x00, 0x00
45+
};
46+
47+
EXPECT_CALL(m_mock, onMsg(_, _)).Times(1);
48+
49+
m_fpm.processFpmMessage(reinterpret_cast<fpm_msg_hdr_t*>(static_cast<void*>(fpmMsgBuffer)));
50+
}
51+
52+
TEST_F(FpmLinkTest, TwoNlMessagesInFpmMessage)
53+
{
54+
// Single FPM message containing RTM_DELROUTE and RTM_NEWROUTE
55+
alignas(fpm_msg_hdr_t) unsigned char fpmMsgBuffer[] = {
56+
0x01, 0x01, 0x00, 0x6C, 0x2C, 0x00, 0x00, 0x00, 0x19, 0x00, 0x01, 0x04, 0x00, 0x00, 0x00, 0x00, 0xE0, 0x12,
57+
0x6F, 0xC4, 0x02, 0x18, 0x00, 0x00, 0xFE, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x01, 0x00,
58+
0x01, 0x01, 0x01, 0x00, 0x08, 0x00, 0x06, 0x00, 0x14, 0x00, 0x00, 0x00, 0x3C, 0x00, 0x00, 0x00, 0x18, 0x00,
59+
0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0xE0, 0x12, 0x6F, 0xC4, 0x02, 0x18, 0x00, 0x00, 0xFE, 0x02, 0x00, 0x01,
60+
0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x01, 0x01, 0x01, 0x00, 0x08, 0x00, 0x06, 0x00, 0x14, 0x00,
61+
0x00, 0x00, 0x08, 0x00, 0x05, 0x00, 0xAC, 0x1E, 0x38, 0xA7, 0x08, 0x00, 0x04, 0x00, 0x06, 0x00, 0x00, 0x00
62+
};
63+
64+
EXPECT_CALL(m_mock, onMsg(_, _)).Times(2);
65+
66+
m_fpm.processFpmMessage(reinterpret_cast<fpm_msg_hdr_t*>(static_cast<void*>(fpmMsgBuffer)));
67+
}
68+

0 commit comments

Comments
 (0)