[fpmsyncd] Add support for SRv6 by cscarpitta · Pull Request #2515 · sonic-net/sonic-swss

cscarpitta · 2022-11-12T22:27:38Z

The dplane_fpm_nl module in FRR allows SONiC to learn the forwarding information computed by FRR. FRR already supports many SRv6 functionalities. There is an open PR (FRRouting/frr/pull/12301) to extend the dplane_fpm_nl module of FRR to export the required SRv6 information to SONiC.

OrchAgent (specifically, RouteOrch and SRv6Orch) in SONiC SWSS can already extract the SRv6 information from the APP_DB and push it towards the ASIC_DB (sonic-net/sonic-swss/pull/1964).

However, the fpmsyncd in SONiC SWSS does not process the SRv6 information received from FRR. This PR extends fpmsyncd to parse the SRv6 information received from FRR and push it toward the APP_DB.

What I did

I added support for SRv6 to fpmsyncd:

I added a handler onSrv6RouteMsg() which is able to parse Netlink messages containing an SRv6 Steering Route and create an entry in the ROUTE_TABLE of APP_DB.
I added a handler onSrv6LocalSidMsg() which is able to parse Netlink messages containing an SRv6 Local SID and create an entry in the SRV6_MY_SID_TABLE of APP_DB.
I changed onMsgRaw() to pass the Netlink messages to the appropriate handler depending on the encapsulation type.
I added the unit tests to verify that fpmsyncd processes the Netlink messages and updates the APP_DB correctly.

Why I did it

FRR supports many SRv6 functionalities. There is an open PR (FRRouting/frr/pull/12301) to extend the dplane_fpm_nl module of FRR to export the required SRv6 information to SONiC.

However, the fpmsyncd in SONiC SWSS did not process the SRv6 information received from FRR.

How I verified it

I added new unit tests in tests/mock_tests/fpmsyncd/receive_srv6_localsids_ut.cpp and tests/mock_tests/fpmsyncd/receive_srv6_steer_routes_ut.cpp.

Details if related
SRv6 HLD: https://github.com/sonic-net/SONiC/blob/master/doc/srv6/srv6_hld.md
SRv6 uSID HLD: https://github.com/sonic-net/SONiC/blob/master/doc/srv6/SRv6_uSID.md

linux-foundation-easycla · 2022-11-12T22:27:52Z

The committers listed above are authorized under a signed CLA.

✅ login: cscarpitta / name: Carmine Scarpitta (5ad6686, 5081801, f50814e, f9a7cdf, 3dfa88d, cab3b30)

reshmaintel · 2022-11-21T20:07:24Z

@svshah-intel Hi Shitanshu, could you please help review this PR. Thanks.

svshah-intel · 2022-11-28T01:16:46Z

fpmsyncd/routesync.cpp

+    }
+
+    vector<FieldValueTuple> fvVectorRoute;
+    if (!vpn_sid_str.empty())


this is not required since this check is already performed few lines above

I've removed it, thanks!

svshah-intel · 2022-11-28T01:28:36Z

fpmsyncd/routesync.cpp

+        fvVector.push_back(adj);
+    }
+
+    m_srv6LocalSidTable.set(my_sid_table_key, fvVector);


srv6orch.cpp says
/* Key for mySid : block_len:node_len:function_len:args_len:sid-ip */

here looks like only sid-ip is used as my_sid_table_key?

Exactly! I fixed it.

svshah-intel · 2022-11-28T01:31:09Z

fpmsyncd/routesync.cpp

+            onSrv6LocalSidMsg(h, len);
+            break;
+        default:
+            onEvpnRouteMsg(h, len);


it may be good to have a comment here to mention why default case handles onEvpnRouteMsg. I understand you are mainting functionality as before switch case, but for future reading it may be confusing without supporting comment

I added a comment, thanks for the suggestion!

svshah-intel · 2022-11-28T01:36:40Z

fpmsyncd/routesync.cpp

+
+        /* Duplicated delete as we do not know if it is a seg6 route, seg6local
+         * route or regular route */
+        m_srv6LocalSidTable.del(destipaddress);


don't you need to check ip type to be v6 first?

How about handling warmRestartInProgress check just like is handled for normal route delete?

Same as earlier comments - srv6orch.cpp seems to be expecting more than just ip address as a key value

I moved the delete code to onSrv6LocalSidMsg() and changed to use the correct key value.

* Adding the ability to pass the routes to the appropriate handler depending on the encap type (getEncapType()) * Adding a handler onSrv6RouteMsg() for parsing the routes containing SRv6 steer route nexthops (i.e., routes with encap type NH_ENCAP_SRV6_ROUTE) * Adding the ability to parse SRv6 steer route nexthops (getSrv6SteerRouteNextHop()) * Adding the ability to decode a Netlink message containing an SRv6 steer route nexthop (parseEncapSrv6SteerRoute()) * Adding the ability to include the SRv6 information in the entries pushed to the ROUTE_TABLE Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>

