-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
bgpd: vpn - vrf route leaking #1739
bgpd: vpn - vrf route leaking #1739
Conversation
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2547/ This is a comment from an EXPERIMENTAL automated CI system. CLANG Static Analyzer Summary
New warnings:
Static Analysis warning summary compared to base:
21 Static Analyzer issues remaining.See details at |
The "new" static analysis issue at ripd/ripd.c line 2325 seems to have already been addressed in commit aea175a |
🚧 Basic BGPD CI results: Partial FAILURE, 1 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2552/ This is a comment from an EXPERIMENTAL automated CI system. CLANG Static Analyzer Summary
New warnings:
Static Analysis warning summary compared to base:
21 Static Analyzer issues remaining.See details at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first walk through the changes.
I guess there will be some more zebra changes ?
bgpd/bgp_zebra.c
Outdated
} | ||
#endif | ||
zlog_debug("%s: withdraw vpn->vrf", __func__); /* TBD delete me */ | ||
/* vpn -> vrf (happend within bgp but we hijack redist bits */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this trace will be cleaned I suppose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes commit 480cef0
ECOMMUNITY_FORMAT_ROUTE_MAP, | ||
ECOMMUNITY_ROUTE_TARGET); | ||
vty_out(vty, " rt fromvpn %s\n", b); | ||
XFREE(MTYPE_ECOMMUNITY_STR, b); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was there an agreement on the vty naming convention ?
rt from vpn <=> rt export
rt to vpn <=> rt import
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think we achieved consensus. This is placeholder CLI so we can exercise code until we finalize syntax.
bgpd/bgp_mplsvpn.c
Outdated
info_vrf->type); | ||
|
||
if (info_vrf->type == ZEBRA_ROUTE_BGP_VPN) { | ||
/* loop check! */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless braces.
you can simplify
bgpd/bgp_mplsvpn.c
Outdated
@@ -233,6 +236,762 @@ int bgp_nlri_parse_vpn(struct peer *peer, struct attr *attr, | |||
#undef VPN_PREFIXLEN_MIN_BYTES | |||
} | |||
|
|||
static int ecom_intersect(struct ecommunity *e1, struct ecommunity *e2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be moved to bgp_ecommunity.c and Could we please add a little description of what the API is suppose to do ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using ecom_intersect() is fine but as a second step, we should do some optimization. In EVPN, we have built a hash of VRFs that are interested in a particular RT and so there is just a hash lookup (per RT in route) followed by getting the interested VRFs. @mkanjari or I could help do the optimization too after initial commit of this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that optimization in this way is a good idea. After this round is committed, let's revisit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds fine to me
bgpd/bgp_mplsvpn.c
Outdated
|
||
bgp_attr_unintern(&new_attr); | ||
if (debug) | ||
zlog_debug("%s: Found route, no change", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a more specific debug statement ? This is pretty generic and won't give any information while debugging. Or if this is not necessary, can we please remove it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gives an indication of which code branch was taken. What additional information would you like to see?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some additional information like the prefix at the least ? generally we will grep for a specific prefix or some attr while debugging. It will be helpful if we print that information too in the same line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like a good idea. Let me look at these more closely...
bgpd/bgp_mplsvpn.c
Outdated
bgp_unlock_node(bn); | ||
|
||
if (debug) | ||
zlog_debug("%s: Found route, changed attr", __func__); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, we need a more spefic debug statement here.
bgp_unlock_node(bn); | ||
bgp_process(bgp, bn, afi, safi); | ||
|
||
if (debug) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above.
@paulzlabn - I have gone over the code flow pretty completely. The code is pretty easy to follow! Well done! I will refrain from comments on the CLI since we're discussing that internally - except for one point below. I have a few general comments below and a few specific comments which I'll include inline.
|
@@ -4478,6 +4523,11 @@ static void bgp_static_update_safi(struct bgp *bgp, struct prefix *p, | |||
/* Process change. */ | |||
bgp_aggregate_increment(bgp, p, ri, afi, safi); | |||
bgp_process(bgp, rn, afi, safi); | |||
|
|||
if (SAFI_UNICAST == safi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check seems wrong, the safi should be SAFI_MPLS_VPN here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What safi is supposed to be here? @paulzlabn I see agreement but I don't see the change here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@donaldsharp In bgp_static_withdraw_safi() and bgp_static_update_safi(), it should be SAFI_MPLS_VPN because it applies to routes originating in ipv4|ipv6 vpn and leaking to a unicast vrf. In bgp_static_update() and bgp_static_withdraw(), it should be SAFI_UNICAST because it applies to routes that originate in a unicast vrf and leaking to ipv4|ipv6 vpn.
bgpd/bgp_mplsvpn.c
Outdated
|
||
for (bi = bn->info; bi; bi = bi->next) { | ||
|
||
if (bi->type == ZEBRA_ROUTE_BGP_VPN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is mentioned in my general comment too, but in addition, this won't work for VRF-route-leaking because the route exported from (one) VRF to the VPN table is of type ZEBRA_ROUTE_BGP_VPN and this check will prevent it being imported into another VRF - unless I misunderstood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. For now I have added a check also between the source and destination vrf, which should permit vrf1->vpn->vrf2.
bgpd/bgp_vty.c
Outdated
if (nhself) { | ||
UNSET_FLAG(bgp->vpn_policy[afi].flags, | ||
BGP_VPN_POLICY_TOVPN_NEXTHOP_SET); | ||
continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "continue" seems incorrect. Don't we have to do the vpn_leak_postchange() even if the nexthop is set to "self"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Fixed.
bgpd/bgp_mplsvpn.c
Outdated
} | ||
|
||
/* Set originator ID to "me" */ | ||
SET_FLAG(static_attr.flag, ATTR_FLAG_BIT(BGP_ATTR_ORIGINATOR_ID)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the use/need of the Originator ID is only for Route Reflection. Why is it being set here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that the originator id is only included in the outgoing update when bgpd is acting as a RR:
bgp_attr.c:
3078 /* Route Reflector. */
3079 if (peer->sort == BGP_PEER_IBGP && from
3080 && from->sort == BGP_PEER_IBGP) {
3081 /* Originator ID. */
3082 stream_putc(s, BGP_ATTR_FLAG_OPTIONAL);
3083 stream_putc(s, BGP_ATTR_ORIGINATOR_ID);
3084 stream_putc(s, 4);
3085
3086 if (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_ORIGINATOR_ID))
3087 stream_put_in_addr(s, &attr->originator_id);
3088 else
3089 stream_put_in_addr(s, &from->remote_id);
imported/exported routes still have their original peer value, which can lead to the wrong originator id when we act as RR (problem we encountered some years ago in the vnc/rfapi code). Always setting it seems to do the right thing. Does this approach break anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems okay to always set the originator id -- it is generally ignored except in the case of an RR. Might be worth having @dwalton76 comment here, however, to be certain.
One more general comment. The calls to vpn_leak_from_vrf_update()/withdraw() is missing in the route redistribute code path (say, when 'static' or 'ospf' route in a VRF is redistributed into BGP). |
Vivek, thank you for the detailed comments. I will follow up to your comments on specific code blocks individually, but here are responses to your numbered items:
In case (a), the current code tests bi->extra->parent against (struct bgp *)bgp_vrf, so the type check is not critical. However, we should be able to flag VPN RIB routes that came from a vrf in the "show route" output somehow. Also, if I stop abusing bi->extra->parent (per your comment 3), there needs to be some way for the code to identify VPN RIB routes that were imported from a specific vrf. Maybe a new vrf field in struct bgp_info_extra? In case (b), if we have a BGP_ROUTE_VPN sub_type, that would suffice (see vpn_leak_to_vrf_withdraw_all())
b) bi->extra->parent: Although I don't see a conflict with the existing code (maybe there is one I don't see), I agree that putting the source bgp_vrf pointer there is not the original purpose. If nobody minds the extra memory usage, I could add a new field to struct bgp_info_extra.
|
Hi Vivek,
With respect to the point below - a typical L3VPN use case is to fully
interconnect customer sites so all route export is just what is needed.
I certainly agree that this isn't the only use case and often only a
select set of routes should be exported/redistributed -- and this is
supported via routemaps. Make sense?
Lou
…On 2/15/2018 8:21 AM, Vivek Venkatraman wrote:
The code attempts to export all routes (i.e., route entry/info) from
VRF table to VPN table and vice versa (based on configs). This is
debatable, it may be ok to just export the selected path.
|
Thank you for catching that, fixing now... |
@vivek-cumulus Here is a proposal to do away with ZEBRA_ROUTE_BGP_VPN and parent = bgp_vrf
|
🚧 Basic BGPD CI results: Partial FAILURE, 1 tests failedResults table
For details, please contact louberger |
@pguibert6WIND @qlyoung where are we on the review? @paulzlabn can you update to get rid of the conflict? |
I just realized there were some more comments from last week which I hadn't addressed. Changeset incoming... |
🛑 Basic BGPD CI results: FAILUREResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an EXPERIMENTAL automated CI system. Get source and apply patch from patchwork: SuccessfulBuilding Stage: FailedFedora24 amd64 build: FailedMake failed for Fedora24 amd64 build:
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI015BUILD/config.status/config.status FreeBSD11 amd64 build: FailedMake failed for FreeBSD11 amd64 build:
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI009BUILD/config.status/config.status NetBSD7 amd64 build: FailedMake failed for NetBSD7 amd64 build:
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI012BUILD/config.status/config.status Ubuntu1604 amd64 build: FailedMake failed for Ubuntu1604 amd64 build:
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI014BUILD/config.status/config.status Ubuntu 16.04 i386: FailedMake failed for Ubuntu 16.04 i386:
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/U1604I386/config.status/config.status NetBSD6 amd64 build: FailedMake failed for NetBSD6 amd64 build:
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI007BUILD/config.status/config.status CentOS7 amd64 build: FailedMake failed for CentOS7 amd64 build:
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI005BUILD/config.status/config.status Debian8 amd64 build: FailedMake failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI008BLD/config.status/config.status CentOS6 amd64 build: FailedMake failed for CentOS6 amd64 build:
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI006BUILD/config.status/config.status FreeBSD10 amd64 build: FailedMake failed for FreeBSD10 amd64 build:
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI003BUILD/config.status/config.status FreeBSD9 amd64 build: FailedMake failed for FreeBSD9 amd64 build:
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI004BUILD/config.status/config.status Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI021BUILD/config.status/config.status
OmniOS amd64 build: FailedMake failed for OmniOS amd64 build:
OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI010BUILD/config.status/config.status Ubuntu1404 amd64 build: FailedMake failed for Ubuntu1404 amd64 build:
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI001BUILD/config.status/config.status OpenBSD60 amd64 build: FailedMake failed for OpenBSD60 amd64 build:
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI011BUILD/config.status/config.status Ubuntu1204 amd64 build: FailedMake failed for Ubuntu1204 amd64 build:
Ubuntu1204 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI002BUILD/config.status/config.status Warnings Generated during build:Checkout code: Successful with additional warnings:Fedora24 amd64 build: FailedMake failed for Fedora24 amd64 build:
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI015BUILD/config.status/config.status FreeBSD11 amd64 build: FailedMake failed for FreeBSD11 amd64 build:
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI009BUILD/config.status/config.status NetBSD7 amd64 build: FailedMake failed for NetBSD7 amd64 build:
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI012BUILD/config.status/config.status Ubuntu1604 amd64 build: FailedMake failed for Ubuntu1604 amd64 build:
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI014BUILD/config.status/config.status Ubuntu 16.04 i386: FailedMake failed for Ubuntu 16.04 i386:
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/U1604I386/config.status/config.status NetBSD6 amd64 build: FailedMake failed for NetBSD6 amd64 build:
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI007BUILD/config.status/config.status CentOS7 amd64 build: FailedMake failed for CentOS7 amd64 build:
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI005BUILD/config.status/config.status Debian8 amd64 build: FailedMake failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI008BLD/config.status/config.status CentOS6 amd64 build: FailedMake failed for CentOS6 amd64 build:
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI006BUILD/config.status/config.status FreeBSD10 amd64 build: FailedMake failed for FreeBSD10 amd64 build:
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI003BUILD/config.status/config.status FreeBSD9 amd64 build: FailedMake failed for FreeBSD9 amd64 build:
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI004BUILD/config.status/config.status Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI021BUILD/config.status/config.status
OmniOS amd64 build: FailedMake failed for OmniOS amd64 build:
OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI010BUILD/config.status/config.status Ubuntu1404 amd64 build: FailedMake failed for Ubuntu1404 amd64 build:
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI001BUILD/config.status/config.status OpenBSD60 amd64 build: FailedMake failed for OpenBSD60 amd64 build:
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI011BUILD/config.status/config.status Ubuntu1204 amd64 build: FailedMake failed for Ubuntu1204 amd64 build:
Ubuntu1204 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2903/artifact/CI002BUILD/config.status/config.status
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Executed full RFC compliance for OSPFv2, OSPFv3, ISIS IPv4 & IPv6, BGP4 IPv4 & IPv6, LDP and all good (same as master & 4.0)
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an EXPERIMENTAL automated CI system. Get source and apply patch from patchwork: SuccessfulBuilding Stage: FailedFreeBSD11 amd64 build: Successful Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/CI021BUILD/config.status/config.status
Ubuntu 16.04 i386: FailedPackage building failed for Ubuntu 16.04 i386:
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/U1604I386/config.status/config.status Ubuntu1604 amd64 build: FailedPackage building failed for Ubuntu1604 amd64 build:
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/CI014BUILD/config.status/config.status Debian8 amd64 build: FailedPackage building failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/CI008BLD/config.status/config.status Warnings Generated during build:Checkout code: Successful with additional warnings:Debian9 amd64 build: FailedDebian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/CI021BUILD/config.status/config.status
Ubuntu 16.04 i386: FailedPackage building failed for Ubuntu 16.04 i386:
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/U1604I386/config.status/config.status Ubuntu1604 amd64 build: FailedPackage building failed for Ubuntu1604 amd64 build:
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/CI014BUILD/config.status/config.status Debian8 amd64 build: FailedPackage building failed for Debian8 amd64 build:
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2907/artifact/CI008BLD/config.status/config.status
|
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
🚧 Basic BGPD CI results: Partial FAILURE, 1 tests failedResults table
For details, please contact louberger |
Filing issue against myself to fix issues at later time
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-2908/ This is a comment from an EXPERIMENTAL automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings:
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base19 Static Analyzer issues remaining.See details at |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
I spoke with @vivek-cumulus and he indicated it would be good to merge this as well. |
ok to merge. |
PR FRRouting#1739 added code to leak routes between (default VRF) VPN safi and unicast RIBs in any VRF. That set of changes included temporary CLI including vpn-policy blocks to specify RD/RT/label/&c. After considerable discussion, we arrived at a consensus CLI shown below. The code of this PR implements the vpn-specific parts of this syntax: router bgp <as> [vrf <FOO>] address-family <afi> unicast rd (vpn|evpn) export (AS:NN | IP:nn) label (vpn|evpn) export (0..1048575) rt (vpn|evpn) (import|export|both) RTLIST... nexthop vpn (import|export) (A.B.C.D | X:X::X:X) route-map (vpn|evpn|vrf NAME) (import|export) MAP [no] import|export [vpn|evpn|evpn8] [no] import|export vrf NAME User documentation of the vpn-specific parts of the above syntax is in PR FRRouting#1937 Signed-off-by: G. Paul Ziemba <paulz@labn.net>
PR FRRouting#1739 added code to leak routes between (default VRF) VPN safi and unicast RIBs in any VRF. That set of changes included temporary CLI including vpn-policy blocks to specify RD/RT/label/&c. After considerable discussion, we arrived at a consensus CLI shown below. The code of this PR implements the vpn-specific parts of this syntax: router bgp <as> [vrf <FOO>] address-family <afi> unicast rd (vpn|evpn) export (AS:NN | IP:nn) label (vpn|evpn) export (0..1048575) rt (vpn|evpn) (import|export|both) RTLIST... nexthop vpn (import|export) (A.B.C.D | X:X::X:X) route-map (vpn|evpn|vrf NAME) (import|export) MAP [no] import|export [vpn|evpn|evpn8] [no] import|export vrf NAME User documentation of the vpn-specific parts of the above syntax is in PR FRRouting#1937 Signed-off-by: G. Paul Ziemba <paulz@labn.net>
bgpd: vpn-vrf route leaking (includes default instance unicast as special case of vrf)