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: add VRF support to RPKI #15052

Merged
merged 16 commits into from
Jan 22, 2024
Merged

bgpd: add VRF support to RPKI #15052

merged 16 commits into from
Jan 22, 2024

Conversation

louis-6wind
Copy link
Contributor

@louis-6wind louis-6wind commented Dec 21, 2023 •
edited
Loading

Add VRF support to RPKI

Re-Add #5015 that got reverted in #6740 due to rtlib lacking the required support (rtrlib/rtrlib#252).

Mentioned in #14220

This PR is not actually a draft but the topotest has a GPL-3 depedancy that needs to be solved. See #15034 (comment)

@louis-6wind louis-6wind marked this pull request as draft December 21, 2023 13:09
@louis-6wind louis-6wind force-pushed the rpki-vrf-92 branch 4 times, most recently from 9445cb7 to a73037b Compare December 27, 2023 09:00
@riw777 riw777 self-requested a review January 3, 2024 12:42
@louis-6wind louis-6wind force-pushed the rpki-vrf-92 branch 2 times, most recently from 0b2ecdb to 795677e Compare January 10, 2024 14:32
@louis-6wind louis-6wind force-pushed the rpki-vrf-92 branch 2 times, most recently from 0a490c6 to 35d58e6 Compare January 15, 2024 10:52
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@louis-6wind louis-6wind marked this pull request as ready for review January 16, 2024 15:00
@riw777
Copy link
Member

riw777 commented Jan 16, 2024

Could you add before and after CLI examples here? I'm wondering if there's an issue with configuring RPKI per vrf?

ton31337
ton31337 reviewed Jan 18, 2024
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.

And yes, agree with @riw777, could you show the configuration with VRF X, and a default VRF together? No breaking changes?

doc/user/rpki.rst Outdated
router bgp 60001 vrf vrf_connect
bgp router-id 141.22.28.223
network 192.168.0.0/16
neighbor 123.123.123.0 remote-as 60002
Copy link
Member

Choose a reason for hiding this comment

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

Use documentation ranges, please.

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

doc/user/rpki.rst Outdated
!
exit-vrf
router bgp 60001 vrf vrf_connect
bgp router-id 141.22.28.223
Copy link
Member

Choose a reason for hiding this comment

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

Use documentation ranges, please.

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

bgpd/bgp_rpki.c Outdated

for (ALL_LIST_ELEMENTS_RO(rpki_vrf_list, rpki_vrf_nnode, rpki_vrf)) {
if ((!vrfname && rpki_vrf->vrfname) ||
(vrfname && !rpki_vrf->vrfname) ||
Copy link
Member

Choose a reason for hiding this comment

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

!rpki_vrf->vrfname means this is a default VRF?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I have clarified that with a small rework and some comments

bgpd/bgp_rpki.c Outdated
else
rpki_vrf = VTY_GET_CONTEXT(rpki_vrf);

if (!rpki_vrf->cache_list)
Copy link
Member

Choose a reason for hiding this comment

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

rpki_vrf can be NULL, and we should check before dereferencing 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.

done

bgpd/bgp_rpki.c Outdated
return reset(true, rpki_vrf) == SUCCESS ? CMD_SUCCESS : CMD_WARNING;
}

DEFUN (rpki_reset_config_mode,
Copy link
Member

Choose a reason for hiding this comment

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

DEFPY

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

bgpd/bgp_rpki.c Outdated
rpki_vrf = get_rpki_vrf(vrfname);

if (!rpki_vrf)
return CMD_SUCCESS;
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if this really should be treated as SUCCESS...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMD_WARNING instead

pguibert6WIND and others added 5 commits January 18, 2024 12:01
@pguibert6WIND @louis-6wind
bgpd: move rpki variables in a per vrf fist
f1916d4 
RPKI stores its data in global variables. It does not allow specific
date per VRF.

Move global variable to a new structure named rpki_vrf and maintain a
per VRF list of rpki_vrf. The changes are cosmetic because only the
default VRF is supported yet.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: remove rpki config command from enable node
9746175 
Remove rpki config command from enable node. It cannot work.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: add a hook to inform a vrf is enabled/disabled
b9d6487 
Add a hook to call a future callback function when bgpd knows from zebra
about the activation of de-activation of a VRF. It will be used by the
RPKI module in next commits.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
bgpd: start or stop rpki at vrf creation or deletion
a3bd9ba 
Start or stop a RPKI cache servers in VRF when they are created or
deleted.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: create cache server socket in vrf
a951752 
Create cache server socket in vrf

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
pguibert6WIND and others added 11 commits January 18, 2024 13:23
@pguibert6WIND @louis-6wind
bgpd: add support of vrf to rpki in enable mode
fd83486 
Add a "vrf <vrfname>" argument to "show rpki" and "rpki" commands in
enable mode

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: add support of rpki in vrf configure context
1420189 
Add support of RPKI commands in the VRF configure context.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: show per vrf rpki configuration in show run
a42d490 
Show per VRF RPKI configuration in "show run".

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: set rpki validation state in vrf table
a54050a 
Set the RPKI validation state in the VRF BGP table. It allows applying
a route-maps with "match rpki <state>" on a VRF neighbor.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: add vrf name to some rpki logs
36cef51 
Add VRF name to some rpki logs

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
doc: use documentation range in rpki manual
aca422d 
Use documentation range in rpki manual

Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
doc: fix rpki ipv4 address family example
1aa75f3 
Fix RPKI IPv4 address family example.

Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
doc: remove rpki ssh local server public key
45dd49e 
It is not used anymore.

Fixes: 2a5f5ec ("bgpd: Drop SSH public key for RPKI CLI option")
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
doc: add information for per vrf rpki commands
ba91613 
Add documentation for per VRF RPKI commands

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@pguibert6WIND @louis-6wind
bgpd: add show bgp rpki prefix-count command
98e9655 
Add "show bgp rpki prefix-count" command to show the number of received
prefixes from RPKI cache servers.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind
topotests: add vrf test to bgp_rpki_topo1
2b30c4a 
Add vrf test to bgp_rpki_topo1

Signed-off-by: Louis Scalbert <[email protected]>
@frrbot frrbot bot added the bugfix label Jan 18, 2024
@louis-6wind
Copy link
Contributor Author

@riw777 @ton31337 one configuration example for the default VRF and a VRF now

@ton31337 ton31337 added this to the 10.0 milestone Jan 19, 2024
@ton31337
Copy link
Member

ci:rerun CI successful, but github check (hook) missed

@ton31337 ton31337 merged commit 20ec72d into FRRouting:master Jan 22, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ton31337 ton31337 ton31337 approved these changes

@riw777 riw777 Awaiting requested review from riw777

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

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