* Adding a handler onSrv6LocalSidRouteMsg() for parsing the routes containing SRv6 Local SID nexthops (i.e., routes with encap type NH_ENCAP_SRV6_LOCAL_SID) * Adding the ability to parse SRv6 Local SID nexthops (getSrv6LocalSidNextHop()) * Adding the ability to decode a Netlink message containing an SRv6 Local SID nexthop (parseEncapSrv6LocalSid()) * Adding the ability to decode a Netlink message containing the format of an SRv6 Local SID (parseEncapSrv6LocalSidFormat()) * Adding the ability to write an SRv6 Local SID entry to APP_DB * Adding the ability to delete SRv6 Local SIDs from APP_DB Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>

* Added an helper function (`create_srv6_steer_route_nlmsg()`) to build a Netlink request message for adding an SRv6 steering route. * Added an helper function (`create_srv6_localsid_nlmsg()`) to build a Netlink request message for adding an SRv6 local SID. * Add several helper functions for adding Netlink attributes to an existing Netlink message. * Add a mock function (`__wrap_rtnl_link_i2name()`) that simulates calls to `rtnl_link_i2name()` to get the name of an interface. Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>

cscarpitta · 2023-05-11T07:16:38Z

@svshah-intel Many thanks for your review! I addressed all your comments. Could you please take another look?

* Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 steer route with an IPv4 destination prefix (`RecevingSRv6SteerRoutesWithIPv4Prefix`). * Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 steer route with an IPv6 destination prefix (`RecevingSRv6SteerRoutesWithIPv6Prefix`). Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>

* Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 Local SID bound to the End.DT4 behavior (`RecevingRouteWithSRv6LocalSIDEndDT4`). * Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 Local SID bound to the End.DT6 behavior (`RecevingRouteWithSRv6LocalSIDEndDT6`). * Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 Local SID bound to the End.DT46 behavior (`RecevingRouteWithSRv6LocalSIDEndDT46`). * Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 Local SID bound to the uDT4 behavior (`RecevingRouteWithSRv6LocalSIDUDT4`). * Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 Local SID bound to the uDT6 behavior (`RecevingRouteWithSRv6LocalSIDUDT6`). * Added a unit test to check that fpmsyncd updates the APP_DB correctly when it receives an SRv6 Local SID bound to the uDT46 behavior (`RecevingRouteWithSRv6LocalSIDUDT46`). Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>

* Ignore the `tests_fpmsyncd` binary Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>

cscarpitta · 2024-10-23T11:58:19Z

This PR has been replaced by #3123. Closing it

cscarpitta requested a review from prsunny as a code owner November 12, 2022 22:27

cscarpitta force-pushed the feature/fpmsyncd-srv6-support branch 2 times, most recently from aed277f to a0920f2 Compare November 14, 2022 10:36

svshah-intel reviewed Nov 28, 2022

View reviewed changes

cscarpitta added 3 commits May 11, 2023 08:45

cscarpitta force-pushed the feature/fpmsyncd-srv6-support branch from a0920f2 to 697a940 Compare May 11, 2023 07:00

cscarpitta requested a review from svshah-intel May 11, 2023 07:17

cscarpitta added 3 commits May 16, 2023 17:37

[gitignore]: Ignore tests_fpmsyncd binary

6513fc5

* Ignore the `tests_fpmsyncd` binary Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>

cscarpitta force-pushed the feature/fpmsyncd-srv6-support branch from 697a940 to 6513fc5 Compare May 16, 2023 15:42

eddieruan-alibaba mentioned this pull request May 26, 2023

SRv6 VPN HLD for 202305 release sonic-net/SONiC#1252

Merged

AlanYoush mentioned this pull request Oct 14, 2024

[fpmsyncd]: Add support for SRv6 #3123

Merged

cscarpitta closed this Oct 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fpmsyncd] Add support for SRv6#2515

[fpmsyncd] Add support for SRv6#2515
cscarpitta wants to merge 6 commits intosonic-net:masterfrom
cscarpitta:feature/fpmsyncd-srv6-support

cscarpitta commented Nov 12, 2022

Uh oh!

linux-foundation-easycla bot commented Nov 12, 2022 •

edited

Loading

Uh oh!

reshmaintel commented Nov 21, 2022

Uh oh!

svshah-intel Nov 28, 2022

Uh oh!

cscarpitta May 11, 2023

Uh oh!

svshah-intel Nov 28, 2022

Uh oh!

cscarpitta May 11, 2023

Uh oh!

svshah-intel Nov 28, 2022

Uh oh!

cscarpitta May 11, 2023

Uh oh!

svshah-intel Nov 28, 2022

Uh oh!

cscarpitta May 11, 2023 •

edited

Loading

Uh oh!

cscarpitta commented May 11, 2023

Uh oh!

cscarpitta commented Oct 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cscarpitta commented Nov 12, 2022

Uh oh!

linux-foundation-easycla bot commented Nov 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reshmaintel commented Nov 21, 2022

Uh oh!

svshah-intel Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

cscarpitta May 11, 2023

Choose a reason for hiding this comment

Uh oh!

svshah-intel Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

cscarpitta May 11, 2023

Choose a reason for hiding this comment

Uh oh!

svshah-intel Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

cscarpitta May 11, 2023

Choose a reason for hiding this comment

Uh oh!

svshah-intel Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

cscarpitta May 11, 2023 • 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!

cscarpitta commented May 11, 2023

Uh oh!

cscarpitta commented Oct 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linux-foundation-easycla bot commented Nov 12, 2022 •

edited

Loading

cscarpitta May 11, 2023 •

edited

Loading