Skip to content
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

[FRR Patch] Added patch in FRR to send tag value associated with route via Netlink to fpmsyncd #20692

Merged
merged 6 commits into from
Dec 12, 2024

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Nov 4, 2024

What I did:
Added patch in FRR to send tag value associated with route via NETLINK RTA_PRIORITY field which can be used as attribute/metadata in fpmsyncd for different use-cases.

Why I did:
Some of use cases:

  1. Do not program specific routes into APP_DB
  2. Fallback to default route for specific routes.

How I verify:
Manual Verification.

via NETLINK RTA_PRIORITY field which can be used as attribute/metadata
in fpmsyncd for different use-cases.

Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi abdosi requested a review from lguohan as a code owner November 4, 2024 19:08
@abdosi abdosi requested review from arlakshm and prsunny November 4, 2024 19:09
Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@prsunny prsunny requested a review from dgsudharsan November 4, 2024 21:07
@prsunny
Copy link
Contributor

prsunny commented Nov 4, 2024

@StormLiangMS for viz

ri->rtm_type = RTN_UNICAST;
- ri->metric = &re->metric;
+ // Patch to send tag value via NETLINK Priority field (RTA_PRIORITY). The Tag vale can be used as metadata/attribute in fpmsyncd for further processing
+ ri->metric = &re->tag;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given now we migrated to dplane_fpm_sonic, does it require any changes over there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgsudharsan thanks. i have revised the patch. Change is now inside API netlink_route_multipath_msg_encode which is called by both dplane_fpm_sonic.c and dplane_fpm_nl.c


ri->rtm_protocol = netlink_proto_from_route_type(re->type);
ri->rtm_type = RTN_UNICAST;
- ri->metric = &re->metric;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are overriding the current meaning of metric by default. Would it impact any existing usecases/ scenarios?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgsudharsan not applicable as change has been revised. please review again.

…K RTA_PRIORITY field which can be used as attribute/metadata in fpmsyncd for different use-cases.

Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi
Copy link
Contributor Author

abdosi commented Nov 12, 2024

@dgsudharsan : can you review again

Copy link
Collaborator

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abdosi I request you to wait for FRR 10.0.1 merge and rebase this PR based on it. @hasan-brcm FYI

…a NETLINK RTA_PRIORITY field which can be used as attribute/metadata in fpmsyncd for different use-cases."

This reverts commit 2776138.
@abdosi
Copy link
Contributor Author

abdosi commented Dec 11, 2024

@abdosi I request you to wait for FRR 10.0.1 merge and rebase this PR based on it. @hasan-brcm FYI

@dgsudharsan : Please approve again.

@abdosi
Copy link
Contributor Author

abdosi commented Dec 11, 2024

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rlhui rlhui merged commit 5b6ff85 into sonic-net:master Dec 12, 2024
22 checks passed
@abdosi abdosi deleted the frr-patch branch December 12, 2024 02:03
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to msft-202412: Azure/sonic-buildimage-msft#496

VladimirKuk pushed a commit to Marvell-switching/sonic-buildimage that referenced this pull request Jan 21, 2025
…e via Netlink to fpmsyncd (sonic-net#20692)

* Added patch in FRR to send tag value associated with route
via NETLINK RTA_PRIORITY field which can be used as attribute/metadata
in fpmsyncd for different use-cases.

---------

Signed-off-by: Abhishek Dosi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants