bgpd: don't send NOTIFY twice for malformed attrsMost of the attribute parsing functions were already sending a notify,
let's clean up the code to make it happen only once.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: fix memory leak on malformed attributeWhen bgp_attr_parse returns BGP_ATTR_PARSE_ERROR, it may already have
parsed and allocated some attributes before hitting that error. Free
the attr's data before returning.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: fix double free after extcommunity set (BZ#799)The route-map extcommunity set code was incorrectly assuming that it
owns the intern'd struct ecommunity reference. In reality, the intern'd
reference belongs to bgp_update_receive() and we're not supposed to
touch it in the route-map code.
Instead, like all the other set commands, we use a on-heap but
non-intern'd ecommunity to set the new value. This is then either
intern'd in bgp_update_m...
bgpd: remove duplicate route-map extcommunity coderoute_set_ecommunity_rt and _soo share almost all of their code.
Let's remove one of the redundant copies.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: fix some bgp_update_main() attribute leaksbgp_update_main() wasn't doing anything to release attribute values
set from route maps for two of its error paths. To fix, pull up the
appropriate cleanup from further down and apply it here.
bgp_update_rsclient() doesn't have the issue since it immediately
does bgp_attr_intern() on the results from bgp_{export,import}_modifier.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
*: nuke ^L (page feed)Quagga sources have inherited a slew of Page Feed (^L, \xC) characters
from ancient history. Among other things, these break patchwork's
XML-RPC API because \xC is not a valid character in XML documents.
Nuke them from high orbit.
Patches can be adapted simply by:
sed -e 's%^L%%' -i filename.patch
(you can type page feeds in some environments with Ctrl-V Ctrl-L)
Signed-off-by: David Lampar...
build: Quagga 0.99.23-rc1this is not a full release version, so neither release notes nor
documentation are updated yet. Also, signing the tag with my private
GPG key instead of the Quagga one.
isisd: ignore the unrecognized TLVsWhen processing LSPDUs, the unrecognized TLVs/sub-TLVs should be
silently ignored.
In parse_tlvs(), ISIS_WARNING is returned once an unrecognized TLV
exists. It breaks the processing in lsp_authentication_check() and
lsp_update_data(). So remove it.
Signed-off-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ripd: fix "show ip rip status" documentationThe command was mis-named in the documentation as "show ip protocols".
Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ripd & ripngd: avoid the zero interface metricThe interface metric is initialized to 0 in the commit db19c85:
zebra: set metric for directly connected routes via netlink to 0
Ripd and ripngd must be aware of it and avoid increase the
route metric by 0.
Signed-off-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
lib: remove redundant and incorrect sys/fcntl.h includePOSIX defines <fcntl.h>, <sys/fcntl.h> is the same thing. However,
it should not be used as it's existence can depend on C-library
implementation. E.g. musl gives warning if <sys/fcntl.h> is used.
Signed-off-by: Timo Teräs <timo.teras@iki.fi>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: fix crash when allowas-in is done on inactive peerWhen allowas-in is changed on a peer that is not up, BGP would crash
trying to do route_refresh. If peer is not up, there is no need
to do notification or send.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: efficient NLRI packing for AFs != ipv4-unicastISSUE:
Currently, for non-ipv4-unicast address families where prefixes are
encoded in MP_REACH/MP_UNREACH attributes, BGP ends up sending one
prefix per UPDATE message. This is quite inefficient. The patch
addresses the issue.
PATCH:
We introduce a scratch buffer in the peer structure that stores the
MP_REACH/MP_UNREACH attributes for non-ipv4-unicast families. This
enables us ...
bgpd: don't compare next-hop to router-idWhile announcing a path to a peer, the code currently compares the path's
next-hop with the peer's router-id. This can lead to problems as the router
IDs are unique only within an AS. Suppose AS 1 sends route with next-hop
10.1.1.1. It is possible that the speaker has an established BGP peering
with a router in AS 2 with router ID 10.1.1.1. The route will not be
advertised to that peer in AS 2....
vtysh: fix build against readline 6.3readline 6.3 removes some old deprecated funnily-named types. This
updates vtysh to use the new types so it builds again.
Reported-by: Joel Teichroeb <klusark@archlinux.invalid>
References: https://bugs.archlinux.org/task/39495
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
zebra: raise the privileges before calling socket()Because of recent changes when creating AF_NETLINK socket, kernel will
cache capabilities of the caller and if file descriptor is used or
otherwise handed to another process it will check that current user has
necessary capabilities to use the socket. Hence we need to ensure we
have necessary capabilities when creating the socket and at the time we
use the socket.
See: http://www.spinics.net/l...
bgpd: Fix condition allowas-in in rsclient codeCurrently when you set neighbour's 'allowas-in' option on route server side
you get redistribution of the prefixes from this neighbour's table into all
neighbour's tables which have the same AS number. I think that wanted behaviour
is to allow import prefixes from neighbour's tables with the same AS num
into neighbour which has 'allowas-in' option set.
Signed-off-by: Milan Kocian <milon@wq.cz>...
bgpd: support TTL-security with iBGPTraditionally, ttl-security feature has been associated with EBGP
sessions as those identify directly connected external peers. The
GTSM RFC (rfc 5082) does not make any restrictions on type of
peering. In fact, it is beneficial to support ttl-security for both
EBGP and IBGP sessions. Specifically, in data centers, there are
directly connected IBGP peerings that will benefit from the protection...
bgpd: factor out eBGP multihop checkThe check for an eBGP multihop configuration is unwieldy; factor it out
into a separate function.
[DL: originally by Dinesh G Dutt <ddutt@cumulusnetworks.com>,
split off from the next commit]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: factor out TTL settingTTL/min TTL are set from both bgp_accept() and bgp_connect(). Factor
them out so the following change to enable iBGP GTSM becomes more
readable.
[DL: originally by Dinesh G Dutt <ddutt@cumulusnetworks.com>,
split off from the next commit]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: fix fast external fallover behaviorISSUES
1. When an interface goes down, the zclient callbacks are invoked
in the following order: (a) address_delete() that removes the
connected address list: ifp->connected, (b) interface_down()
that performs "fast external fallover" operation. The operation
relies on ifp->connected to look for peers that should be brought
down. That's a cyclic dependency.
2. 'ttl-security' co...
bgpd: increase TCP socket buffer sizeBGP does not respond fairly in high scale. As the number of BGP peers
and prefixes increase, triggers like interface flaps which lead to BGP
peer flaps, cause blockage in bgp_write.
BGP does handle the cases of TCP socket buffer full by queuing a write
event back, there is no functional issue there as such. Still,
increasing the peer socket buffer size should help reduce event queueing
in BGP...
bgpd: fix O_NONBLOCK on outgoing connectsBGP was setting sockets to be non-blocking only for the accepted passive
peers. As a fix, setting the BGP sockets to be non-blocking even for
the active peers.
Signed-off-by: Vipin Kumar <vipin@cumulusnetworks.com>
Reviewed-by: Pradosh Mohapatra <pmohapat@cumulusnetworks.com>
Reviewed-by: Dinesh Dutt <ddutt@cumulusnetworks.com>
[DL: patch split, this is item 1.]
Signed-off-by: David Lamparter...
bgpd: send notify in OpenSent when stopping manuallyThe issue it fixes is that the notification message is not sent to a
second peer when bgp is stopped manually.
According to BGP RFC4271, section 8.2.2, regarding the FSM transitions,
in OpenSent state:
If a ManualStop event (Event 2) is issued in the OpenSent state, the
local system:
* sends the NOTIFICATION with a Cease,
* sets the ConnectRetryTimer to zero,
* releases all BGP resourc...
bgpd: display multipath status in "show ip bgp"The output of "show ip bg" does not show whether and which routes are
installed as multipath routes along the best route:
BGP table version is 0, local router ID is 10.10.100.209
Status codes: s suppressed, d damped, h history, * valid, > best, i - internal,
r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete
Network Next Hop M...
bgpd: track correct originator-id in reflected routesISSUE:
Suppose route1 and route2 received from route-reflector-client1 and client2
respectively have identical attributes. The current logic of creating the
adj-rib-out for a peer threads the 'adv' structures for both routes against
the same attribute. This results in 'bgp_update_packet()' to pack those
routes in the same UPDATE message with one attr structure formatted. The
originator-id is t...
bgpd: add 'bgp bestpath as-path multipath-relax'Compute multipath in BGP based on AS_PATH hop count match. If the knob
is turned on, it is not required to have an exact match of AS_PATHs
(provided other multipath conditions are met, of course).
Signed-off-by: Pradosh Mohapatra <pmohapat at cumulusnetworks.com>
Reviewed-by: Dinesh G Dutt <ddutt at cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: use ATTR_FLAG_BIT() for BGP_ATTR_ values* bgp_attr.c: this UNSET_FLAG()s are bogus. I did a quick review and
I think that they could not cause any bug anyway.
Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
Acked-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ospfd: add debug messages for router lsa-generationAdd log messages to lsa_link_broadcast_set so it becomes more
apparent why a particular broadcast interface was added as
transit or stub interface.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ospfd: For an ABR, ensure the right LSID is MaxAge'dPROBLEM:
Accurate garbage collection of maxage LSAs. The global OSPF structure has
a maxage_lsa tree - the key to the tree is <ls-id, adv-router> tuple. Suppose
the ABR has multiple areas and has originated some intra-area LSAs. The
key for all those LSAs is the same. The code then ends up in a state where
all but the first LSA do not get cleaned up from the areas' LSDB. A subsequent
event wou...
ospfd: clarify indentation and comments in ospf_lsa_maxage_deleteSigned-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ospfd: fix a reference counting issue introduced by commit 4de8bf0011Commit 4de8bf0011 added a return statement to a loop iterating over a
route_table. That loop uses route_top/route_next.
As commit 4de8bf0011 failed to add a route_node_unlock before the
return statement, a reference is leaked when this codepath is taken.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@...
ospfd: check the LS-Ack's recentness instead of only comparing the #seqISSUE:
RTA(DR)-----(BackupDR)RTB
RTA advertises a new LSA to RTB, and then flushes the LSA (with setting
the age of the LSA to MaxAge) within 1 second. Then the LSA is deleted
from RTA, while it still exists on RTB with non-MaxAge and can not be
flushed any more.
FIX:
The reason can be explained in below:
a) RTA -- new LSA, #seq=1 --> RTB (RTB will send the delayed Ack in 1s)
b) RTA -- Max...
ospfd: don't allow to set network type on loopback interfacesOSPFd only allocates some stub information for loopback interfaces.
This causes a crash when the interface state machine is started on
that interface by configuring a different network type.
It doesn't make much sense to configure the network type of a loopback
interface, therefore, just forbid it.
See also bugzilla #670.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-o...
ospfd: run DR election prior to LSA regenerationThe results from DR election are used when constructing router-LSAs.
E.g. they are used to determine whether a broadcast interface should
be added with a link type of stub interface or transit interface.
Therefore, we should run DR election prior before regenerating LSAs.
Before commit c363d3861b5384a31465a72ddc3b0f6ff007a95a the DR election
was called synchronously prior to router-LSA regene...
zebra: Change the mechanism for comparing route ID's.The current format uses subtraction of two ints. Unfortunately, the
subtraction method does not work for all combinations of numbers.
For example, the with numbers represented by 10.x.x.x and 192.x.x.x,
10.x.x.x - 192.x.x.x will yield a very large positive number indicating
that 10.x.x.x is larger.
Signed-off-by: Ken Williams <kenneth.j.williams@intel.com>
Acked-by: Feng Lu <lu.feng@6wind.com>...
lib: use heap to manage timersSigned-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
tests: Add tests for timersSigned-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
lib/command.c: rewrite command matching/parsingAdd support for keyword commands.
Includes new documentation for DEFUN() in lib/command.h, for preexisting
features as well as new keyword specification.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
tests: fix build & disable testcommandsThe perils of having tests, the test wasn't tested thoroughly enough...
Fixup various automake problems, and then disable it since it depends on
configure parameters in its current version.
For 0.99.24 we can ship a static copy of vtysh_cmd.c and have it
reenabled.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
tests: add a test program for lib/command.cSigned-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd, ospfd, zebra: fix some DEFUN definitionsFixup some DEFUNS with incorrect command strings or mixed up helpstrings.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
zebra: apply syntactic sugar to rib_dump()strip the explicit __func__ present on all calls and make the prefix
argument a transparent union.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
zebra: set metric for directly connected routes via netlink to 0a value of 1 is hard coded for the metric field, much like the
ifconfig utility it may have roots in. in order to be in line
with the metric used in the linux kernel itself, we switch this
to 0.
Signed-off-by: Brett Ciphery <brett.ciphery@windriver.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
build: remove now-useless --{en, dis}able-testscommit d771020 "don't build tests unless make check is run" has made the
--{en,dis}able-tests switch completely useless. The differentiation is
now made by running "make check" or not doing so. The only effect of
the switch is an "empty" excursion of make into the tests/ directory.
(well, and it turns "make check" useless from the main directory if
--disable-tests is given, which I don't t...