-
Notifications
You must be signed in to change notification settings - Fork 562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fpmsyncd]: Add support for SRv6 #3123
Conversation
962e839
to
13cfd44
Compare
13cfd44
to
525d2e2
Compare
525d2e2
to
98225dc
Compare
/azpw run Azure.sonic-swss |
/AzurePipelines run Azure.sonic-swss |
Azure Pipelines successfully started running 1 pipeline(s). |
faf75e9
to
171fbfb
Compare
@cscarpitta can you rebase it to fix the compile issue? sonic-net/sonic-buildimage#18715 is merged. |
This change has been cherry-picked into phoenix wing and used in daily run. |
fpmsyncd/routesync.cpp
Outdated
#define NH_ENCAP_SRV6_ROUTE 101 | ||
|
||
#define RTM_NEWSRV6LOCALSID 1000 | ||
#define RTM_DELSRV6LOCALSID 1001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These constants are defined in fpmlink.h, do we need it here? Pls remove if not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed these constants are not needed here. I removed them.
fpmsyncd/routesync.cpp
Outdated
@@ -81,6 +145,8 @@ RouteSync::RouteSync(RedisPipeline *pipeline) : | |||
m_vnet_routeTable(pipeline, APP_VNET_RT_TABLE_NAME, true), | |||
m_vnet_tunnelTable(pipeline, APP_VNET_RT_TUNNEL_TABLE_NAME, true), | |||
m_warmStartHelper(pipeline, &m_routeTable, APP_ROUTE_TABLE_NAME, "bgp", "bgp"), | |||
m_srv6LocalSidTable(pipeline, APP_SRV6_MY_SID_TABLE_NAME, true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: could we change the table name to mysidTable. I think we changed most of the reference of localSid to MySid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. I changed localSid -> mySid everywhere in the code.
|
||
if (vpn_sid.empty()) | ||
{ | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error logs for debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an error message:
SWSS_LOG_ERROR("Received an invalid SRv6 route: vpn_sid is empty");
Thanks.
a1671dc
to
b0c2f60
Compare
@cscarpitta |
b0c2f60
to
7954e2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes are needed for programming SRv6 from FRR.
ed29566
to
9df0769
Compare
07405df
to
6f04b16
Compare
dvs.runcmd("vtysh -c \"configure terminal\" -c \"segment-routing\" -c \"srv6\" -c \"locators\" -c \"locator loc1\" -c \"prefix fc00:0:2::/48 block-len 32 node-len 16 func-bits 16\" -c \"behavior usid\"") | ||
|
||
# create srv6 mysid udt4 behavior | ||
dvs.runcmd("ip -6 route add fc00:0:2:ff05::/128 encap seg6local action End.DT4 vrftable {} dev sr0".format(self.vrf_table_id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cscarpitta To support TUNNEL DSCP/TTL mode MY_SID object has tunnel_id. Currently, there is no orchagent support for this. But how do we add this in FRR/Fpmsyncd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kperumalbfn To support the DSCP/TTL, we can extend FRR to provide these parameters to fpmsyncd and extend orchagent to process them
6f04b16
to
2192b36
Compare
SRV6_LOCALSID_ACTION_END = 1, | ||
SRV6_LOCALSID_ACTION_END_X = 2, | ||
SRV6_LOCALSID_ACTION_END_T = 3, | ||
SRV6_LOCALSID_ACTION_END_DX2 = 4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current SAI doesn't support L2 XConnect, does FRR support this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, currently FRR does not support L2 XConnect.
fpmsyncd/routesync.cpp
Outdated
if (!parseSrv6MySidFormat(tb[SRV6_LOCALSID_FORMAT], block_len, | ||
node_len, func_len, arg_len)) | ||
{ | ||
SWSS_LOG_ERROR("Invalid Srv6 MySid format"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add details about node/block/function in the error log message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added. Thanks for the comment.
strlen((char *)RTA_DATA(tb[SRV6_LOCALSID_VRFNAME]))); | ||
} | ||
|
||
action = mySidAction2Str(action_buf); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unknown action, return false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks for the comment.
d624557
to
d9f662c
Compare
@cscarpitta can you rebase your branch ? "This branch is out-of-date with the base branch" |
d9f662c
to
9928298
Compare
Done. |
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cscarpitta please include sonic-mgmt test PR(sonic-net/sonic-mgmt#15510) in the description.
Changes LGTM |
@kperumalbfn Thanks for the review. I updated the PR description. |
* Extend fpmsyncd to process SRv6 routes and local SIDs received from FRR * Add test cases to verify SRv6 functionality Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
9928298
to
853df15
Compare
* [fpmsyncd]: Add support for SRv6 What I did Extended fpmsyncd to process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes). Extend fpmsyncd to process SRv6 routes and local SIDs received from FRR Add test cases to verify SRv6 functionality Why I did it fpmsyncd did not process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes).
* [fpmsyncd]: Add support for SRv6 What I did Extended fpmsyncd to process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes). Extend fpmsyncd to process SRv6 routes and local SIDs received from FRR Add test cases to verify SRv6 functionality Why I did it fpmsyncd did not process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes).
* [fpmsyncd]: Add support for SRv6 What I did Extended fpmsyncd to process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes). Extend fpmsyncd to process SRv6 routes and local SIDs received from FRR Add test cases to verify SRv6 functionality Why I did it fpmsyncd did not process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes).
* [fpmsyncd]: Add support for SRv6 What I did Extended fpmsyncd to process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes). Extend fpmsyncd to process SRv6 routes and local SIDs received from FRR Add test cases to verify SRv6 functionality Why I did it fpmsyncd did not process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes).
* [fpmsyncd]: Add support for SRv6 What I did Extended fpmsyncd to process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes). Extend fpmsyncd to process SRv6 routes and local SIDs received from FRR Add test cases to verify SRv6 functionality Why I did it fpmsyncd did not process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes).
HLD: sonic-net/SONiC#1620
What I did
Extended fpmsyncd to process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes).
Why I did it
fpmsyncd did not process Netlink TLVs containing SRv6 information (SRv6 local SIDs and routes).
How I verified it
This PR implements new set of test cases for SRv6
test_srv6.py
The sonic-mgmt tests for SRv6 are added in this PR: sonic-net/sonic-mgmt#15510