[SRv6] Add support for SRv6 VPN #3293
Conversation
|
/azp run Azure.sonic-swss |
|
Azure Pipelines successfully started running 1 pipeline(s). |
shuaishang
force-pushed
the
project-phoenixwing-srv6-vpn
branch
from
87f3488 to
f24fa65
Compare
shuaishang
force-pushed
the
project-phoenixwing-srv6-vpn
branch
from
f24fa65 to
80b439d
Compare
|
/azp run Azure.sonic-swss |
|
Commenter does not have sufficient privileges for PR 3293 in repo sonic-net/sonic-swss |
|
/azp run Azure.sonic-swss |
|
Commenter does not have sufficient privileges for PR 3293 in repo sonic-net/sonic-swss |
shuaishang
force-pushed
the
project-phoenixwing-srv6-vpn
branch
from
afc1311 to
290102b
Compare
|
How is this related to #3123? Is there any dependency which one to go first? |
|
@prsunny @kperumalbfn @lguohan Can you help to review and close this PR? |
shuaishang
force-pushed
the
project-phoenixwing-srv6-vpn
branch
from
15400fb to
1d02012
Compare
|
@shuaishang could you rebase with the latest master branch and check all VS test failures. |
| } | ||
| else | ||
| { | ||
| srv6_nh = k.is_srv6_nexthop(); |
There was a problem hiding this comment.
should we include VPN SID for the mismatch? I believe there shouldn't be any mix of NHs.
There was a problem hiding this comment.
There is no "vpn_sid" field in "APP_NEXTHOP_GROUP_TABLE_NAME" for nhgorch.
So don't need to check mismatch for VPN SID.
shuaishang
force-pushed
the
project-phoenixwing-srv6-vpn
branch
4 times, most recently
from
8c5b903 to
61fdcef
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -115,7 +117,8 @@ struct NextHopKey | |||
| { | |||
| if (srv6_nh) | |||
| { | |||
| return ip_address.to_string() + NH_DELIMITER + srv6_segment + NH_DELIMITER + srv6_source; | |||
| return ip_address.to_string() + NH_DELIMITER + srv6_segment + NH_DELIMITER + srv6_source + NH_DELIMITER | |||
| + srv6_vpn_sid + NH_DELIMITER; | |||
There was a problem hiding this comment.
Thanks @goomadao. But the key would be ending with the delimiter, is that ok?
| else if (m_syncdNextHopGroups[it_nhg.first].ref_count == 0) | ||
| { | ||
| removeNextHopGroup(it_nhg.first); | ||
| } | ||
| } | ||
|
|
||
| /* Reduce reference for srv6 next hop group */ |
There was a problem hiding this comment.
Could you provide the reason to decouple 'removeSrv6Nexthops' from 'removeNextHoproup'? How about handling NH removals in addRoutePost?
There was a problem hiding this comment.
Because we need to remove srv6 related objects such as tunnels [maps [entries]], aggregate ids & references for sidlists besides the next hop [group] objects. NH removals are done in deleteSrv6Nexthop called by removeSrv6Nexthops.
|
|
||
| /* Reduce reference for srv6 next hop group */ | ||
| /* Later delete for increase refcnt early */ | ||
| if (!m_bulkSrv6NhgReducedVec.empty()) |
There was a problem hiding this comment.
Also for VPN NHs, should we also check context_index refcount to be zero?
There was a problem hiding this comment.
Context_index is always empty in this situation because vpn_sids(nexthopgroups) is owned by RouteOrch, not NhgOrch & Srv6Orch.
While deleting pic_context_id in doTaskPicContextTable, zero check will be done for the refcount.
There was a problem hiding this comment.
@kperumalbfn can you review @goomadao 's response.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@abdosi please review and approve. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
What I did
[HLD] SRv6 VPN HLD
Why I did it
This PR is to support SRv6 VPN functions
How I verified it
Details if related
Depends on sonic-swss-common#919