Philippe Guibert [Fri, 20 Jul 2018 15:24:43 +0000 (17:24 +0200)]
bgpd: flowspec 'no local-install any' wrong order in show runni
When configuring an interface, the no local-install any command appears,
and leads to confusions. because the effect of that command differs if
it is executed after local-install <interfaces> or before executing
local-install <interfaces>, the proposal fix here is to suppress that
command from the vty available commands.
PR=59595 Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com> Acked-by: Alain Ritoux <alain.ritoux@6wind.com>
bgpd: flush remaining entry if bgp_pbr_match is equal
When adding an entry, a check is done in order to flush previously
configured entries. The whole parameters are checked so as to not remove
some entries that have ipset entries equal, but not iptable settings.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Thu, 28 Jun 2018 15:26:22 +0000 (17:26 +0200)]
bgpd: flowspec pbr entries listed on the bgp information entry
Because one flowspec entry can create 1-N bgp pbr entries, the list is
now updated and visible. Also, because the bgp_extra structure is used,
this list is flushed when the bgp_extra structure is deleted.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Wed, 18 Jul 2018 15:58:45 +0000 (17:58 +0200)]
zebra: sometimes, it is not possible to assign a NSID to a vrf
This test case happens in scenarios with mininet, where external netns
may be impossible for the local instance to be modified. The error is
ignored and the netns parsed is ignored too.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Christian Franke [Tue, 17 Jul 2018 19:14:54 +0000 (15:14 -0400)]
isisd: don't crash when isis_sock_init fails
When isis_sock_init fails in isis_circuit_up, isis_circuit_down would
be called to cancel timers which were scheduled. However
isis_circuit_down would immediately return, since the state had not been
changed to 'UP' yet.
Fix this by having isis_circuit_down always cancel all the timers.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Philippe Guibert [Wed, 30 May 2018 09:46:13 +0000 (11:46 +0200)]
doc: replace 'intact' keyword with something more clear
Keeping the config intact might be misunderstood. I say that even if VRF
netns is automatically discovered, it is possible for administrator to
save the netns information in the config file, to bring more clarity (
hence the config commands available).
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Wed, 30 May 2018 09:38:24 +0000 (11:38 +0200)]
doc: inform the diff between config from zebra and outside
To avoid some confusions, it is precised in the documentation that
the configuration not done from zebra will not be injected in the
configuration context. As consequence, the config file will not be
impacted by underlying network context. But also, this will not be
possible for *Zebra* to attempt to modify outside networking objects.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Martin Winter [Thu, 5 Jul 2018 07:40:17 +0000 (00:40 -0700)]
FRRouting Release 5.0.1
Changes since 5.0:
- Support Automake 1.16.1
- BGPd: Support for flowspec ICMP, DSCP, packet length, fragment and tcp flags
- BGPd: fix rpki validation for ipv6
- VRF: Workaround for kernel bug on Linux 4.14 and newer
- Zebra: Fix interface based routes from zebra not marked up
- Zebra: Fix large zebra memory usage when redistribute between protocols
- Zebra: Allow route-maps to match on source instance
- BGPd: Backport peer-attr overrides, peer-level enforce-first-as and filtered-routes fix
- BGPd: fix for crash during display of filtered-routes
- BGPd: Actually display labeled unicast routes received
- Label Manager: Fix to work correctly behind a label manager proxy
- Debian Package: Fix build dependency for install-info
Signed-off-by: Martin Winter <mwinter@opensourcerouting.org>
Arthur Jones [Mon, 11 Jun 2018 21:55:50 +0000 (14:55 -0700)]
tests: use BUILT_SOURCES in tests/Makefile.am for automake 1.16.1
When trying to run make check using automake 1.16.1, we get:
CC isisd/test_fuzz_isis_tlv-test_fuzz_isis_tlv.o
isisd/test_fuzz_isis_tlv.c:1:10: fatal error: test_fuzz_isis_tlv_tests.h: No such file or directory
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
Makefile:1096: recipe for target 'isisd/test_fuzz_isis_tlv-test_fuzz_isis_tlv.o' failed
make[1]: *** [isisd/test_fuzz_isis_tlv-test_fuzz_isis_tlv.o] Error 1
make[1]: Leaving directory '/src/frr-frr-5.0/tests'
Makefile:1220: recipe for target 'check-am' failed
make: *** [check-am] Error 2
From reading the automake docs, it looks like there may be a more
reliable way to express built files in the Makefile.am using BUILT_SOURCES.
Using this method, we seem to build fine now on 1.16.1 and this
has been tested on Ubuntu 18.04, CentOS 7 and Alpine edge (which uses
automake 1.16.1).
NB 5.0:
This cherry-pick from master will make Alpine packaging easier
Issue: https://github.com/FRRouting/frr/issues/2403 Signed-off-by: Arthur Jones <arthur.jones@riverbed.com>
Arthur Jones [Wed, 6 Jun 2018 14:47:17 +0000 (07:47 -0700)]
tests/isisd: bypass test_fuzz_isis_tlv when inet_ntop is broken
On Alpine Linux edge, musl does not seem to be RFC 5952 4.2.2
compliant (how to print a single :0: in the IPv6 address). Let's
skip that test, as we get false negatives when running against
that version of musl.
Credit for the idea for the fix and how to fix it is due to
chris@opensourcerouting.org.
NB 5.0:
This cherry-pick from master will simplify frr packaging for alpine
Testing done:
make check on alpine linux passes now
Issue: https://github.com/FRRouting/frr/issues/2375 Signed-off-by: Arthur Jones <arthur.jones@riverbed.com>
If a cache server was added after rpki was started it's tr_socket would
not be initialized. This would lead to a segfault if the rtr manager
ever decides to switch to that socket or if rpki support is stopped.
Philippe Guibert [Wed, 20 Jun 2018 14:59:17 +0000 (16:59 +0200)]
bgpd: rework icmp enumerate list
As the other enumerate list, icmp type and code are handled as the other
combinations. The icmp type and code options are the last options to be
injected into PBR. If icmp type is present only, all the filtering will
apply to this icmp type. if icmp code is present only, then all the
combination will be done with icmp type ranging from 0 to 255 values.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Wed, 20 Jun 2018 13:30:40 +0000 (15:30 +0200)]
bgpd: fix recursive call combination
The recursive algorithm was taking into account the fact that all the
bpof structures were filled in. Because the dscp value was not given,
the pkt_len parsing could not be achieved. Now the iteration takes into
account each type according to the previous one, thus guaranting all
parameters to be parsed.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Wed, 20 Jun 2018 11:55:20 +0000 (13:55 +0200)]
bgpd: support for flowspec fragment list into policy routing
The flowspec fragment attribute is taken into account to be pushed in
BGP policy routing entries. Valid values are enumerate list of 1, 2, 4,
or 8 values. no combined value is supported yet.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Wed, 20 Jun 2018 06:32:43 +0000 (08:32 +0200)]
bgpd: align fragment flowspec decoding with tcpflags
As fragment bitmask and tcpflags bitmask in flowspec protocol is encoded
in the same way, it is not necessary to differentiate those two fields.
Moreover, it overrides the initial fragment limit set to 1. It is now
possible to handle multiple framgent values.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Mon, 18 Jun 2018 09:18:21 +0000 (11:18 +0200)]
bgpd: extend enumerate API to handle or operations
The flowspec enumerate list can either be and values or or values.
In the latter case, a list is created that will be used later.
Also, the API supports the check for both and or or operations. This API
does not permit to handle both and and or operations at the same time.
The list will have to be either and or or. An other API retrieves the
operator unary value that is used: and or or. or 0 is the two operators
are used at the same time.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Wed, 13 Jun 2018 09:56:35 +0000 (11:56 +0200)]
bgpd: do not add default route for flowspec for each FS entry
Because the Flowspec entries are parsed first, then injected to Zebra,
there are cases where the install feedback from zebra is not received.
This leads to unnecessary add route events, whereas one should be
enough.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Tue, 12 Jun 2018 12:45:35 +0000 (14:45 +0200)]
bgpd: simplify API in BGP policy-routing to handle Flowspec
To handle FS params between FS RIB and BGP PBR entities, a structure
intermediate named bgp_pbr_filter is used, and contains all filtering
information that was before passed as a parameter.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Mon, 11 Jun 2018 13:30:11 +0000 (15:30 +0200)]
zebra: add packet length into pbr support
The packet length is added to iptable zapi message.
Then the iptable structure is taking into account the pkt_len field.
The show pbr iptable command displays the packet length used if any.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
It is possible for flowspec entries containing ICMP rule to insert PBR
entries based on ICMP type and ICMP code.
Flowspec ICMP filtering can either have icmp type or icmp code or both.
Not all combinations are permitted:
- if icmp code is provided, then it is not possible to derive the
correct icmp value. This will not be installed
- range of ICMP is authorised or list of ICMP, but not both.
- on receiving a list of ICMPtype/code, each ICMP type is attempted to
be associated to ICMP code. If not found, then ICMPtype is combined
with all known ICMP code values associated to that ICMP type.
- if a specific ICMP type/code is needed, despite the ICMP code/type
combination does not exist, then it is possible to do it by forging a
FS ICMP type/code specific for that.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
G. Paul Ziemba [Sun, 24 Jun 2018 19:39:03 +0000 (12:39 -0700)]
bgpd: don't nexthop-track twice-leaked routes that came from zebra
Issue 2381: interface based routes not marked "up" when they originate
in zebra, redistributed to bgp vrf, then imported to vpn and then
imported by another vrf.
Routes that are redistributed into BGP from zebra should not get
nexthop tracking (the assumption is that the originating protocol
is responsible to export or withdraw the route according to its own
notion of nexthop status).
The vpn-vrf route-leaking code checks the source route sub_type to
decide whether to use nexthop tracking on the resulting leaked route.
A route that is redistributed from zebra into bgp will have
sub_type==BGP_ROUTE_REDISTRIBUTE. If it is leaked to the vpn RIB,
the resulting vpn RIB route will have sub_type==BGP_ROUTE_IMPORTED.
If THAT vpn route is leaked to another vrf, the original code will
examine only the leak-source route sub_type and, since it is
not BGP_ROUTE_REDISTRIBUTE, will wrongly try to use nexthop tracking
on the new route in the final vrf.
This change modifies the leak function to track back up the
parent links to the ultimate parent of the leak source route
and look at that route's sub_type instead.
Donald Sharp [Fri, 22 Jun 2018 11:46:03 +0000 (07:46 -0400)]
zebra: Gather up output data and push in buffer to send
This commit gathers data from the client->obuf_fifo and
puts it all into the buffer for writing.
This will address the fact that the multiple events
created caused the memory of zebra to grow to
unrealistic levels of usage when we are redistributing
data to other protocols.
Recreate Memory:
robot# show memory
Memory statistics for zebra:
System allocator statistics:
Total heap allocated: 1930 MiB
Holding block headers: 16 MiB
Used small blocks: 0 bytes
Used ordinary blocks: 1911 MiB
Free small blocks: 1968 bytes
Free ordinary blocks: 19 MiB
Ordinary blocks: 5210
Small blocks: 58
Holding blocks: 1
New Zebra Memory:
Memory statistics for zebra:
System allocator statistics:
Total heap allocated: 478 MiB
Holding block headers: 16 MiB
Used small blocks: 0 bytes
Used ordinary blocks: 415 MiB
Free small blocks: 1968 bytes
Free ordinary blocks: 63 MiB
Ordinary blocks: 4909
Small blocks: 58
Holding blocks: 1
New show threads cpu for Zebra:
robot# show thread cpu
Thread statistics for zebra:
Showing statistics for pthread main
-----------------------------------
CPU (user+system): Real (wall-clock):
Active Runtime(ms) Invoked Avg uSec Max uSecs Avg uSec Max uSecs Type Thread
0 6465.766 801 8072 54775 10810 225356 T work_queue_run
1 0.096 4 24 37 24 38 R vtysh_accept
0 8.690 533 16 54 154 6286 W zserv_flush_data
0 254.102 290 876 2224 971 6958 W zserv_write
0 1992.936 7854 253 115333 266 116288 E &zserv_process_messages
1 1.351 6 225 245 226 249 R zebra_accept
1 0.152 8 19 22 19 23 T zebra_ptm_connect
1 0.124 7 17 24 18 24 R kernel_read
1 121.460 122 995 107273 1021 108707 R vtysh_read
6 686.460 7854 87 150 93 6006 R zserv_read
0 0.040 1 40 40 39 39 T zebra_route_map_update_timer
0 0.412 6 68 170 499 1520 T if_zebra_speed_update
Fixes: #2527 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Pascal Mathis [Tue, 19 Jun 2018 16:25:55 +0000 (18:25 +0200)]
tests: Fix compatibility with FRR v5.0
This commit introduces a small fix to the peer-attributes test suite to
make it compatible with the slightly different API in FRR v5.0 for
bgp_init(), which requires that an explicit instance ID gets passed.
Pascal Mathis [Thu, 14 Jun 2018 17:40:36 +0000 (19:40 +0200)]
bgpd: Fix crash when showing filtered routes
This commit fixes the issue mentioned in #2419, which is caused by a
double-free. The problem of the current implementation is that
*bgp_input_modifier* already frees the passed attributes under specific
circumstances, which can then lead to a double-free as *bgp_attr_undup*
does not check if the attributes are set to NULL.
As it is not transparent to the function caller if the attributes get
freed or not and the similar function *bgp_output_modifier* also does
not flush the passed attributes, the line has been removed altogether.
All callers of *bgp_input_modifier* already deal by themself with
freeing/flushing/unduping BGP attributes, so it is safe to remove.
Pascal Mathis [Thu, 14 Jun 2018 16:03:33 +0000 (18:03 +0200)]
bgpd: Finalize group-overrides for peer/AF attrs
This commit finalizes the previous commits which introduced a generic
approach for making all BGP peer and address-family attributes
overrideable by keeping track of the configuration origin in separate
internal structures.
First of all, the test suite was greatly extended to also check the
internal data structures of peer/AF attributes, so that inheritance for
internal values like 'peer->weight' is also being checked in all cases.
This revealed some smaller issues in the implementation, which were also
fixed in this commit. The test suite now fully passes and covers all the
usual situations that should normally occur.
Pascal Mathis [Wed, 13 Jun 2018 17:34:17 +0000 (19:34 +0200)]
bgpd: Implement group-overrides for peer attrs
This commit introduces BGP peer-group overrides for the last set of
peer-level attrs which did not offer that feature yet. The following
attributes have been implemented: description, local-as, password and
update-source.
Each attribute, with the exception of description because it does not
offer any inheritance between peer-groups and peers, is now also setting
a peer-flag instead of just modifying the internal data structures. This
made it possible to also re-use the same implementation for attribute
overrides as already done for peer flags, AF flags and AF attrs.
The `no neighbor <neigh> description` command has been slightly changed
to support negation for no parameters, one parameter or * parameters
(LINE...). This was needed for the test suite to pass and is a small
change without any bigger impact on the CLI.
Pascal Mathis [Wed, 13 Jun 2018 00:34:43 +0000 (02:34 +0200)]
bgpd: Implement group-overrides for peer timers
This commit implements BGP peer-group overrides for the timer flags,
which control the value of the hold, keepalive, advertisement-interval
and connect connect timers. It was kept separated on purpose as the
whole timer implementation is quite complex and merging this commit
together with with the other flag implementations did not seem right.
Basically three new peer flags were introduced, namely
*PEER_FLAG_ROUTEADV*, *PEER_FLAG_TIMER* and *PEER_FLAG_TIMER_CONNECT*.
The overrides work exactly the same way as they did before, but
introducing these flags made a few conditionals simpler as they no
longer had to compare internal data structures against eachother.
Last but not least, the test suite has been adjusted accordingly to test
the newly implemented flag overrides.
Pascal Mathis [Tue, 12 Jun 2018 22:39:19 +0000 (00:39 +0200)]
bgpd: Improve test suite for peer-group overrides
This commit introduces the current test suite for BGP peer-group
overrides by adding support for custom check handlers (which can check
internal data structures more thoroughly) and by fixing several small
mistakes and issues that slipped through. Also some parts of the code
have been cleaned up to avoid duplicate and/or hard-to-read code.
Additionally a first experimental check for a BGP peer attribute with
values (advertisement-interval <value>) has been added to the test
suite. As this test suite is currently not passing, it has not been
added to the python test caller.
This commit cleans up some ugly leftovers from previous flag-override
implementation and refactors the AF-flag override implementation to
match the same behavior the newly added peer-flag override
implementation has.
Pascal Mathis [Tue, 12 Jun 2018 15:09:49 +0000 (17:09 +0200)]
bgpd: Fix AF-attribute overrides when binding peer
The current implementation of the overrides for peer address-family
attributes suffered a bug, which caused all peer-specific attributes to
be lost when the peer was added to a peer-group which already had that
specific address-family active.
This commit extends the *peer_group2peer_config_copy_af* function to
respect overridden flags properly. Additionally, the arguments of the
macros *PEER_ATTR_INHERIT* and *PEER_STR_ATTR_INHERIT* have been
reordered to be more consistent and easy to read.
This commit also adds further test cases to the BGP peer attributes test
suite, so that this kind of error is being caught in future commits. The
missing AF-attribute *distribute-list* has also been added to the test
suite.