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

bgpd: misc RPKI fixes #15051

Closed
wants to merge 13 commits into from
Closed

bgpd: misc RPKI fixes #15051

wants to merge 13 commits into from

Conversation

louis-6wind
Copy link
Contributor

misc RPKI fixes

louis-6wind and others added 10 commits December 21, 2023 14:04
@louis-6wind
bgpd: fix show rpki json void output
f53e5d5 
"show rpki XX json" should not return a void output because json.loads()
considers it to be an incorrect JSON.

> >>> json.loads("")
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
>   File "/usr/lib/python3.9/json/__init__.py", line 346, in loads
>     return _default_decoder.decode(s)
>   File "/usr/lib/python3.9/json/decoder.py", line 337, in decode
>     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
>   File "/usr/lib/python3.9/json/decoder.py", line 355, in raw_decode
>     raise JSONDecodeError("Expecting value", s, err.value) from None
> json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
> >>> json.loads("{}")
> {}

Return "{}" instead in such a case.

Link: FRRouting#15034
Fixes: dff41cc ("bgpd: Add JSON output for `show rpki prefix` and other show commands")
Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
bgpd: fix rpki compilation without rtrlib ssh
0b7a7ba 
Fix RPKI module compilation when rtrlib is compiled without SSH support,
ie. with cmake option:

> -D RTRLIB_TRANSPORT_SSH=No

