[fpmsyncd] updates for MPLS #1794

qbdwlr · 2021-06-18T20:13:25Z

What I did
SONiC swss fpmsyncd support for MPLS:

fpmsyncd support for routes with AF_MPLS and NHs with MPLS attributes.

Why I did it
SONiC swss fpmsyncd support for MPLS

How I verified it
Unit-tests in sonic-swss/tests/test_mpls.py
System tests in sonic-mgmt

Details if related
Build dependency on sonic-net/sonic-buildimage#8064
Refer to HLD: sonic-net/SONiC#706

smaheshm · 2021-07-01T21:30:13Z

fpmsyncd/routesync.cpp

@@ -897,6 +1004,123 @@ bool RouteSync::getIfName(int if_index, char *if_name, size_t name_len)
    return true;
 }

+#ifndef LIBNL3_ENCAP_MPLS


lets remove this and update azure pipeline to fetch libnl3 debian package from sonic-buildimaage

@smaheshm I am not familiar with the azure pipeline infrastructure and will need input with exact changes required to fetch libnl3 debian packages from sonic-buildimage.

@smaheshm I have submitted a new PR sonic-net/sonic-buildimage#8064 that is the only other change required to make this PR work in the Azure pipeline as it is currently.

smaheshm · 2021-07-07T18:44:28Z

.azure-pipelines/build-template.yml

@@ -21,6 +21,9 @@ parameters:
 - name: sonic_slave
  type: string

+- name: buildimage_artifact_name


The value for this variable must be passed from "azure-pipelines.yml", I don't see that change in the PR.

@smaheshm Did you look at the pipeline details for this? Doesn't even get to the point where the .azure-pipelines stuff is needed. lgtm.yml is run first and fails... so this issue you have pointed out is irrelevant for now.

qiluo-msft · 2021-07-12T03:12:34Z

lgtm.yml

+      - sudo dpkg -i libnl-route-3-dev_3.5.0-1_amd64.deb
+      - sudo dpkg -i libnl-nf-3-dev_3.5.0-1_amd64.deb
+      - popd
+      - git clone https://github.com/qbdwlr/sonic-swss-common; pushd sonic-swss-common; git checkout azp; ./autogen.sh; fakeroot dpkg-buildpackage -us -uc -b; popd


Why using a personal repo?

I guess @qbdwlr was trying out if this change will fix the LGTM analysis. Looks like it didn't.

rlhui · 2021-07-13T16:24:18Z

/azp run

azure-pipelines · 2021-07-13T16:24:27Z

Azure Pipelines failed to run 1 pipeline(s).

smaheshm · 2021-07-15T00:27:45Z

fpmsyncd/routesync.cpp

+    /* Get the index of the master device */
+    uint32_t master_index = rtnl_route_get_table(route_obj);
+    /* if the table_id is not set in the route obj then route is for default vrf. */
+    if (master_index)


How did you find out non-zero value implies a VRF table. "254" is the main table that ip route installs, 255 and 253 are local and default route table. All these are non-VRF tables.

@smahesh You do know that SONiC has modified the fpm netlink message content from frr to something non-standard, right? The rtnl_route_get_table() function will not return the RT_TABLE value in SONiC fpmsyncd, it returns the VRF ifindex value instead because SONiC has patched this.
https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-frr/patch/0003-Use-vrf_id-for-vrf-not-tabled_id.patch

thanks for the info, no I wasn't aware.

