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.

Sign up for GitHub

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

Jump to bottom

FRR patches to support IPv6 Link local enhancements. #5584

Merged
merged 5 commits into from
Jul 20, 2021
Merged

FRR patches to support IPv6 Link local enhancements. #5584

merged 5 commits into from
Jul 20, 2021

Conversation

AkhileshSamineni
Copy link
Contributor

@AkhileshSamineni AkhileshSamineni commented Oct 10, 2020 •
edited
Loading

As per HLD - sonic-net/SONiC#625

FRR Patches:

0009-Link-local-scope-was-not-set-while-binding-socket-for-bgp-ipv6-link-local-neighbors.patch
Files modified : bgpd_network.c and bgpd/bgp_zebra.c
Fix for : Link local scope was not set while binding socket with local address causing socket errors for bgp ipv6 link local neighbors.

0010-VRF-interface-lookup-was-still-done-in-the-default-vrf.patch
Files modified : staticd/static_zebra.c
Fix for : VRF interface lookup was still done in the default-vrf which was causing the interface lookup to fail. Due to this static-route pointing to link-local was not getting installed.

0011-Changes-to-send-ipv6-link-local-address-as-nexthop-to-fpmsyncd.patch
Files modified : zebra/zebra_fpm_netlink.c
Fix for : Made changes to send ipv6 address as nexthop to fpmsyncd.

Depends on:
sonic-net/sonic-utilities#1159
sonic-net/sonic-swss#1463

Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com

This was referenced Oct 10, 2020
Merged
Merged
@AkhileshSamineni
Copy link
Contributor Author

retest this please

@AkhileshSamineni
Copy link
Contributor Author

retest vsimage please.

@AkhileshSamineni
Copy link
Contributor Author

Retest this please.

@pavel-shirshov
Copy link
Contributor

Hi @AkhileshSamineni

Can you please make a PR with your changes to https://github.com/FRRouting/frr ?
As soon as your change is reviewed there, we can backport your change to sonic.

Thanks

@AkhileshSamineni
Copy link
Contributor Author

Hi @AkhileshSamineni

Can you please make a PR with your changes to https://github.com/FRRouting/frr ?
As soon as your change is reviewed there, we can backport your change to sonic.

Thanks

Sure @pavel-shirshov , Could you please tell me on which branch need to raise the PR, is it master or frr 7.4 branch ?

@pavel-shirshov
Copy link
Contributor

@AkhileshSamineni master please

@AkhileshSamineni
Copy link
Contributor Author

retest vsimage please.

prsunny
prsunny reviewed Nov 17, 2020
View reviewed changes
files/image_config/sysctl/sysctl-net.conf Outdated
@@ -32,6 +32,10 @@ net.ipv6.conf.eth0.accept_dad=0
net.ipv6.conf.default.keep_addr_on_down=1
net.ipv6.conf.all.keep_addr_on_down=1
net.ipv6.conf.eth0.keep_addr_on_down=1
net.ipv6.conf.default.disable_ipv6=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the review comment, have we agreed to enable IPv6 by default and disable based on some profile?

sonic-net/SONiC#625 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per design change, enabled the ipv6 by default.

@pavel-shirshov
Copy link
Contributor

Has the patches pushed into FRR master as PRs?

@AkhileshSamineni
Copy link
Contributor Author

AkhileshSamineni commented Nov 18, 2020 •
edited
Loading

Has the patches pushed into FRR master as PRs?

@pavel-shirshov Here are the details :

0009-Link-local-scope-was-not-set-while-binding-socket-for-bgp-ipv6-link-local-neighbors.patch => PR FRRouting/frr#7434

0010-VRF-interface-lookup-was-still-done-in-the-default-vrf.patch => This fix is not needed in master branch as all interface up/down/add/delete events are replaced with ifp callbacks using PR FRRouting/frr#5009, and it is merged.

0011-Changes-to-send-ipv6-link-local-address-as-nexthop-to-fpmsyncd.patch => This fix is not needed in master as already relevant below PRs are raised and merged.
BGP : Fix for nexthop as IPv4 mapped IPv6 address : FRRouting/frr#6821
Clean up the zebra's Netlink API : FRRouting/frr#6569

@AkhileshSamineni
Copy link
Contributor Author

Retest this please.

@AkhileshSamineni AkhileshSamineni changed the title Setting disable ipv6 defaults for interfaces and few FRR changes to support IPv6 Link local enchancements. FRR patches to support IPv6 Link local enhancements. Apr 5, 2021
arlakshm
arlakshm reviewed Apr 6, 2021
View reviewed changes
src/sonic-frr/patch/0011-Changes-to-send-ipv6-link-local-address-as-nexthop-to-fpmsyncd.patch Outdated
@@ -0,0 +1,50 @@
From 6a74cc3b97f405473c7500d5cd0adadba854067f Mon Sep 17 00:00:00 2001
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need still need this patch file, looks like we are not using it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As FRR updated to 7.5 version, not needed this patch file at all.