> bgpd/bgp_rpki.c: In function ‘config_write’:
> bgpd/bgp_rpki.c:1062:3: error: enumeration value ‘SSH’ not handled in switch [-Werror=switch-enum]
>  1062 |   switch (cache->type) {
>       |   ^~~~~~
> bgpd/bgp_rpki.c: In function ‘show_rpki_cache_connection_magic’:
> bgpd/bgp_rpki.c:1598:3: error: enumeration value ‘SSH’ not handled in switch [-Werror=switch-enum]
>  1598 |   switch (cache->type) {
>       |   ^~~~~~
> cc1: all warnings being treated as errors

Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
topotests: add bgp_rpki_topo1
0cf6446 
Add bgp_rpki_topo1 topotest to validate the RPKI feature. Use a RTR RPKI
server from the above link.

Link: FRRouting#15034
Link: https://raw.githubusercontent.com/SmartValidator/rtr-python/368a4e268aaae0de7d88d0c34665315661851d40/rtrd.py
Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
topotests: bgp_rpki_topo1, load config at startup
ed1726f 
Load r2 rpki configuration at bgpd startup. Set retry interval to 5s
instead of the 600s value so rtrlib retries to re-connect every 5s if
the network is not ready.

Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
topotests: resent cache at reset in rtrd.py
aac9088 
rtrd must resent all the cache after receiving a "reset query". However,
it is not done because the last_serial is not reset before reaching the
get_announcements4() method.

Reset last_serial to allow cache resent.

Link: https://www.rfc-editor.org/rfc/rfc8210.html#page-12
Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
topotests: add tests to bgp_rpki_topo1
a947ebd 
Add tests to bgp_rpki_topo1:

 - removing and re-adding a RPKI cache server.
 - test RPKI validity in BGP table
 - test application of route-map with "match rpki valid"

Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
bgpd: fix crash when re-adding a rpki server
b13fbfa 
Fix a crash when re-adding a rpki server:

> r2# sh run bgpd
> [...]
> rpki
>  rpki retry_interval 5
>  rpki cache 192.0.2.1 15432 preference 1
> exit
> [...]
> r2# conf t
> r2(config)# rpki
> r2(config-rpki)# no rpki cache 192.0.2.1 15432 preference 1
> r2(config-rpki)# do show rpki cache-connection
> Cannot find a connected group.
> r2(config-rpki)# rpki cache 192.0.2.1 15432 preference 1
> r2(config-rpki)# do show rpki cache-connection
> vtysh: error reading from bgpd: Resource temporarily unavailable (11)Warning: closing connection to bgpd because of an I/O error!

> #0  raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007f3fd2d16e57 in core_handler (signo=11, siginfo=0x7ffffd5931b0, context=0x7ffffd593080) at lib/sigevent.c:246
> #2  <signal handler called>
> #3  0x00007f3fd26926b4 in tommy_list_head (list=0x2e322e302e323931) at /home/lscalber/git/rtrlib/./third-party/tommyds/tommylist.h:125
> FRRouting#4  0x00007f3fd2693812 in rtr_mgr_get_first_group (config=0x55fbf31d7f00) at /home/lscalber/git/rtrlib/rtrlib/rtr_mgr.c:409
> FRRouting#5  0x00007f3fd2ebef59 in get_connected_group () at bgpd/bgp_rpki.c:718
> FRRouting#6  0x00007f3fd2ec0b39 in show_rpki_cache_connection_magic (self=0x7f3fd2ec69c0 <show_rpki_cache_connection_cmd>, vty=0x55fbf31f9ef0, argc=3, argv=0x55fbf31f99d0, uj=0x0)
> #   at bgpd/bgp_rpki.c:1575
> FRRouting#7  0x00007f3fd2ebd4da in show_rpki_cache_connection (self=0x7f3fd2ec69c0 <show_rpki_cache_connection_cmd>, vty=0x55fbf31f9ef0, argc=3, argv=0x55fbf31f99d0) at ./bgpd/bgp_rpki_clippy.c:648
> FRRouting#8  0x00007f3fd2c8a142 in cmd_execute_command_real (vline=0x55fbf31f9990, vty=0x55fbf31f9ef0, cmd=0x0, up_level=0) at lib/command.c:978
> FRRouting#9  0x00007f3fd2c8a25c in cmd_execute_command (vline=0x55fbf31e5260, vty=0x55fbf31f9ef0, cmd=0x0, vtysh=0) at lib/command.c:1028
> FRRouting#10 0x00007f3fd2c8a7f1 in cmd_execute (vty=0x55fbf31f9ef0, cmd=0x55fbf3200680 "do show rpki cache-connection ", matched=0x0, vtysh=0) at lib/command.c:1203
> FRRouting#11 0x00007f3fd2d36548 in vty_command (vty=0x55fbf31f9ef0, buf=0x55fbf3200680 "do show rpki cache-connection ") at lib/vty.c:594
> FRRouting#12 0x00007f3fd2d382e1 in vty_execute (vty=0x55fbf31f9ef0) at lib/vty.c:1357
> FRRouting#13 0x00007f3fd2d3a519 in vtysh_read (thread=0x7ffffd5963c0) at lib/vty.c:2365
> FRRouting#14 0x00007f3fd2d2faf6 in event_call (thread=0x7ffffd5963c0) at lib/event.c:1974
> FRRouting#15 0x00007f3fd2cc238e in frr_run (master=0x55fbf2a0cd60) at lib/libfrr.c:1214
> FRRouting#16 0x000055fbf073de40 in main (argc=9, argv=0x7ffffd596618) at bgpd/bgp_main.c:510

Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
bgpd: fix resetting rpki timer at no rpki
eaeb581 
RPKI configuration is not totally flushed when doing "no rpki". Timers
remains to default values.

> r2# sh run bgpd
> [...]
> rpki
>  rpki retry_interval 5
>  rpki cache 192.0.2.1 15432 preference 1
> exit
> [...]
> r2# conf t
> r2(config)# no rpki
> r2(config)# do sh run
> [...]
> rpki
>  rpki retry_interval 5
> exit

Reset the timers after doing "no rpki"

Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: remove double spaces with rpki running config & ssh
c665d33 
remove double spaces when doing show running-config.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: do not show default rpki timers in show run
7a40143 
"show run" displays the default RPKI timers when at least one cache
server is configured.

Only display the RPKI timers that differs from the default values.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
ton31337
ton31337 requested changes Dec 21, 2023
View reviewed changes
bgpd/bgp_rpki.c Show resolved Hide resolved
bgpd/bgp_rpki.c
@@ -1684,6 +1691,46 @@ DEFPY (show_rpki_cache_connection,
return CMD_SUCCESS;
}

DEFPY(show_rpki_configuration, show_rpki_configuration_cmd,
Copy link
Member

Choose a reason for hiding this comment

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

Documentation is missing for this new CLI.

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 d1e523b

bgpd/bgp_rpki.c Show resolved Hide resolved
louis-6wind and others added 3 commits December 21, 2023 17:33
@louis-6wind
doc: add show rpki configuration
d1e523b 
Add documentation about the new "show rpki configuration" command.

Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: add hooks for displaying module debug messages
69a6875 
when a plugin is attached, some debugs may be attached to that plugin.
For that, add one hook that is interacting with vty: a boolean indicates
what the usage is for: either for impacting the 'show running-config',
or for impacting the 'show debugging' command.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: fix debug rpki display in show run
a4c54dd 
Only include "debug rpki" in "show run" if it was requested from the
configure mode but not it was from the enabled mode.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@frrbot frrbot bot added the documentation label Dec 21, 2023
@github-actions github-actions bot added size/XL and removed size/L labels Dec 21, 2023
ton31337
ton31337 requested changes Dec 22, 2023
View reviewed changes
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.

This PR still has changes included in #15034. Is this supposed to be?

pguibert6WIND
pguibert6WIND reviewed Dec 22, 2023
View reviewed changes
bgpd/bgp_rpki.c
if (!uj)
if (uj)
vty_json(vty, json);
else
vty_out(vty, "No Connection to RPKI cache server.\n");
return CMD_WARNING;
Copy link
Member

Choose a reason for hiding this comment

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

is it possible to encapsulate error message in a json attribute ?

Copy link
Member

Choose a reason for hiding this comment

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

Good point @pguibert6WIND, we might do this for all warning/error handling. Would be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I carry on in #15034
1f1b839 has been added for that

pguibert6WIND
pguibert6WIND reviewed Dec 22, 2023
View reviewed changes
Copy link
Member

@pguibert6WIND pguibert6WIND left a comment

Choose a reason for hiding this comment

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

only one remark about json output: if it is possible to keep the vty error messages in a field in json.

@louis-6wind
Copy link
Contributor Author

Merged in #15034

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pguibert6WIND pguibert6WIND pguibert6WIND left review comments

@ton31337 ton31337 ton31337 requested changes

Assignees
No one assigned
Labels
bgp bugfix documentation master size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@louis-6wind @ton31337 @pguibert6WIND