[fpmsyncd] updates for MPLS by qbdwlr · Pull Request #1794 · sonic-net/sonic-swss

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

    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

 - 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

Conversation

qbdwlr commented Jun 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rlhui commented Jul 13, 2021

Uh oh!

azure-pipelines bot commented Jul 13, 2021

Uh oh!

smaheshm Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smaheshm Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rlhui commented Aug 23, 2021

Uh oh!

smaheshm commented Aug 23, 2021

Uh oh!

rlhui commented Aug 26, 2021

Uh oh!

smaheshm commented Aug 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

qbdwlr commented Jun 18, 2021 •

edited

Loading

smaheshm Jul 15, 2021 •

edited

Loading

smaheshm Jul 15, 2021 •

edited

Loading