Issue: bgpd is not replaying the BFD registrations to PTM after quagga restart.
Root Cause: This problem happens when BFD configuration is part of the peer group template. Currently, the BFD configuration is being copied to the peer from template as part of the AF (address family) configuration. But, when the saved config is used after the quagga restart the peer group template is applied to the peer before the AF configuration is configured for the template. Due to this the BFD configuration never gets copied from the template to the peer and the BGP peers have no BFD configuration after the restart
Donald Sharp [Thu, 19 May 2016 13:56:35 +0000 (09:56 -0400)]
lib: Fix some poll semantics
Two Fixes:
1) When a fd has both read and write as a .events.
(POLLHUP | POLLIN | POLLOUT) and a
thread_cancel_read_write call is executed
from a protocol, the code was blindly removing
the fd from consideration at all.
2) POLLNVAL was being evaluated before POLLIN|POLLOUT
were being evaluated. While I didn't see a case
of POLLNVAL being included with other .revent flags
I decided to move the POLLNVAL and POLLHUP handling
to the same section of code.
Additionally the function thread_cancel_read_write
was poorly named and let me to poorly implement
the poll version of it. I've renamed the function
thread_cancel_read_or_write in an attempt to
make this problem moot in the future.
Don Slice [Fri, 10 Jun 2016 13:58:03 +0000 (06:58 -0700)]
bgpd: remove vrf->iflist deleted to avoid a crash
Ticket: CM-11327 Signed-off-by: Don Slice Reviewed-by: Donald Sharp
Testing Done: Manual testing, bgp-min, vrf-min, bgp-smoke, vrf-smoke all successful
When bgp was configured in a vrf and then deleted, the vrf->iflist
was being deleted from the vrf. Since the vrf itself was not deleted,
it was assumed in later calls that the vrf->iflist was still there
and when it was referenced, the crash occurred.
Sam Tannous [Thu, 9 Jun 2016 16:26:06 +0000 (12:26 -0400)]
vtysh json output not JSON for show ip bgp neigh json
Ticket: CM-11350
Reviewed By: dsharp
Testing Done: built amd64 images and tested output of both json and non-json
Upstream patch was applied in wrong section of code so JSON
output contained plain text. The upstream patch was
commit baa376fc1 (cherry picked from ef757700d0f)
This patch moves the text output to the correct if clause
and also adds a new JSON line for the same data.
Signed-off-by: Sam Tannous <stannous@cumulusnetworks.com> Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
David Lamparter [Tue, 15 Sep 2015 08:53:09 +0000 (01:53 -0700)]
*: make sure zebra.h is always included first
zebra.h pulls in config.h, which results in fiddling with things like
__FILE_OFFSET_BITS. It must always be included first, in order to set
flags that influence the compiler via <features.h>.
Donald Sharp [Thu, 17 Sep 2015 14:54:25 +0000 (10:54 -0400)]
lib/zclient: Consolidate error reporting for zclient_read_header
All functions that call zclient_read_header immediately turn around
and check to ensure that the version and marker fields are correct
Move this code into zclient_read_header
Michael Rossberg [Mon, 27 Jul 2015 05:56:25 +0000 (07:56 +0200)]
ospfd: Fast OSPF convergence
When considering small networks that have extreme requirements on
availability and thus convergence delay, the timers given in the OSPF RFC
seem a little “conservative”, i.e., the delay between accepted LSAs and the
rate at which LSAs are sent. Cisco introduced two commands 'timers throttle
lsa all’ and 'timers lsa arrival’, which allow operators to tune these
parameters.
I have been writing a patch to also support 'timers lsa arrival’ fully and
‘timers throttle lsa all’ (without the throttling part) also in quagga.
Feng Lu [Fri, 22 May 2015 09:39:55 +0000 (11:39 +0200)]
ospf6d, bgpd: avoid calling if_nametoindex
As the comments in if.h, it is better to call ifname2ifindex()
instead of if_nametoindex().
And ifname2ifindex() can work for VRF by appending a parameter
while if_nametoindex() can not.
Signed-off-by: Feng Lu <lu.feng@6wind.com> Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Acked-by: Vincent JARDIN <vincent.jardin@6wind.com> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 395828eea809e8b2b8c5824d3639cefedd7aa9f0)
Timo Teräs [Sat, 23 May 2015 08:08:41 +0000 (11:08 +0300)]
zebra: use prefix2str for logging where possible
This makes code more robust, consice and readable.
Signed-off-by: Timo Teräs <timo.teras@iki.fi> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit be6335d682c5ee1b6930345193eda875705fbab2)
Timo Teräs [Sat, 23 May 2015 08:08:40 +0000 (11:08 +0300)]
zebra/vty: use prefix2str and unify show ip/ipv6 route code
Use prefix2str where possible. As now ip/ipv6 are practically
identical, they are merged removing unneeded code duplication.
Signed-off-by: Timo Teräs <timo.teras@iki.fi> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 53a5c39c705f917567d5b1764f1fe12ad5c5e577)
Timo Teräs [Sat, 23 May 2015 08:08:39 +0000 (11:08 +0300)]
lib: make prefix2str simpler to use, and use it in zclient
Returning the buffer allows using it in the logging functions
in easier way. This also makes the API consistent with sockunion.
Add also PREFIX_STRLEN to be the generic buffer length required
for any prefix string representation.
Signed-off-by: Timo Teräs <timo.teras@iki.fi> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 41eb9a4305fbcb206c900a18af7df7115d857d60)
vivek [Thu, 9 Jun 2016 01:14:17 +0000 (18:14 -0700)]
bgpd: Do not process workqueue upon instance delete
During instance cleanup, an earlier patch walked the workqueue in order
to process queued routes of the instance. However, since the workqueue
is not per instance, the code walks and immediately processes all routes
across all instances.
This may not be ideal in the presence of VRFs, when multiple instances
will be a fact. Revert that part of the change from earlier patch. This
needs to be revisited later for a better solution.
Feng Lu [Fri, 22 May 2015 09:39:54 +0000 (11:39 +0200)]
ripngd: allow to enable/disable the ECMP feature
Introduce a new command "[no] allow-ecmp" to enable/disable the
ECMP feature in RIPng. By default, ECMP is not allowed.
Once ECMP is disabled, only one route entry can exist in the list.
* ripng_zebra.c: adjust a debugging information, which shows the number
of nexthops according to whether ECMP is enabled.
* ripngd.c: ripng_ecmp_add() will reject the new route if ECMP is not
allowed and some entry already exists.
A new configurable command "allow-ecmp" is added to control
whether ECMP is allowed.
When ECMP is disabled, ripng_ecmp_disable() is called to
remove the multiple nexthops.
* ripngd.h: Add a new member "ecmp" to "struct ripng", indicating whether
ECMP is allowed or not.
Signed-off-by: Feng Lu <lu.feng@6wind.com> Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Acked-by: Vincent Jardin <vincent.jardin@6wind.com> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 72855b16b72e9ad2c7eb0c0bfd8f5985f779608f)
Feng Lu [Fri, 22 May 2015 09:39:53 +0000 (11:39 +0200)]
ripngd: add ECMP support
* Each node in the routing table is changed into a list, holding
the multiple equal-cost paths.
* If one of the multiple entries gets less-preferred (greater
metric or greater distance), it will be directly deleted instead
of starting a garbage-collection timer for it.
The garbage-collection timer is started only when the last entry
in the list gets INFINITY.
* Some new functions are used to maintain the ECMP list. And hence
ripng_route_process(), ripng_redistribute_add() and ripng_timeout()
are significantly simplified.
* ripng_zebra_ipv6_add() and ripng_zebra_ipv6_delete() now can share
the common code. The common part is moved to ripng_zebra_ipv6_send().
Signed-off-by: Feng Lu <lu.feng@6wind.com> Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Acked-by: Vincent Jardin <vincent.jardin@6wind.com> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
As any new compiler version, clang 3.6 has new warnings, one of these
being that it now warns for testing whether the address of an array will
be true.
Of course there is no point in this check for the sysid, so let's always
just print the sysid.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 14 May 2015 12:24:06 +0000 (14:24 +0200)]
lib/vty: don't clear output buffer on input EOF
A VTY's input can be closed without the output becoming unavailable.
This happens both on stdio when stdin ends, as well as over TCP when an
unidirectional input shutdown() happens.
In such a case, resetting the output buffer is not appropriate since
there might still be data to be successfully written.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Tue, 12 May 2015 19:56:18 +0000 (21:56 +0200)]
lib/vty: add vty_stdio at-close hook
This is intended to be used for either "exit on close", "fork on close"
or "reopen vty on close" functionality for the stdio vty. Which of
these options to take depends on the context, the use case right now is
test programs exiting on EOF.
David Lamparter [Thu, 30 May 2013 14:33:45 +0000 (16:33 +0200)]
lib/vty: add vty_stdio()
this introduces a new public/API function to the vty code for opening a
VTY on stdin/stdout. Intended for unrestricted use by the individual
daemons, i.e. "offical API".
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 30 May 2013 14:31:49 +0000 (16:31 +0200)]
lib/vty: add separate output fd support to VTYs
to be used with stdin/stdout terminals, this adds support for writing to
a different FD than we're reading from. Also fixes error messages from
config load being written to stdin.
Paul Jakma [Fri, 23 Jan 2015 16:43:49 +0000 (16:43 +0000)]
ospfd: Remove another odd flooding hack in opaque LSA code
* ospf_opaque.c: (ospf_opaque_adjust_lsreq) Odd hack to general OSPF
database exchange but made to act only on opaque LSAs. It's either covering
up bugs in the flooding code or its wrong. If it's covering up bugs, those
would affect all LSAs and should be fixed at a lower layer in ospfd, indeed
perhaps those bugs are long fixed anyway (?). Alternatively, it's just plain
wrong. Nuke.
(ospf_opaque_exclude_lsa_from_lsreq) helper to above, nuke.
* ospf_packet.c: Nuke call to ospf_opaque_adjust_lsreq.
David Lamparter [Wed, 13 May 2015 10:44:50 +0000 (12:44 +0200)]
lib: assert(0) still needs a return
assert(0) is not guaranteed to not return since assert() in general can
be optimised out when building without debug / with optimisation. This
breaks the build in clang, which warns/errors about the missing return.
David Lamparter [Tue, 12 May 2015 15:18:04 +0000 (17:18 +0200)]
lib: fix "reduce strcmp in CLI" fallout (10bac801)
In "lib/cli: reduce strcmp in CLI hot paths", I failed to notice that
CMD_VARIABLE as a boolean test covers a superset of the other types of
variables. Thus, the patch broke processing of IP/IPv6/Integer range
parameters in the CLI.
Fix by some reordering and introducing TERMINAL_RECORD macro (which
marks whether a given terminal type is a parameter) to be used in places
where the check is really for all kinds of variables.
Reported-by: Timo Teräs <timo.teras@iki.fi> Tested-by: Martin Winter <mwinter@netdef.org> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Mon, 13 Apr 2015 07:50:00 +0000 (09:50 +0200)]
bgpd: speed up "no-hit" withdraws for routeservers
This accelerates handling of incoming Withdraw messages for routes that
don't exist in the table to begin with. Cisco IOS 12.4(24)T4 has a bug
in this regard - it sends withdraws instead of doing nothing for
prefixes that are filtered.
Pulling up the adj_in removal in Quagga should have no ill effect, but
we can avoid the costly iteration over all rsclients if there was no
adj_in entry.
Performance impact of this change on routeserver with 3 buggy peers,
startup/sync time:
before patch: 143.12 seconds (user cpu)
after patch: 7.01 seconds (user cpu)
Many thanks to Nick Hilliard & INEX for providing real-world test data!
Signed-off-by: David Lamparter <equinox@opensourcerouting.org> Acked-by: Paul Jakma <paul@jakma.org>
Paul Jakma [Tue, 20 Jan 2015 15:45:36 +0000 (15:45 +0000)]
ospfd: Remove the blocking of opaque LSAs origination & flooding 'optimisation'
* Opaque support contains some kind of hack/optimisation to
origination/flooding to suppress some origins/floods until an opaque LS
Acks are received. Previous versions of the code have already been shown
to have bugs in them (see e16fd8a5, e.g.). It seems over-complex and fragile,
plus its conceptually the wrong place to try implement flooding hacks that,
AFAICT, do not depend particularly on the semantics of opaque LSA.
David Lamparter [Tue, 5 May 2015 09:10:20 +0000 (11:10 +0200)]
lib/cli: reduce strcmp in CLI hot paths
Er, no idea how anyone could ever have thought that it would be a good
idea to have a zillion of strcmp() calls in the CLI's active paths, just
to compare against things like "A.B.C.D".
Reduces 40k prefix list load time from 1.65s to 1.23s (1.34:1).
Acked-by: Paul Jakma <paul@jakma.org>
[v2: killed CMDS_* macros] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 10bac80195cf5a781da6e4415e6580fd7080f734)
Timo Teräs [Wed, 29 Apr 2015 06:43:01 +0000 (09:43 +0300)]
lib: constify sockunion api
Add const to read-only api calls.
Signed-off-by: Timo Teräs <timo.teras@iki.fi> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 81b139bdd530adda045d22a4daf0054b89703dab)
David Lamparter [Tue, 3 Mar 2015 09:00:43 +0000 (10:00 +0100)]
build: add --enable-werror
This allows enabling -Werror in a consistent way. Note that this is
different from just specifiying it in CFLAGS, since that would either
break configure tests (if done on ./configure), or would override
configure's CFLAGS (if done on make).
Using --enable-werror instead provides a new WERROR variable that is
additionally used during make with a consistent set of warning flags.
The tests/ directory is exempt. (Rationale being, better to have more
tests than pedantically complain about them.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 4 Mar 2015 06:18:24 +0000 (07:18 +0100)]
vtysh: drop unused variables & RETSIGTYPE
Drop unused return values in vtysh. Also gets rid of the rather funny
prototyping of signal setup in vtysh - which as a side effect makes it
not need AC_TYPE_SIGNAL in configure.ac anymore. It wasn't used
sensibly to begin with...
David Lamparter [Tue, 3 Mar 2015 09:30:27 +0000 (10:30 +0100)]
ospf6d: use existing union, avoid strict-aliasing
There are preexisting fields u.lp.id and u.lp.adv_router in struct
prefix that do the same thing as these type-punning pointer derefs.
Use these and shut up the strict-aliasing warnings.
Donald Sharp [Wed, 8 Jun 2016 15:47:33 +0000 (11:47 -0400)]
pimd: Fix register message checksum
The register message checksum was being calculated over
the first 4 bytes of the packet, instead of the first
8 bytes. From the RFC:
PIM Version, Type, Reserved, Checksum
Described in Section 4.9. Note that in order to reduce
encapsulation overhead, the checksum for Registers is done only
on the first 8 bytes of the packet, including the PIM header and
the next 4 bytes, excluding the data packet portion. For
interoperability reasons, a message carrying a checksum
calculated over the entire PIM Register message should also be
accepted. When calculating the checksum, the IPv6 pseudoheader
"Upper-Layer Packet Length" is set to 8.
Ticket: CM-11265 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
kernel_delete_ipv6_old(), removed in 51bdeba a little while ago, was the
last user of netlink_route() and kernel_rtm_ipv6(). Everything else
uses the _multipath variants of these functions.
David Lamparter [Tue, 3 Mar 2015 07:54:54 +0000 (08:54 +0100)]
bgpd, zebra: fix struct/pointer sizeof mixups
Two places were taking sizeof(pointer) instead of the sizeof(struct),
while performing operations on the struct. Both are initialisation
functions; I guess we haven't seen fallout since they weren't critical.
Fix anyway.
[v2: fix mistake that actually broke bgpd RS workqueue init] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit d43f8b39b075fe60e0c8fdb33b07b284d3fae503)
David Lamparter [Tue, 3 Mar 2015 08:07:25 +0000 (09:07 +0100)]
*: add/cleanup initialisers
There were some (inconsequential) warnings about uninitialised use of
variables. Also, in one case, sub-structs were mixed in initialisation,
which doesn't quite work as intended.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Lou Berger [Tue, 12 Jan 2016 18:41:55 +0000 (13:41 -0500)]
bgpd: wire up VPNv6 protocol processing
There wasn't much missing for VPNv6 to begin with; just a few bits of
de- & encoding and a few lists to be updated.
Signed-off-by: Lou Berger <lberger@labn.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
[Editorial note: Signed-off-by may imply an authorship claim, but need not]
Lou Berger [Tue, 12 Jan 2016 18:41:46 +0000 (13:41 -0500)]
lib: fix bookkeeping for libreadline malloc()s
When libreadline is used, we mistakenly mix in strdup() done in
libreadline with Quagga's lib/memory bookkeeping/counting, leading to
counter underflows on MTYPE_TMP.
Signed-off-by: Lou Berger <lberger@labn.net> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 672900382d47137638086bd8351b2678f589a546)
Donald Sharp [Thu, 7 Jan 2016 14:33:28 +0000 (09:33 -0500)]
lib, bgpd: Fixup afi_t to be an enum and cleanup zebra.h
This code change does two things:
1) Removes ZEBRA_AFI_XXX #defines since they were redundant information
2) Switches afi_t to an enumerated type so that the compiler
can do a bit more compile time checking.
Paul Jakma [Tue, 1 Dec 2015 14:32:11 +0000 (14:32 +0000)]
bgpd: bgp_scan shouldn't queue up route_nodes with no routes for processing
* bgp_nexthop.c: (bgp_scan) There is little point queueing an rn with no routing
information for processing.
* bgp_route.c: (bgp_process) Do nothing on rn's with no routes. Add an assert
for now, to try catch any other cases, but prob should be removed.
(bgp_best_selection) rn with no routes == finish early.
Paul Jakma [Wed, 25 Nov 2015 17:14:35 +0000 (17:14 +0000)]
bgpd: Check capability falls on right multiple of size, where possible.
* bgp_open.c: (cap_modsizes) Table of multiple a capability's data size
should fall on, if applicable.
(bgp_capability_parse) Check the header lengthcap_modsizes should fall on.
Inspiration from Cumulus bgpd-capability-cleanup.patch patch, with a
slightly different approach.
Timo Teräs [Thu, 22 Oct 2015 08:35:18 +0000 (11:35 +0300)]
bgpd: update rtt on soft clear
rtt is calculated dynamically by the kernel. Refresh it on
soft clear.
Fixes: ef757700d0 "bgpd: allow using rtt in route-map's set metric" Signed-off-by: Timo Teräs <timo.teras@iki.fi>
(cherry picked from commit 5a2a1ec18c89daec5de6690a9b0f47c0d11a0f2d)
Timo Teräs [Thu, 22 Oct 2015 08:35:17 +0000 (11:35 +0300)]
bgpd: check rtt later after the real peer is known
OPEN message handler moves the connection from the temporary
"struct peer" (used to accept it) to the real "struct peer" based
on the configuration. RTT needs to be updated only to the real
struct peer, and this patch moves the RTT query to point where
realpeer is known.
Fixes: ef757700d0 "bgpd: allow using rtt in route-map's set metric" Signed-off-by: Timo Teräs <timo.teras@iki.fi>
(cherry picked from commit 0edba8b6ad9c83fa0a3cc58765fe9f123f4109ac)
Timo Teräs [Wed, 24 Jun 2015 12:27:21 +0000 (15:27 +0300)]
bgpd: Make bgp_info_cmp robust to paths that do not have su_remote info
My original su_remote == NULL check is not correct. It seems that
* bgp_route.c: (bgp_info_cmp) Some bgp_info is compared with su_remote=NULL
and it's supposed to be perfectly legal. E.g. configured subnet announces
("network a.b.c.d/n"). Ensure bgp_info_cmp is robust if such a path gets
as far as the neighbour address comparison step.
Timo Teräs [Fri, 22 May 2015 10:41:00 +0000 (13:41 +0300)]
zebra: simplify redistribution code
Merge the conditionals as one to avoid code duplication.
Signed-off-by: Timo Teräs <timo.teras@iki.fi> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit f85592e05ae6463727433893e61afd1081fcf7e0)