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

[fpmsyncd] Fpmsyncd Next Hop Table Enhancement #16762

Merged
merged 7 commits into from
Sep 17, 2024

Conversation

ntt-omw
Copy link
Contributor

@ntt-omw ntt-omw commented Oct 2, 2023

Why I did it

Implementing code changes for sonic-net/SONiC#1425

How I did it

Added RTA_NH_ID attribute patch to libnl3 to add nexthop group feature to fpmsyncd.

How to verify it

enable/disable nexthop group feature
  • Klish will call REST API to configure feature next-hop-group enable.
  • FEATURE|nexthop_group will be created in CONFIG_DB
    • template zebra.conf.j2 will generate zebra.conf with fpm use-next-hop-groups if FEATURE|nexthop_group exists in CONFIG_DB. Else, it will generate zebra.conf with no fpm use-next-hop-groups (default behavior)
  • Do config save comman and write to /etc/sonic/config_db.json
  • restart SONiC: virsh reboot sonic-nhg
  • /etc/frr/zebra.conf has fpm use-next-hop-groups instead of no fpm use-next-hop-groups
Klish CLI for feature nexthop_group
  • Enable: sonic(config)# feature next-hop-group enable
  • Disable: sonic(config)# no feature next-hop-group

Enable

admin@sonic:~$ sonic-cli

sonic# configure terminal

sonic(config)#
  end        Exit to EXEC mode
  exit       Exit from current mode
  feature    Configure additional feature
  interface  Select an interface
  ip         Global IP configuration subcommands
  mclag      domain
  no         To delete / disable commands in config mode

sonic(config)# feature
  next-hop-group  Next-hop Groups feature

sonic(config)# feature next-hop-group
  enable  Enable Next-hop Groups feature

sonic(config)# feature next-hop-group enable
  <cr>

Disable

sonic(config)# no
  feature  Disable additional feature
  ip       Global IP configuration subcommands
  mclag    domain

sonic(config)# no feature
  next-hop-group  Disable Next-hop Groups feature

sonic(config)# no feature next-hop-group
  <cr>

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@ntt-omw ntt-omw requested a review from lguohan as a code owner October 2, 2023 10:46
@ntt-omw ntt-omw changed the title nexthop group support [fpmsyncd]nexthop group support Oct 2, 2023
@ntt-omw ntt-omw changed the title [fpmsyncd]nexthop group support [fpmsyncd] Fpmsyncd Next Hop Table Enhancement Oct 2, 2023
@@ -7,8 +7,13 @@
{% endblock banner %}
!
{% block fpm %}
{% if ( ('nexthop_group' in FEATURE) and ('state' in FEATURE['nexthop_group']) and
(FEATURE['nexthop_group']['state'] == 'enabled') ) %}
fpm use-next-hop-groups
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@lguohan Moved under device metadata following the example.

@nakano-omw
Copy link

/AzurePipelines run

Copy link

Commenter does not have sufficient privileges for PR 16762 in repo sonic-net/sonic-buildimage

@nakano-omw
Copy link

/AzurePipelines run Azure.sonic-buildimage

Copy link

Commenter does not have sufficient privileges for PR 16762 in repo sonic-net/sonic-buildimage

@nakano-omw
Copy link

@lguohan
Please help in restarting the /AzurePipelines run Azure.sonic-buildimage. Would you be able to grant me the privilege?

@ntt-omw
Copy link
Contributor Author

ntt-omw commented Nov 16, 2023

/AzurePipelines run Azure.sonic-buildimage

Copy link

Commenter does not have sufficient privileges for PR 16762 in repo sonic-net/sonic-buildimage

@nakano-omw
Copy link

/azpw run Azure.sonic-buildimage

@ridahanif96
Copy link
Contributor

@nakano-omw you need to repush your code.

@nakano-omw
Copy link

nakano-omw commented Dec 15, 2023

@ridahanif96 Thanks for the comment. Fix conflicts and repush.

@nakano-omw
Copy link

/azpw run Azure.sonic-buildimage

@nakano-omw
Copy link

@ridahanif96 @zhangyanzhao It has been reviewed in Routing WG. Could we approve and merge the PR ?

@zhangyanzhao
Copy link
Collaborator

@lguohan can you please merge this PR if you have no more comments? Thanks.

@nakano-omw
Copy link

@lguohan can you please merge this PR. Thanks.

@nakano-omw nakano-omw force-pushed the ntt_fpmsyncd_enhanced branch from f1ed3fc to 07cd4ff Compare May 14, 2024 04:55
Copy link

linux-foundation-easycla bot commented May 14, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@nakano-omw nakano-omw force-pushed the ntt_fpmsyncd_enhanced branch 2 times, most recently from cd0a5de to 035a3c6 Compare May 14, 2024 06:10
Signed-off-by: nakano.kanji <[email protected]>
@nakano-omw nakano-omw force-pushed the ntt_fpmsyncd_enhanced branch from 7aaece3 to cef3946 Compare May 14, 2024 06:12
Signed-off-by: nakano.kanji <[email protected]>
@dgsudharsan
Copy link
Collaborator

@ntt-omw Can you please sync to the latest. We re enabled nexthop kernel support #18953

@@ -254,6 +254,16 @@ module sonic-device_metadata {
type inet:ipv4-address;
description "BGP Router identifier";
}

leaf nexthop_group {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For yang, you need to add unit tests as well update sample_config_db, add update configuration schema. Please refer to this for example #14045

Choose a reason for hiding this comment

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

I added the sample_config_db. Thanks.

@dgsudharsan dgsudharsan added the YANG YANG model related changes label May 23, 2024
Signed-off-by: nakano.kanji <[email protected]>
@eddieruan-alibaba
Copy link
Collaborator

Cherry picked into phoenixwing folk.http://phoenixwing.com.cn/codes

@kperumalbfn
Copy link
Contributor

@ntt-omw please update the branch and resolve the conflicts.

@nakano-omw
Copy link

@kperumalbfn @eddieruan-alibaba @qiluo-msft Branch updated and conflicts resolved. Please approve and merge.

@eddieruan-alibaba
Copy link
Collaborator

Thanks @nakano-omw I have reviewed it again. My approval is still there.

@nakano-omw
Copy link

@qiluo-msft It is marked as needing review by qiluo-msft. This PR has been reviewed by the Routing WG. Please approve.

(DEVICE_METADATA['localhost']['nexthop_group'] == 'enabled') ) %}
! enable next hop group support
fpm use-next-hop-groups
{% else %}
Copy link
Contributor

Choose a reason for hiding this comment

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

@ebiken Do we have sonic-mgmt tests for FPM nexthop_group? If not, could you please add it.

Choose a reason for hiding this comment

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

Thanks for the review. But I'm away from this project now. Hi @nakano-omw , could you please follow up with @kperumalbfn ? Thanks.

Choose a reason for hiding this comment

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

@kperumalbfn Thanks for the review. The sonic-mgmt tests are being created in the Phoenixwing .
sonic-net/sonic-mgmt#13785

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @nakano-omw Will review sonic-mgmt tests as well.

@kperumalbfn kperumalbfn merged commit 2c47e35 into sonic-net:master Sep 17, 2024
23 checks passed
aidan-gallagher pushed a commit to aidan-gallagher/sonic-buildimage that referenced this pull request Nov 16, 2024
[fpmsyncd] Fpmsyncd Next Hop Table Enhancement (sonic-net#16762)

Signed-off-by: nakano.kanji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants