1. Added 5 test cases to verify BGP AS-allow-in behavior in FRR
2. Enhanced framework to support BGP AS-allow-in config(lib/bgp.py)
3. Added API in bgp.py to verify BGP RIB table(lib/bgp.py)
bgpd: Trigger EVPN type-5 injection upon link-bandwidth change
Ensure that upon a link-bandwidth change - for e.g., due to change in
the number of multipaths - EVPN type-5 route injection is triggered.
In the absence of this, the proper link-bandwidth is not updated in
EVPN type-5 routes originated by the router.
David Lamparter [Wed, 8 Apr 2020 13:17:21 +0000 (15:17 +0200)]
tests: fix parallel build race
If we're building with a separate build directory, these two build
targets can fail in case their output directory hasn't been created by
some other target that may or may not have run earlier.
Signed-off-by: David Lamparter <equinox@diac24.net>
tests: Adding new test suite bgp_communities_topo1
1. Added 1 test case to verify NO-ADVERTISE Community functionality
2. Enhanced bgp.py to exclude routers from verification, if doesn't have bgp config
1. Added 5 test cases to verify BGP AS-allow-in behavior in FRR
2. Enhanced framework to support BGP AS-allow-in config(lib/bgp.py)
3. Added API in bgp.py to verify BGP RIB table(lib/bgp.py)
for some reason, when issuing a 'no metric-style' command we were
setting the metric-style to narrow, even though the default is
actually wide. Use NULL to avoid similar problems in the future.
Likewise, the 'no is-type' command was still trying to implement
the old logic of applying a different default for the first area.
In practice this had no effect because the value would now be the
same in both cases, but it's better to remove useless code anyway.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Mark Stapp [Thu, 26 Mar 2020 18:11:56 +0000 (14:11 -0400)]
lib: support replacement in the nexthop-group cli
Use more limited matching logic so that nexthops within a
nexthop-group are unique based only on vrf, type, and gateway.
Treat configuration of a nexthop that matches an existing
nexthop as a replace operation.
zebra should only check whether a get_chunk operation succeeded
when processing the response, rather than insde the get_chunk
call itself. Spllitting the request and response hooks was done
precisely to allow for asynchronous calls to an external label
manager; in this case, the requested chunk is not necessarily
going to be available at request time.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Yang constraints enforced by the northbound callbacks require that
the maximum lifetime be >= than (refresh interval + 300). When we are
moving from one config to another through frr-reload.py, we issue
a number of vtysh -c commands ('no lsp-refresh-interval level-1 500',
'no max-lsp-lifetime level-1 1000'), which reset these parameters to their
default values, respectively 900 and 1200. Depending on the actual
values in the current config, the order in which these commands are sent
might be the wrong one, in that we hit an invalid intermediate state and
make vtysh (and by extension frr-reload.py) return an error.
As a workaround, let's add a one-liner command that sets all these
inter-related parameters in one go, and make isisd display them as a
single line too, so that the diff will be computed as a single command.
The old individual commands are kept to ensure backwards compatibility.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Quentin Young [Sun, 5 Apr 2020 21:11:25 +0000 (17:11 -0400)]
bgpd: fix multiple bugs with cluster_list attrs
Multiple different issues causing mostly UAFs but maybe other more
subtle things.
- Cluster lists were the only attributes whose pointers were not being
NULL'd when freed, resulting in heap UAF
- When performing an insert into the cluster hash, our temporary struct
used for hash_get() was inconsistent with our hash keying and
comparison functions. In the case of a zero length cluster list, the
->length field is 0 and the ->list field is NULL. When performing an
insert, we set the ->list field regardless of whether the length is 0.
This resulted in the two cluster lists hashing equal but not comparing
equal. Later, when removing one of them from the hash before freeing
it, because the key matched and the comparison succeeded (because it
was set to NULL *after* the search but *before* inserting into the
hash) we would sometimes release the duplicated copy of the struct,
and then free the one that remained in the hash table. Later accesses
constitute UAF. This is fixed by making sure the fields used for the
existence check match what is actually inserted into the hash when
that check fails.
This patch also makes cluster_unintern static, because it should be.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
lib: consolidate flexible array hack in a single place
Old gcc versions (< 5.x) have a bug that prevents C99 flexible
arrays from working properly on shared libraries.
We already have a hack in place to work around this problem, but it
needs to be replicated in every declaration of a frr_yang_module_info
variable within libfrr. This clearly isn't a good solution if we
consider that many more libfrr YANG modules are about to come in
the future.
This commit introduces a different workaround that operates within
the northbound layer itself, such that implementers of libfrr YANG
modules won't need to worry about this problem anymore.
lib, tools: silence harmless warnings in the northbound tools
Our two northbound tools don't have embedded YANG modules like the
other FRR binaries. As such, ly_ctx_set_module_imp_clb() shouldn't be
called when the YANG subsystem it being initialized by a northbound
tool. To make that possible, add a new "embedded_modules" parameter
to the yang_init() function to control whether libyang should look
for embedded modules or not.
With this fix, "gen_northbound_callbacks" and "gen_yang_deviations"
won't emit "YANG model X not embedded, trying external file"
warnings anymore.
David Lamparter [Thu, 2 Apr 2020 19:16:04 +0000 (21:16 +0200)]
bgpd, ospfd, ospf6d: long is not bool :(
... Oops ...
(for context, the defaults code originally didn't have a dedicated
"bool" variant and just used long for bools... I derp'd this when
adding bool as a separate case :( )
Reported-by: Donald Sharp <sharpd@cumulusnetworks.com> Signed-off-by: David Lamparter <equinox@diac24.net>
Stephen Worley [Wed, 1 Apr 2020 19:31:40 +0000 (15:31 -0400)]
zebra: free unhashable (dup) NHEs via ID table cleanup
Free unhashable (duplicate NHEs from the kernel) via ID table
cleanup. Since the NHE ID hash table contains extra entries,
that's the one we need to be calling zebra_nhg_hash_free()
on, otherwise we will never free the unhashable NHEs.
This was found via a memleak:
==1478713== HEAP SUMMARY:
==1478713== in use at exit: 10,267 bytes in 46 blocks
==1478713== total heap usage: 76,810 allocs, 76,764 frees, 3,901,237 bytes allocated
==1478713==
==1478713== 208 (88 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 35 of 41
==1478713== at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==1478713== by 0x48E35E8: qcalloc (memory.c:110)
==1478713== by 0x451CCB: zebra_nhg_alloc (zebra_nhg.c:369)
==1478713== by 0x453DE3: zebra_nhg_copy (zebra_nhg.c:379)
==1478713== by 0x452670: nhg_ctx_process_new (zebra_nhg.c:1143)
==1478713== by 0x4523A8: nhg_ctx_process (zebra_nhg.c:1234)
==1478713== by 0x452A2D: zebra_nhg_kernel_find (zebra_nhg.c:1294)
==1478713== by 0x4326E0: netlink_nexthop_change (rt_netlink.c:2433)
==1478713== by 0x427320: netlink_parse_info (kernel_netlink.c:945)
==1478713== by 0x432DAD: netlink_nexthop_read (rt_netlink.c:2488)
==1478713== by 0x41B600: interface_list (if_netlink.c:1486)
==1478713== by 0x457275: zebra_ns_enable (zebra_ns.c:127)
Repro with:
ip next add id 1 blackhole
ip next add id 2 blackhole
valgrind /usr/lib/frr/zebra
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
projects / mirror / frr.git / log