Build #1,694

FreeRangeRouting Protocol Suite

Build: #1694 failed Changes by Donald Sharp

Build result summary

Details

Completed
Duration
140 minutes
Labels
branch=masterbuildurl=https_//ci1_netdef_org/browse/frr-frr-1694git=https_//github_com/frrouting/frr_gitversion=frr-6_1-dev-794-g44711aef4
Revision
44711aef421652e340a94c9bf53b57883794ec52 44711aef421652e340a94c9bf53b57883794ec52
Total tests
7016
Fixed in
#1695 (Changes by David Lamparter)

Tests

Responsible

Code commits

Author Commit Message Commit date
Donald Sharp Donald Sharp 44711aef421652e340a94c9bf53b57883794ec52 44711aef421652e340a94c9bf53b57883794ec52 Merge pull request #3310 from adeg/bugfix/bgpd-mplsvpn-route-import-check
bgpd: fix bgp path info for mplsvpn leaked routes
Donald Sharp Donald Sharp bddea5fdf87832021d00b43afa1439ce0102a1e5 bddea5fdf87832021d00b43afa1439ce0102a1e5 Merge pull request #3051 from mitch-skiba/addpath_change_V1
Addpath - Reuse IDs
Anton Degtyarev <adeg47@gmail.com> Anton Degtyarev <adeg47@gmail.com> e23b9ef6d271223d29c7f91a10d98aa6dcd252b3 m e23b9ef6d271223d29c7f91a10d98aa6dcd252b3 bgpd: fix bgp path info for mplsvpn leaked routes so that they are correctly seen (and checked) by the rnh module in Zebra
Mitch Skiba <mskiba@amazon.com> Mitch Skiba <mskiba@amazon.com> dcc68b5e2a598a180b370163a7022324a9b5da96 m dcc68b5e2a598a180b370163a7022324a9b5da96 bgpd: Re-use TX Addpath IDs where possible
The motivation for this patch is to address a concerning behavior of
tx-addpath-bestpath-per-AS. Prior to this patch, all paths' TX ID was
pre-determined as the path was received from a peer. However, this meant
that any time the path selected as best from an AS changed, bgpd had no
choice but to withdraw the previous best path, and advertise the new
best-path under a new TX ID. This could cause significant network
disruption, especially for the subset of prefixes coming from only one
AS that were also communicated over a bestpath-per-AS session.

The patch's general approach is best illustrated by
txaddpath_update_ids. After a bestpath run (required for best-per-AS to
know what will and will not be sent as addpaths) ID numbers will be
stripped from paths that no longer need to be sent, and held in a pool.
Then, paths that will be sent as addpaths and do not already have ID
numbers will allocate new ID numbers, pulling first from that pool.
Finally, anything left in the pool will be returned to the allocator.

In order for this to work, ID numbers had to be split by strategy. The
tx-addpath-All strategy would keep every ID number "in use" constantly,
preventing IDs from being transferred to different paths. Rather than
create two variables for ID, this patch create a more generic array that
will easily enable more addpath strategies to be implemented. The
previously described ID manipulations will happen per addpath strategy,
and will only be run for strategies that are enabled on at least one
peer.

Finally, the ID numbers are allocated from an allocator that tracks per
AFI/SAFI/Addpath Strategy which IDs are in use. Though it would be very
improbable, there was the possibility with the free-running counter
approach for rollover to cause two paths on the same prefix to get
assigned the same TX ID. As remote as the possibility is, we prefer to
not leave it to chance.

This ID re-use method is not perfect. In some cases you could still get
withdraw-then-add behaviors where not strictly necessary. In the case of
bestpath-per-AS this requires one AS to advertise a prefix for the first
time, then a second AS withdraws that prefix, all within the space of an
already pending MRAI timer. In those situations a withdraw-then-add is
more forgivable, and fixing it would probably require a much more
significant effort, as IDs would need to be moved to ADVs instead of
paths.

Signed-off-by Mitchell Skiba <mskiba@amazon.com>
Mitch Skiba <mskiba@amazon.com> Mitch Skiba <mskiba@amazon.com> a94eca0968d914d58dff819c2411542377f56bae m a94eca0968d914d58dff819c2411542377f56bae lib: Implement an allocator for 32 bit ID numbers
This commit introduces lib/id_alloc, which has facilities for both an ID number
allocator, and less efficient ID holding pools. The pools are meant to be a
temporary holding area for ID numbers meant to be re-used, and are implemented
as a linked-list stack.

The allocator itself is much more efficient with memory. Based on sizeof
values on my 64 bit desktop, the allocator requires around 155 KiB per
million IDs tracked.

IDs are ultimately tracked in a bit-map split into many "pages." The
allocator tracks a list of pages that have free bits, and which sections
of each page have free IDs, so there isn't any scanning required to find
a free ID. (The library utility ffs, or "Find First Set," is generally a
single CPU instruction.) At the moment, totally empty pages will not be
freed, so the memory utilization of this allocator will remain at the
high water mark.

The initial intended use case is for BGP's TX Addpath IDs to be pulled
from an allocator that tracks which IDs are in use, rather than a free
running counter.  The allocator reserves ID #0 as a sentinel value for
an invalid ID numbers, and BGP will want ID #1 reserved as well. To
support this, the allocator allows for IDs to be explicitly reserved,
though be aware this is only practical to use with low numbered IDs
because the allocator must allocate pages in order.

Signed-off-by Mitchell Skiba <mskiba@amazon.com>

Tests

New test failures 1
Status Test View job Duration
Collapse Failed test_ldp_vpls_topo1 test_ldp_bindings History
Topology Tests on Ubuntu 16.04 amd64 1 min
AssertionError: "r1" JSON output mismatches the expected result assert json value is different (   --- Expected value   +++ Current value   @@ -21 +21 @@   -        "localLabel": "17",    +        "localLabel": "18",    @@ -29 +29 @@   -        "localLabel": "17",    +        "localLabel": "18",    @@ -37 +37 @@   -        "localLabel": "18",    +        "localLabel": "17",    @@ -45 +45 @@   -        "localLabel": "18",    +        "localLabel": "17", )
E   AssertionError: "r1" JSON output mismatches the expected result
    assert json value is different (
      --- Expected value
      +++ Current value
      @@ -21 +21 @@
      -        "localLabel": "17", 
      +        "localLabel": "18", 
(9 more lines...)