-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[preview]Add PIC support in the srv6 VPN scenario. #16879
base: master
Are you sure you want to change the base?
[preview]Add PIC support in the srv6 VPN scenario. #16879
Conversation
3c64ad3
to
ad254d2
Compare
This PR implements the separation of nh_srv6-related information, which was originally stored in the nexthop, in the srv6-vpn scenario. It generates a new pic nexthop that contains only the next-hop forwarding information. Meanwhile, the original nexthop containing nh_srv6 is used as pic context information and indexed separately. Signed-off-by: hanyu.zly&freddy <[email protected]>
ad254d2
to
669850c
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.
A few comments.
The commit should be broken into multiple commits (bgpd, lib, zebra, ...)
https://docs.frrouting.org/projects/dev-guide/en/latest/workflow.html#commit-guidelines
@@ -2289,6 +2289,9 @@ ssize_t netlink_route_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx | |||
|
|||
/* Table corresponding to this route. */ | |||
table_id = dplane_ctx_get_table(ctx); | |||
if (fpm) | |||
table_id = dplane_ctx_get_vrf(ctx); |
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.
Why are we overriding the table ID with the VRF ID here? Isn't this a breaking change?
@@ -65,6 +65,8 @@ struct mgmt_be_client *mgmt_be_client; | |||
/* Route retain mode flag. */ | |||
int retain_mode = 0; | |||
|
|||
bool fpm_pic_nexthop = 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.
What is this variable used for? It is set to true here and never changed. It seems that it is unnecessary and can be removed?
|
||
ctx->u.rinfo.nhe.id = nhe->id; | ||
ctx->u.rinfo.nhe.old_id = 0; | ||
if (nhe->pic_nhe) { | ||
/**/ |
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.
Please fix the empty comment.
What I did
[HLD] BGP PIC HLD
Why I did it
This PR implements the separation of nh_srv6-related information, which was originally stored in the nexthop, in the srv6-vpn scenario. It generates a new pic nexthop that contains only the next-hop forwarding information. Meanwhile, the original nexthop containing nh_srv6 is used as pic context information and indexed separately.
This is used to achieve fast switching of nexthop group members by identifying the corresponding members in the nexthop group based on route changes, and then performing operations on the members.
This PR is temporarily for review purposes. Documentation and test cases will be added later, and a complete PR will be resubmitted.