bgpd: merge branch tteras/master~3 (b304dcb)

route-map tidying + next-hop-self all

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: fix some bgp_update_main() attribute leaks

bgp_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>
bgpd: remove duplicate route-map extcommunity code

route_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 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_main/_rsclient() through bgp_attr_intern(), or
free'd through bgp_attr_flush().

This fixes Bugzilla #799, which is that bgpd otherwise crashes with a
double free.  The ecommunity got unintern'd first in the route-map set
command, then in bgp_update_receive().

Debugged-by: Milan Kocian <milon@wq.cz>
Reported-by: Florian S <florian@herrenlohe.de>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: fix memory leak on malformed attribute

When 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 IP endianness in debug message

inet_ntop expects network byte order.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: don't send NOTIFY twice for malformed attrs

Most 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>
*: merge branch stable/0.99.23

bgp extcommunity fixes from stable branch

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ripd: use only one constant for derivation

RIP_MAX_RTE is defined in ripd.h as 25 but is in fact the
result of a formula. More over it is not used in the code:
the code itself includes the fomula. This makes it un-clear
for maintenance.

Signed-off-by: Feng Lu <lu.feng@6wind.com>
Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
tests/bgpd: don't hardcode error number (fix f57000c)

f57000c ("bgpd: don't send NOTIFY twice for malformed attrs") introduces
BGP_ATTR_PARSE_ERROR_NOTIFYPLS as additional error code that implies the
caller should sent a NOTIFY and convert it to BGP_ATTR_PARSE_ERROR.
Sadly, the latter was hardcoded in bgp_mp_attr_test.c, which now didn't
consider the new value to be an error.

Make the testcase treat all nonzero values as error without discern.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
isisd: don't require IPv4 for adjacency

This was precluding isisd from IPv6-only operation; no adjacency would
come up unless there was IPv4 in parallel.

Reported-by: Martin Winter <mwinter@netdef.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
*: fix detection and usage of sys/cdefs.h

This header is non-standard (though present on many systems) and
there is no standard for what it should or should not define.

Remove it where it is not really needed. But add also a configure
check, so it can be used if available but otherwise fallback to
defining the needed macroes.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
zebra: fix struct msghdr initializers

struct msghdr field orders are not strictly specified in POSIX.
Improve portability by using designated initializer. This fixes
build against musl c-library where struct msghdr is POSIX
compliant (Linux kernel and glibc definitions are non-conforming).

As the result is also more readable, struct iovec initilizers
were also converted.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
build: do not assume glibc on linux

The whole IPv6 stack detection could need refactoring. But this
fixes the linux check to not assume glibc. Fixes build against
musl c-library.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: memmove needed in community_del_val

In bgpd/bgp_community_del_val memcpy is used for potentially overlapping
regions which is *not* safe. It may "work" in some cases but is not
guaranteed to work in all cases. The case that I saw fail was on an
x86_64 architecture with the number of bytes being moved/copied equal to
8.

The way the code is written the uint32_t pointers will always differ by
1, which is equivalent to a memcpy/memmove of regions that are 4 bytes
away from one another. So the code failed while copying an 8 byte region
to an address that is 4 bytes lower i.e. overlapping regions.

Interestingly, the same architecture had no problems with a 12 byte
copy.

When the code failed the communities were [200,300,400] and a call was
made to delete the 200 community. The result of this was an array that
looked like [400,400] which was uniquified to [400]. Of course the
expected result should have been [300, 400].

One additional point - in our production environment memmove would not
*link* without including <string.h> but in an isolated quagga git repo
this #include does not seem to be required and I see memmove is used in
vtysh.c without this #include either.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
isisd: type mix-up in 28a8cfc "don't require IPv4"

Whoops, these are in6_addrs, not prefix_ipv6... funnily enough, it does the
right thing either way, if it compiles, which it only does on Linux because
IN6_IS_ADDR_LINKLOCAL contains a cast to the right type.  On BSD there is no
such cast, hence it explodes on trying to compile, trying to access struct
members of in6_addrs while operating on prefix_ipv6...

Fixes: 28a8cfc ("isisd: don't require IPv4 for adjacency")
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
lib: unset ZEBRA_IFA_PEER if no dst addr present (BZ#801)

On OpenBSD, carp interfaces claim to be PtP interfaces with a 0.0.0.0/0
peer address.  We process those in zebra and try to send them to
clients, at which point they get encoded as all-0.  The client code,
however, decodes that to a NULL pointer instead of 0.0.0.0.  This later
turns into a SEGV when CONNECTED_PREFIX sees that ZEBRA_IFA_PEER is set
and tries to access the peer prefix.

This is a band-aid fix for stable/0.99.23, a long-term solution needs
some conceptual improvements on the entire thing.

(The usefulness of a PtP-to-0.0.0.0/0 is a separate question;  at this
point dropping the peer prefix seems the least intrusive solution.)

Reported-by: Laurent Lavaud <laurent.lavaud@ladtech.fr>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
*: merge branch stable/0.99.23

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ripd: 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
  rip_rte_process(), rip_redistribute_add() and rip_timeout() are
  significantly simplified.

* rip_zebra_ipv4_add() and rip_zebra_ipv4_delete() now can share
  the common code. The common part is moved to rip_zebra_ipv4_send().

Signed-off-by: Feng Lu <lu.feng@6wind.com>
Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ripd: allow to enable/disable the ECMP feature

Introduce a new command "[no] allow-ecmp" to enable/disable the
ECMP feature in RIP. By default, ECMP is not allowed.

Once ECMP is disabled, only one route entry can exist in the list.

* rip_zebra.c: adjust a debugging information, which shows the number
               of nexthops according to whether ECMP is enabled.
* ripd.c: rip_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, rip_ecmp_disable() is called to
          remove the multiple nexthops.
* ripd.h: Add a new member "ecmp" to "struct rip", 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: David Lamparter <equinox@opensourcerouting.org>