@smaheshm Yes, this is the reason I have to disable VRF support in the other PR (#1765) where I provide the fpmsyncd runtime option to listen to kernel netlink instead of frr fpm. kernel netlink messages send the standard RT_TABLE ID in the netlink message, whereas frr fpm substitutes the patched VRF ifindex value.

rlhui · 2021-08-23T19:35:44Z

@prsunny - would you please help review? thanks.

smaheshm · 2021-08-23T19:38:34Z

@prsunny - would you please help review? thanks.

@rlhui We will use either #1870 or #1871 for the merge since it includes AZP pipeline changes too.

rlhui · 2021-08-26T05:09:23Z

@qbdwlr - please close this if this PR is not to be used anymore.

smaheshm · 2021-08-26T18:38:44Z

closing in favor of #1871

…nic-net#1794) #### What I did Implemented [JSON Patch Ordering using YANG Models Design Doc](https://github.com/Azure/SONiC/blob/master/doc/config-generic-update-rollback/Json_Patch_Ordering_using_YANG_Models_Design.md) #### How to verify it Unit-Tests **NOTE: The code in this PR was [reverted](github.com/Azure/sonic-utilities/commit/0a145e8027380e8d4decb36bdfc647062c722612) before because of some [build issues](sonic-net/sonic-utilities#1761). Build issues have been fixed [here](sonic-net/sonic-buildimage#8632). To check the original PR comments please go [here](sonic-net/sonic-utilities#1599

qbdwlr mentioned this pull request Jun 18, 2021

MPLS for SONiC infrastructure HLD sonic-net/SONiC#706

Merged

qbdwlr force-pushed the fpmMpls branch from 7e577b6 to af3ff53 Compare June 23, 2021 19:01

qbdwlr requested a review from prsunny as a code owner June 23, 2021 19:01

qbdwlr force-pushed the fpmMpls branch from af3ff53 to a2119bb Compare June 23, 2021 19:55

qbdwlr force-pushed the fpmMpls branch from 6eb3b9c to 5fc487a Compare July 1, 2021 16:54

smaheshm reviewed Jul 1, 2021

View reviewed changes

qbdwlr force-pushed the fpmMpls branch from ffde670 to 4e5089a Compare July 5, 2021 16:45

qbdwlr mentioned this pull request Jul 5, 2021

[fpmsyncd] updates for MPLS sonic-net/sonic-buildimage#8064

Closed

qbdwlr force-pushed the fpmMpls branch 8 times, most recently from 07b2b94 to b643761 Compare July 7, 2021 13:28

smaheshm reviewed Jul 7, 2021

View reviewed changes

qbdwlr force-pushed the fpmMpls branch 5 times, most recently from a6b6583 to 4a2617b Compare July 8, 2021 16:33

qiluo-msft reviewed Jul 12, 2021

View reviewed changes

qbdwlr force-pushed the fpmMpls branch 3 times, most recently from 979d608 to 06d3387 Compare July 12, 2021 20:03

smaheshm reviewed Jul 15, 2021

View reviewed changes

qbdwlr force-pushed the fpmMpls branch from 06d3387 to aa016ea Compare July 20, 2021 20:48

qbdwlr force-pushed the fpmMpls branch 6 times, most recently from c7a1cc2 to c978f74 Compare July 27, 2021 20:58

qbdwlr force-pushed the fpmMpls branch 2 times, most recently from ddf91d7 to 66d7cf2 Compare July 30, 2021 13:18

qbdwlr force-pushed the fpmMpls branch 5 times, most recently from 122ae51 to a626695 Compare August 12, 2021 18:16

[fpmsyncd] updates for MPLS

db9b563

qbdwlr force-pushed the fpmMpls branch from a626695 to db9b563 Compare August 12, 2021 18:46

judyjoseph added the Request for 202106 Branch label Aug 20, 2021

smaheshm closed this Aug 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fpmsyncd] updates for MPLS #1794

[fpmsyncd] updates for MPLS #1794

qbdwlr commented Jun 18, 2021 •

edited

smaheshm Jul 1, 2021

qbdwlr Jul 5, 2021

qbdwlr Jul 5, 2021

smaheshm Jul 7, 2021

qbdwlr Jul 7, 2021

qiluo-msft Jul 12, 2021

smaheshm Jul 12, 2021

rlhui commented Jul 13, 2021

azure-pipelines bot commented Jul 13, 2021

smaheshm Jul 15, 2021 •

edited

qbdwlr Jul 15, 2021

smaheshm Jul 15, 2021 •

edited

qbdwlr Jul 15, 2021

rlhui commented Aug 23, 2021

smaheshm commented Aug 23, 2021

rlhui commented Aug 26, 2021

smaheshm commented Aug 26, 2021

[fpmsyncd] updates for MPLS #1794

[fpmsyncd] updates for MPLS #1794

Conversation

qbdwlr commented Jun 18, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlhui commented Jul 13, 2021

azure-pipelines bot commented Jul 13, 2021

smaheshm Jul 15, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smaheshm Jul 15, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlhui commented Aug 23, 2021

smaheshm commented Aug 23, 2021

rlhui commented Aug 26, 2021

smaheshm commented Aug 26, 2021

qbdwlr commented Jun 18, 2021 •

edited

smaheshm Jul 15, 2021 •

edited

smaheshm Jul 15, 2021 •

edited