From 8b16ed74fa61523c3348d2584b66a56a8ad4e350 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 6 Jul 2014 22:33:48 +0200 Subject: [PATCH 1/8] 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 --- tests/bgp_mp_attr_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bgp_mp_attr_test.c b/tests/bgp_mp_attr_test.c index 177c1ad8dc..aa8e485d6e 100644 --- a/tests/bgp_mp_attr_test.c +++ b/tests/bgp_mp_attr_test.c @@ -478,7 +478,7 @@ parse_test (struct peer *peer, struct test_segment *t, int type) printf ("parsed?: %s\n", ret ? "no" : "yes"); - if (ret != t->parses) + if ((ret == 0) != (t->parses == 0)) failed++; if (tty) From 28a8cfcbc3a5cc74bb3b87981b878f8b4edc2dd6 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 29 Jun 2014 13:48:18 +0200 Subject: [PATCH 2/8] 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 Signed-off-by: David Lamparter --- isisd/isis_pdu.c | 89 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 10 deletions(-) diff --git a/isisd/isis_pdu.c b/isisd/isis_pdu.c index 8a92789ff1..5f18135e87 100644 --- a/isisd/isis_pdu.c +++ b/isisd/isis_pdu.c @@ -402,6 +402,7 @@ process_p2p_hello (struct isis_circuit *circuit) u_int32_t expected = 0, found = 0, auth_tlv_offset = 0; uint16_t pdu_len; struct tlvs tlvs; + int v4_usable = 0, v6_usable = 0; if (isis->debugs & DEBUG_ADJ_PACKETS) { @@ -518,11 +519,44 @@ process_p2p_hello (struct isis_circuit *circuit) /* * check if it's own interface ip match iih ip addrs */ - if ((found & TLVFLAG_IPV4_ADDR) == 0 || - ip_match (circuit->ip_addrs, tlvs.ipv4_addrs) == 0) + if (found & TLVFLAG_IPV4_ADDR) + { + if (ip_match (circuit->ip_addrs, tlvs.ipv4_addrs)) + v4_usable = 1; + else + zlog_warn ("ISIS-Adj: IPv4 addresses present but no overlap " + "in P2P IIH from %s\n", circuit->interface->name); + } +#ifndef HAVE_IPV6 + else /* !(found & TLVFLAG_IPV4_ADDR) */ + zlog_warn ("ISIS-Adj: no IPv4 in P2P IIH from %s " + "(this isisd has no IPv6)\n", circuit->interface->name); + +#else + if (found & TLVFLAG_IPV6_ADDR) + { + /* TBA: check that we have a linklocal ourselves? */ + struct listnode *node; + struct prefix_ipv6 *ip; + for (ALL_LIST_ELEMENTS_RO (tlvs.ipv6_addrs, node, ip)) + if (IN6_IS_ADDR_LINKLOCAL (ip)) + { + v6_usable = 1; + break; + } + + if (!v6_usable) + zlog_warn ("ISIS-Adj: IPv6 addresses present but no link-local " + "in P2P IIH from %s\n", circuit->interface->name); + } + + if (!(found & (TLVFLAG_IPV4_ADDR | TLVFLAG_IPV6_ADDR))) + zlog_warn ("ISIS-Adj: neither IPv4 nor IPv6 addr in P2P IIH from %s\n", + circuit->interface->name); +#endif + + if (!v6_usable && !v4_usable) { - zlog_warn ("ISIS-Adj: No usable IP interface addresses " - "in LAN IIH from %s\n", circuit->interface->name); free_tlvs (&tlvs); return ISIS_WARNING; } @@ -859,6 +893,7 @@ process_lan_hello (int level, struct isis_circuit *circuit, u_char * ssnpa) struct tlvs tlvs; u_char *snpa; struct listnode *node; + int v4_usable = 0, v6_usable = 0; if (isis->debugs & DEBUG_ADJ_PACKETS) { @@ -1045,14 +1080,48 @@ process_lan_hello (int level, struct isis_circuit *circuit, u_char * ssnpa) /* * check if it's own interface ip match iih ip addrs */ - if ((found & TLVFLAG_IPV4_ADDR) == 0 || - ip_match (circuit->ip_addrs, tlvs.ipv4_addrs) == 0) + if (found & TLVFLAG_IPV4_ADDR) { - zlog_debug ("ISIS-Adj: No usable IP interface addresses " - "in LAN IIH from %s\n", circuit->interface->name); - retval = ISIS_WARNING; - goto out; + if (ip_match (circuit->ip_addrs, tlvs.ipv4_addrs)) + v4_usable = 1; + else + zlog_warn ("ISIS-Adj: IPv4 addresses present but no overlap " + "in LAN IIH from %s\n", circuit->interface->name); } +#ifndef HAVE_IPV6 + else /* !(found & TLVFLAG_IPV4_ADDR) */ + zlog_warn ("ISIS-Adj: no IPv4 in LAN IIH from %s " + "(this isisd has no IPv6)\n", circuit->interface->name); + +#else + if (found & TLVFLAG_IPV6_ADDR) + { + /* TBA: check that we have a linklocal ourselves? */ + struct listnode *node; + struct prefix_ipv6 *ip; + for (ALL_LIST_ELEMENTS_RO (tlvs.ipv6_addrs, node, ip)) + if (IN6_IS_ADDR_LINKLOCAL (ip)) + { + v6_usable = 1; + break; + } + + if (!v6_usable) + zlog_warn ("ISIS-Adj: IPv6 addresses present but no link-local " + "in LAN IIH from %s\n", circuit->interface->name); + } + + if (!(found & (TLVFLAG_IPV4_ADDR | TLVFLAG_IPV6_ADDR))) + zlog_warn ("ISIS-Adj: neither IPv4 nor IPv6 addr in LAN IIH from %s\n", + circuit->interface->name); +#endif + + if (!v6_usable && !v4_usable) + { + free_tlvs (&tlvs); + return ISIS_WARNING; + } + adj = isis_adj_lookup (hdr.source_id, circuit->u.bc.adjdb[level - 1]); if ((adj == NULL) || (memcmp(adj->snpa, ssnpa, ETH_ALEN)) || From 16ffb26fbbf8b3d1fee7a14eb401ecb02eed5058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Tue, 29 Jul 2014 09:41:54 +0000 Subject: [PATCH 3/8] *: fix detection and usage of sys/cdefs.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: David Lamparter --- configure.ac | 2 +- isisd/include-netbsd/iso.h | 2 +- lib/queue.h | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 220124ab8e..3dba3b3577 100755 --- a/configure.ac +++ b/configure.ac @@ -442,7 +442,7 @@ dnl Check other header files. dnl ------------------------- AC_CHECK_HEADERS([stropts.h sys/ksym.h sys/times.h sys/select.h \ sys/types.h linux/version.h netdb.h asm/types.h \ - sys/param.h limits.h signal.h \ + sys/cdefs.h sys/param.h limits.h signal.h \ sys/socket.h netinet/in.h time.h sys/time.h]) dnl Utility macro to avoid retyping includes all the time diff --git a/isisd/include-netbsd/iso.h b/isisd/include-netbsd/iso.h index 1a80aec8e8..42b9bc88de 100644 --- a/isisd/include-netbsd/iso.h +++ b/isisd/include-netbsd/iso.h @@ -192,7 +192,7 @@ extern struct protosw isosw[]; #else /* user utilities definitions from the iso library */ -#ifdef SUNOS_5 +#ifndef HAVE_SYS_CDEFS_H #define __P(x) x #define __BEGIN_DECLS #define __END_DECLS diff --git a/lib/queue.h b/lib/queue.h index 70cffab1b2..48b363e24d 100644 --- a/lib/queue.h +++ b/lib/queue.h @@ -33,8 +33,6 @@ #ifndef _SYS_QUEUE_H_ #define _SYS_QUEUE_H_ -#include - /* * This file defines four types of data structures: singly-linked lists, * singly-linked tail queues, lists and tail queues. From c299ed717eea4dbf7ca3581bcba05ff09f79276c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Tue, 29 Jul 2014 09:41:55 +0000 Subject: [PATCH 4/8] zebra: fix struct msghdr initializers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: David Lamparter --- zebra/rt_netlink.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index 452ea64218..95a82fd2e4 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -282,9 +282,17 @@ netlink_parse_info (int (*filter) (struct sockaddr_nl *, struct nlmsghdr *), while (1) { char buf[NL_PKT_BUF_SIZE]; - struct iovec iov = { buf, sizeof buf }; + struct iovec iov = { + .iov_base = buf, + .iov_len = sizeof buf + }; struct sockaddr_nl snl; - struct msghdr msg = { (void *) &snl, sizeof snl, &iov, 1, NULL, 0, 0 }; + struct msghdr msg = { + .msg_name = (void *) &snl, + .msg_namelen = sizeof snl, + .msg_iov = &iov, + .msg_iovlen = 1 + }; struct nlmsghdr *h; status = recvmsg (nl->sock, &msg, 0); @@ -1312,8 +1320,16 @@ netlink_talk (struct nlmsghdr *n, struct nlsock *nl) { int status; struct sockaddr_nl snl; - struct iovec iov = { (void *) n, n->nlmsg_len }; - struct msghdr msg = { (void *) &snl, sizeof snl, &iov, 1, NULL, 0, 0 }; + struct iovec iov = { + .iov_base = (void *) n, + .iov_len = n->nlmsg_len + }; + struct msghdr msg = { + .msg_name = (void *) &snl, + .msg_namelen = sizeof snl, + .msg_iov = &iov, + .msg_iovlen = 1, + }; int save_errno; memset (&snl, 0, sizeof snl); From 3ef0b877f08344aa52367794aa4ec32b12becd6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Tue, 29 Jul 2014 09:41:56 +0000 Subject: [PATCH 5/8] build: do not assume glibc on linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: David Lamparter --- configure.ac | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 3dba3b3577..f1df482b32 100755 --- a/configure.ac +++ b/configure.ac @@ -1204,21 +1204,22 @@ dnl ---------- if test "$zebra_cv_linux_ipv6" = "yes";then AC_MSG_CHECKING(for GNU libc >= 2.1) AC_DEFINE(HAVE_IPV6,1,Linux IPv6) + AC_DEFINE(LINUX_IPV6,1,Linux IPv6 stack) + AC_EGREP_CPP(yes, [ #include #if __GLIBC__ >= 2 && __GLIBC_MINOR__ >= 1 yes #endif], [glibc=yes - AC_DEFINE(LINUX_IPV6,1,Linux IPv6 stack) AC_MSG_RESULT(yes)], AC_MSG_RESULT(no) ) RIPNGD="ripngd" OSPF6D="ospf6d" if test "$glibc" != "yes"; then - INCLUDES="-I/usr/inet6/include" if test x`ls /usr/inet6/lib/libinet6.a 2>/dev/null` != x;then + INCLUDES="-I/usr/inet6/include" LIB_IPV6="-L/usr/inet6/lib -linet6" fi fi From 4c005e3f65a1f5b4592b1ebbac392cbb1a710998 Mon Sep 17 00:00:00 2001 From: John Glotzer Date: Mon, 4 Aug 2014 19:39:23 +0000 Subject: [PATCH 6/8] 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 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 --- bgpd/bgp_community.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c index fc1bef88b7..1bd2dd84e4 100644 --- a/bgpd/bgp_community.c +++ b/bgpd/bgp_community.c @@ -78,7 +78,7 @@ community_del_val (struct community *com, u_int32_t *val) c = com->size -i -1; if (c > 0) - memcpy (com->val + i, com->val + (i + 1), c * sizeof (*val)); + memmove (com->val + i, com->val + (i + 1), c * sizeof (*val)); com->size--; From ad2f92b6b07883f6a2a26499eab1776933185960 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Mon, 18 Aug 2014 18:05:25 +0200 Subject: [PATCH 7/8] 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 --- isisd/isis_pdu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/isisd/isis_pdu.c b/isisd/isis_pdu.c index 5f18135e87..e0208fa47f 100644 --- a/isisd/isis_pdu.c +++ b/isisd/isis_pdu.c @@ -537,7 +537,7 @@ process_p2p_hello (struct isis_circuit *circuit) { /* TBA: check that we have a linklocal ourselves? */ struct listnode *node; - struct prefix_ipv6 *ip; + struct in6_addr *ip; for (ALL_LIST_ELEMENTS_RO (tlvs.ipv6_addrs, node, ip)) if (IN6_IS_ADDR_LINKLOCAL (ip)) { @@ -1098,7 +1098,7 @@ process_lan_hello (int level, struct isis_circuit *circuit, u_char * ssnpa) { /* TBA: check that we have a linklocal ourselves? */ struct listnode *node; - struct prefix_ipv6 *ip; + struct in6_addr *ip; for (ALL_LIST_ELEMENTS_RO (tlvs.ipv6_addrs, node, ip)) if (IN6_IS_ADDR_LINKLOCAL (ip)) { From 90444ca35e3037ed43ec695428f0ef6d82f9a320 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 1 Jul 2014 16:14:05 +0200 Subject: [PATCH 8/8] 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 Signed-off-by: David Lamparter --- lib/zclient.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/zclient.c b/lib/zclient.c index 20188f6abd..3b5477e903 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -805,6 +805,16 @@ zebra_interface_address_read (int type, struct stream *s) ifc->flags = ifc_flags; if (ifc->destination) ifc->destination->prefixlen = ifc->address->prefixlen; + else if (CHECK_FLAG(ifc->flags, ZEBRA_IFA_PEER)) + { + /* carp interfaces on OpenBSD with 0.0.0.0/0 as "peer" */ + char buf[BUFSIZ]; + prefix2str (ifc->address, buf, sizeof(buf)); + zlog_warn("warning: interface %s address %s " + "with peer flag set, but no peer address!", + ifp->name, buf); + UNSET_FLAG(ifc->flags, ZEBRA_IFA_PEER); + } } } else