src/sonic-frr/patch/0010-VRF-interface-lookup-was-still-done-in-the-default-vrf.patch Outdated Show resolved Hide resolved
src/sonic-frr/patch/series Outdated
@@ -5,3 +5,6 @@
0005-nexthops-compare-vrf-only-if-ip-type.patch
0007-frr-remove-frr-log-outchannel-to-var-log-frr.log.patch
0008-Add-support-of-bgp-l3vni-evpn.patch
0009-Link-local-scope-was-not-set-while-binding-socket-for-bgp-ipv6-link-local-neighbors.patch
#0010-VRF-interface-lookup-was-still-done-in-the-default-vrf.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove them instead of commenting these lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@AkhileshSamineni
Copy link
Contributor Author

/Azurepipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 5584 in repo Azure/sonic-buildimage

@AkhileshSamineni
Copy link
Contributor Author

@arlakshm Please retest it, as I don't have permission for Azurepipelines.

@prsunny
Copy link
Contributor

prsunny commented Apr 8, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lguohan
Copy link
Collaborator

lguohan commented Apr 14, 2021

is this patch being accepted by upstream?

0009-Link-local-scope-was-not-set-while-binding-socket-for-bgp-ipv6-link-local-neighbors.patch

@AkhileshSamineni
Copy link
Contributor Author

is this patch being accepted by upstream?

0009-Link-local-scope-was-not-set-while-binding-socket-for-bgp-ipv6-link-local-neighbors.patch

Yes, it is, merged to FRR community.
FRRouting/frr#7434

@AkhileshSamineni
Setting disable ipv6 defaults for interfaces and few FRR changes to s…
934aa0c 
…upport IPv6 Link local enchancements

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
@AkhileshSamineni
Splitted the single path file to 3 patch files for better tracking.
7e29215 
Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
@AkhileshSamineni
Corrected the changes as per design change.
93f9ebd
@AkhileshSamineni
Modified the sonic-frr series file.
d9f6539
@adyeung
Copy link
Collaborator

adyeung commented Jun 24, 2021

@lguohan Pls help merge this patch, nothing else pending on this Code PR

@anshuv-mfst
Copy link

@arlakshm - can you please help with merge

@rlhui rlhui merged commit a31ee03 into sonic-net:master Jul 20, 2021
judyjoseph pushed a commit that referenced this pull request Aug 7, 2021
@AkhileshSamineni @judyjoseph
FRR patches to support IPv6 Link local enhancements. (#5584)
323483c 
As per HLD - sonic-net/SONiC#625

FRR Patches:

0009-Link-local-scope-was-not-set-while-binding-socket-for-bgp-ipv6-link-local-neighbors.patch
Files modified : bgpd_network.c and bgpd/bgp_zebra.c
Fix for : Link local scope was not set while binding socket with local address causing socket errors for bgp ipv6 link local neighbors.

0010-VRF-interface-lookup-was-still-done-in-the-default-vrf.patch
Files modified : staticd/static_zebra.c
Fix for : VRF interface lookup was still done in the default-vrf which was causing the interface lookup to fail. Due to this static-route pointing to link-local was not getting installed.

0011-Changes-to-send-ipv6-link-local-address-as-nexthop-to-fpmsyncd.patch
Files modified : zebra/zebra_fpm_netlink.c
Fix for : Made changes to send ipv6 address as nexthop to fpmsyncd.

Depends on:
sonic-net/sonic-utilities#1159
sonic-net/sonic-swss#1463

Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
@AkhileshSamineni
FRR patches to support IPv6 Link local enhancements. (sonic-net#5584)
873d5fb 
As per HLD - sonic-net/SONiC#625

FRR Patches:

0009-Link-local-scope-was-not-set-while-binding-socket-for-bgp-ipv6-link-local-neighbors.patch
Files modified : bgpd_network.c and bgpd/bgp_zebra.c
Fix for : Link local scope was not set while binding socket with local address causing socket errors for bgp ipv6 link local neighbors.

0010-VRF-interface-lookup-was-still-done-in-the-default-vrf.patch
Files modified : staticd/static_zebra.c
Fix for : VRF interface lookup was still done in the default-vrf which was causing the interface lookup to fail. Due to this static-route pointing to link-local was not getting installed.

0011-Changes-to-send-ipv6-link-local-address-as-nexthop-to-fpmsyncd.patch
Files modified : zebra/zebra_fpm_netlink.c
Fix for : Made changes to send ipv6 address as nexthop to fpmsyncd.

Depends on:
sonic-net/sonic-utilities#1159
sonic-net/sonic-swss#1463

Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prsunny prsunny prsunny left review comments

@lguohan lguohan lguohan approved these changes

@arlakshm arlakshm arlakshm approved these changes

@pavel-shirshov pavel-shirshov Awaiting requested review from pavel-shirshov

Assignees
No one assigned
Labels
Included in 202106 Branch Request for 202106 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@AkhileshSamineni @pavel-shirshov @prsunny @lguohan @adyeung @anshuv-mfst @arlakshm @rlhui @judyjoseph