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

staticd: Add new command show static ip/ipv6 route #17729

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

guoguojia2021
Copy link
Contributor

@guoguojia2021 guoguojia2021 commented Dec 28, 2024

Add static new command

Hello, this is FRRouting (version 10.3-dev-my-manual-build).
Copyright 1996-2005 Kunihiro Ishiguro, et al.

router2# show static ip route vrf all
Staticd routes:
VRF Vrf1 IPv4 Unicast:
2.2.2.2/32 ip4:192.168.25.100 valid, registered:yes, state:2
ip4:192.168.23.100 valid, registered:yes, state:2
VRF Vrf1 IPv4 Municast:
VRF default IPv4 Unicast:
1.1.1.1/32 ip4:100.24.24.100 valid, registered:yes, state:2
ip4:100.23.23.100 valid, registered:yes, state:2
1.12.12.12/32 ip4:100.24.24.100 valid, registered:yes, state:2
VRF default IPv4 Municast:

router2# show static ip route vrf Vrf1
Staticd routes:
VRF Vrf1 IPv4 Unicast:
2.2.2.2/32 ip4:192.168.25.100 valid, registered:yes, state:2
ip4:192.168.23.100 valid, registered:yes, state:2
VRF Vrf1 IPv4 Municast:

router2# show static ip route
Staticd routes:
VRF default IPv4 Unicast:
1.1.1.1/32 ip4:100.24.24.100 valid, registered:yes, state:2
ip4:100.23.23.100 valid, registered:yes, state:2
1.12.12.12/32 ip4:100.24.24.100 valid, registered:yes, state:2
VRF default IPv4 Municast:

router2# show static ipv6 route
Staticd routes:
VRF default IPv6 Unicast:
1:2:3::4/128 ifname(0):2000:4000:100 invalid, registered:no, state:0
ifname(0):2000:3000:100 invalid, registered:no, state:0
VRF default IPv6 Municast:

router1# show static ip route vrf all json
{
"static route":{
"Vrf1":{
"ipv4-unicast":[
{
"prefix":"2.3.4.5/32",
"vrf":"Vrf1",
"registered":false,
"state":2
},
{
"prefix":"2.3.4.6/32",
"vrf":"Vrf1",
"registered":false,
"state":2
}
],
"ipv4-multicast":[
]
},
"default":{
"ipv4-unicast":[
{
"prefix":"2.2.2.2/32",
"vrf":"default",
"registered":false,
"state":2
},
{
"prefix":"2.2.2.2/32",
"vrf":"default",
"registered":false,
"state":2
}
],
"ipv4-multicast":[
]
}
}
}
router1# show static ip route vrf all
Staticd routes:
VRF Vrf1 IPv4 Unicast:
2.3.4.5/32 ip4:192.168.11.100 valid, registered:yes, state:2
2.3.4.6/32 ip4:192.168.11.200 valid, registered:yes, state:2
VRF Vrf1 IPv4 Municast:
VRF default IPv4 Unicast:
2.2.2.2/32 ip4:100.13.13.200 valid, registered:yes, state:2
ip4:100.13.13.100 valid, registered:yes, state:2
VRF default IPv4 Municast:

router1#

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

Topotest and documentation must be added for new CLI commands.

@riw777 riw777 self-requested a review January 7, 2025 14:42
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

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

code looks okay ... needs topo and docs

@guoguojia2021
Copy link
Contributor Author

code looks okay ... needs topo and docs
@ton31337 @riw777
Thank you! I will add.

@frrbot frrbot bot added documentation tests Topotests, make check, etc labels Feb 10, 2025
@guoguojia2021 guoguojia2021 force-pushed the static_show branch 9 times, most recently from c1dea58 to d236e78 Compare February 10, 2025 10:04
@guoguojia2021
Copy link
Contributor Author

guoguojia2021 commented Feb 11, 2025

Hi, @ton31337
I have added topotest and documentation, please help to review again.
Thank you!

@guoguojia2021 guoguojia2021 force-pushed the static_show branch 2 times, most recently from 2b83dec to 32d2b70 Compare February 12, 2025 06:31
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

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

looks good

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

It's wrong at various level: documentation, JSON. Also, why can't we reuse show ip route static command, but add an additional keyword for these extra details?

@@ -43,6 +43,10 @@ a static prefix and gateway, with several possible forms.

.. clicmd:: ipv6 route NETWORK [from SRCPREFIX] (Null0|blackhole|reject) [DISTANCE] [table TABLENO] [nexthop-vrf VRFNAME] [vrf VRFNAME]

.. clicmd:: show static (ip|ipv6) route [vrf <NAME|all>] [json]
Copy link
Member

Choose a reason for hiding this comment

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

Why do you put this section here? It's for ipv6 route ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both ipv4 and ipv6

Copy link
Member

Choose a reason for hiding this comment

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

NO. Did you look what you changed? You mixed configuration command and show command into a single one.

@guoguojia2021
Copy link
Contributor Author

It's wrong at various level: documentation, JSON. Also, why can't we reuse show ip route static command, but add an additional keyword for these extra details?

This command 'show ip route static' is is defined in ZEBRA, the new commands is STATICD's commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation master rebase PR needs rebase size/L staticd tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants