[fpmsyncd]: Linux netlink runtime option #1765
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abdosi
reviewed
Jun 3, 2021
fpmsyncd/routesync.cpp
Outdated
| @@ -581,7 +581,8 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) | |||
| char master_name[IFNAMSIZ] = {0}; | |||
|
|
|||
| /* if the table_id is not set in the route obj then route is for default vrf. */ | |||
| if (master_index) | |||
| if (master_index && | |||
| (master_index != RT_TABLE_MAIN) && (master_index != RT_TABLE_DEFAULT)) | |||
There was a problem hiding this comment.
what is trigger of this change ?
There was a problem hiding this comment.
Main difference between default fpmsyncd and "fpmsyncd -l net" behavior is related to VRF support. SONiC modifies actual netlink message from FRR to send VRF ifindex value in RTA_RT_TABLE_ID attribute, while Linux kernel netlink message is unmodified and contains the route table ID in RTA_RT_TABLE_ID attribute. This prevents "-l net" option being used in scenarios that test routes with VRF.
There was a problem hiding this comment.
@abdosi I can change this logic to just check a boolean to see if Linux Netlink connection is used and skip VRF if it is.
qbdwlr
force-pushed
the
netlink
branch
3 times, most recently
from
f401c16
to
b1f6fe8
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
qbdwlr
force-pushed
the
netlink
branch
3 times, most recently
from
6a0c877
to
28ae53e
Compare
EdenGri
pushed a commit
to EdenGri/sonic-swss
that referenced
this pull request
Feb 28, 2022
…le (sonic-net#1765) #### What I did This PR fixed issue for [show interface breakout](sonic-net/sonic-buildimage#7957) issue **Before this fix** removed `Ethernet60` from breakout_cfg table for unit testing and got the issue ``` admin@lnos-x1-a-asw04:~$ sudo redis-cli -n 4 del "BREAKOUT_CFG|Ethernet60" (integer) 1 admin@lnos-x1-a-asw04:~$ sudo redis-cli -n 4 hgetall "BREAKOUT_CFG|Ethernet60" (empty array) admin@lnos-x1-a-asw04:~$ show int breakout Traceback (most recent call last): File "/usr/local/bin/show", line 8, in <module> sys.exit(cli()) File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 764, in __call__ return self.main(*args, **kwargs) File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 717, in main rv = self.invoke(ctx) File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1137, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1137, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1114, in invoke return Command.invoke(self, ctx) File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 956, in invoke return ctx.invoke(self.callback, **ctx.params) File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 555, in invoke return callback(*args, **kwargs) File "/usr/local/lib/python3.7/dist-packages/click/decorators.py", line 17, in new_func return f(get_current_context(), *args, **kwargs) File "/usr/local/lib/python3.7/dist-packages/show/interfaces/__init__.py", line 181, in breakout cur_brkout_mode = cur_brkout_tbl[port_name]["brkout_mode"] KeyError: 'Ethernet60' ``` **After the fix** ``` admin@lnos-x1-a-asw04:~$ show int breakout current-mode Ethernet60 +-------------+-------------------------+ | Interface | Current Breakout Mode | +=============+=========================+ | Ethernet60 | Not Available | +-------------+-------------------------+ ``` #### How I did it Gracefully handle the error when port/interface is not present in `BREAKOUT_CFG` table. #### How to verify it ``` show interface breakout show int interface current-mode ``` **_Added unit test case for negetive Test case_**
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I did
Added runtime option for fpmsyncd "-l net" to allow fpmsyncd to listen directly to netlink messages from Linux kernel rather than listening to FRR FPM TCP socket.
Why I did it
FRR currently does not transmit any MPLS related information on its FPM TCP socket to fpmsyncd, not even MPLS attributes on IP routes. MPLS functionality verification had to be using alternate routing stacks (Juniper cRPD and MetaSwitch) but these routing stacks are not open-source and are not available to the community in general.
Some unit-testing during MPLS development was performed by switching fpmsyncd to listen directly to Linux kernel messages, so it was proposed to upstream this capability for general use.
How I verified it
Unit-tests in sonic-swss/tests test_route.py and test_mpls.py were run with "fpmsyncd -l net".
Details if related
Main difference between default fpmsyncd and "fpmsyncd -l net" behavior is related to VRF support. SONiC modifies actual netlink message from FRR to send VRF ifindex value in RTA_RT_TABLE_ID attribute, while Linux kernel netlink message is unmodified and contains the route table ID in RTA_RT_TABLE_ID attribute. This prevents "-l net" option being used in scenarios that test routes with VRF.
Refer to HLD: sonic-net/SONiC#706