From ca2c70bde099e9e9e40d5151388d1e9ba849df0d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 17 Dec 2018 13:57:04 -0500 Subject: [PATCH 1/8] zebra: Convert socket interface to use `union sockunion` The write function converted to v4 and v6 functions to a union sockunion via casting. Just use `union sockunion` instead. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 82 +++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 19d41a402e..38c362a193 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -94,8 +94,8 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, const struct nexthop_group *ng, uint32_t metric) { - struct sockaddr_in *mask = NULL; - struct sockaddr_in sin_dest, sin_mask, sin_gate; + union sockunion *mask = NULL; + union sockunion sin_dest, sin_mask, sin_gate; #ifdef __OpenBSD__ struct sockaddr_mpls smpls; #endif @@ -110,19 +110,19 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, if (IS_ZEBRA_DEBUG_RIB) prefix2str(p, prefix_buf, sizeof(prefix_buf)); - memset(&sin_dest, 0, sizeof(struct sockaddr_in)); - sin_dest.sin_family = AF_INET; + memset(&sin_dest, 0, sizeof(sin_dest)); + sin_dest.sin.sin_family = AF_INET; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_dest.sin_len = sizeof(struct sockaddr_in); + sin_dest.sin.sin_len = sizeof(sin_dest); #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ - sin_dest.sin_addr = p->u.prefix4; + sin_dest.sin.sin_addr = p->u.prefix4; - memset(&sin_mask, 0, sizeof(struct sockaddr_in)); + memset(&sin_mask, 0, sizeof(sin_mask)); - memset(&sin_gate, 0, sizeof(struct sockaddr_in)); - sin_gate.sin_family = AF_INET; + memset(&sin_gate, 0, sizeof(sin_gate)); + sin_gate.sin.sin_family = AF_INET; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_gate.sin_len = sizeof(struct sockaddr_in); + sin_gate.sin.sin_len = sizeof(sin_gate); #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ /* Make gateway. */ @@ -142,7 +142,7 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, || (cmd == RTM_DELETE)) { if (nexthop->type == NEXTHOP_TYPE_IPV4 || nexthop->type == NEXTHOP_TYPE_IPV4_IFINDEX) { - sin_gate.sin_addr = nexthop->gate.ipv4; + sin_gate.sin.sin_addr = nexthop->gate.ipv4; gate = 1; } if (nexthop->type == NEXTHOP_TYPE_IFINDEX @@ -151,7 +151,7 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, if (nexthop->type == NEXTHOP_TYPE_BLACKHOLE) { struct in_addr loopback; loopback.s_addr = htonl(INADDR_LOOPBACK); - sin_gate.sin_addr = loopback; + sin_gate.sin.sin_addr = loopback; bh_type = nexthop->bh_type; gate = 1; } @@ -159,11 +159,12 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, if (gate && p->prefixlen == 32) mask = NULL; else { - masklen2ip(p->prefixlen, &sin_mask.sin_addr); - sin_mask.sin_family = AF_INET; + masklen2ip(p->prefixlen, + &sin_mask.sin.sin_addr); + sin_mask.sin.sin_family = AF_INET; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_mask.sin_len = - sin_masklen(sin_mask.sin_addr); + sin_mask.sin.sin_len = + sin_masklen(sin_mask.sin.sin_addr); #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ mask = &sin_mask; } @@ -176,11 +177,9 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, smplsp = (union sockunion *)&smpls; #endif - error = rtm_write(cmd, (union sockunion *)&sin_dest, - (union sockunion *)mask, - gate ? (union sockunion *)&sin_gate - : NULL, - smplsp, ifindex, bh_type, metric); + error = rtm_write(cmd, &sin_dest, mask, + gate ? &sin_gate : NULL, smplsp, + ifindex, bh_type, metric); if (IS_ZEBRA_DEBUG_KERNEL) { if (!gate) { @@ -188,7 +187,8 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, "%s: %s: attention! gate not found for re", __func__, prefix_buf); } else - inet_ntop(AF_INET, &sin_gate.sin_addr, + inet_ntop(AF_INET, + &sin_gate.sin.sin_addr, gate_buf, INET_ADDRSTRLEN); } @@ -281,8 +281,8 @@ static int sin6_masklen(struct in6_addr mask) static int kernel_rtm_ipv6(int cmd, const struct prefix *p, const struct nexthop_group *ng, uint32_t metric) { - struct sockaddr_in6 *mask; - struct sockaddr_in6 sin_dest, sin_mask, sin_gate; + union sockunion *mask; + union sockunion sin_dest, sin_mask, sin_gate; #ifdef __OpenBSD__ struct sockaddr_mpls smpls; #endif @@ -294,19 +294,19 @@ static int kernel_rtm_ipv6(int cmd, const struct prefix *p, int error; enum blackhole_type bh_type = BLACKHOLE_UNSPEC; - memset(&sin_dest, 0, sizeof(struct sockaddr_in6)); - sin_dest.sin6_family = AF_INET6; + memset(&sin_dest, 0, sizeof(sin_dest)); + sin_dest.sin6.sin6_family = AF_INET6; #ifdef SIN6_LEN - sin_dest.sin6_len = sizeof(struct sockaddr_in6); + sin_dest.sin6.sin6_len = sizeof(sin_dest); #endif /* SIN6_LEN */ - sin_dest.sin6_addr = p->u.prefix6; + sin_dest.sin6.sin6_addr = p->u.prefix6; - memset(&sin_mask, 0, sizeof(struct sockaddr_in6)); + memset(&sin_mask, 0, sizeof(sin_mask)); - memset(&sin_gate, 0, sizeof(struct sockaddr_in6)); - sin_gate.sin6_family = AF_INET6; + memset(&sin_gate, 0, sizeof(sin_gate)); + sin_gate.sin6.sin6_family = AF_INET6; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_gate.sin6_len = sizeof(struct sockaddr_in6); + sin_gate.sin6.sin6_len = sizeof(sin_gate); #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ /* Make gateway. */ @@ -320,7 +320,7 @@ static int kernel_rtm_ipv6(int cmd, const struct prefix *p, || (cmd == RTM_DELETE)) { if (nexthop->type == NEXTHOP_TYPE_IPV6 || nexthop->type == NEXTHOP_TYPE_IPV6_IFINDEX) { - sin_gate.sin6_addr = nexthop->gate.ipv6; + sin_gate.sin6.sin6_addr = nexthop->gate.ipv6; gate = 1; } if (nexthop->type == NEXTHOP_TYPE_IFINDEX @@ -340,17 +340,19 @@ static int kernel_rtm_ipv6(int cmd, const struct prefix *p, (a).s6_addr[3] = (i)&0xff; \ } while (0) - if (gate && IN6_IS_ADDR_LINKLOCAL(&sin_gate.sin6_addr)) - SET_IN6_LINKLOCAL_IFINDEX(sin_gate.sin6_addr, ifindex); + if (gate && IN6_IS_ADDR_LINKLOCAL(&sin_gate.sin6.sin6_addr)) + SET_IN6_LINKLOCAL_IFINDEX(sin_gate.sin6.sin6_addr, + ifindex); #endif /* KAME */ if (gate && p->prefixlen == 128) mask = NULL; else { - masklen2ip6(p->prefixlen, &sin_mask.sin6_addr); - sin_mask.sin6_family = AF_INET6; + masklen2ip6(p->prefixlen, &sin_mask.sin6.sin6_addr); + sin_mask.sin6.sin6_family = AF_INET6; #ifdef SIN6_LEN - sin_mask.sin6_len = sin6_masklen(sin_mask.sin6_addr); + sin_mask.sin6.sin6_len = + sin6_masklen(sin_mask.sin6.sin6_addr); #endif /* SIN6_LEN */ mask = &sin_mask; } @@ -362,9 +364,7 @@ static int kernel_rtm_ipv6(int cmd, const struct prefix *p, smplsp = (union sockunion *)&smpls; #endif - error = rtm_write(cmd, (union sockunion *)&sin_dest, - (union sockunion *)mask, - gate ? (union sockunion *)&sin_gate : NULL, + error = rtm_write(cmd, &sin_dest, mask, gate ? &sin_gate : NULL, smplsp, ifindex, bh_type, metric); /* Update installed nexthop info on success */ From 9ba0e5706c6206e1ba8035b2ae1aa0136f36f4bb Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 17 Dec 2018 16:28:47 -0500 Subject: [PATCH 2/8] zebra: Move sin6_masklen to earlier in the file I'm going to rearrage the kernel_rtm_ipv4 and v6 functions so the sin6_masklen needs to be moved a bit earlier. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 48 +++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 38c362a193..d5a4fcb116 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -89,6 +89,30 @@ static int kernel_rtm_add_labels(struct mpls_label_stack *nh_label, } #endif +#ifdef SIN6_LEN +/* Calculate sin6_len value for netmask socket value. */ +static int sin6_masklen(struct in6_addr mask) +{ + struct sockaddr_in6 sin6; + char *p, *lim; + int len; + + if (IN6_IS_ADDR_UNSPECIFIED(&mask)) + return sizeof(long); + + sin6.sin6_addr = mask; + len = sizeof(struct sockaddr_in6); + + lim = (char *)&sin6.sin6_addr; + p = lim + sizeof(sin6.sin6_addr); + + while (*--p == 0 && p >= lim) + len--; + + return len; +} +#endif /* SIN6_LEN */ + /* Interface between zebra message and rtm message. */ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, const struct nexthop_group *ng, uint32_t metric) @@ -253,30 +277,6 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, return 0; /*XXX*/ } -#ifdef SIN6_LEN -/* Calculate sin6_len value for netmask socket value. */ -static int sin6_masklen(struct in6_addr mask) -{ - struct sockaddr_in6 sin6; - char *p, *lim; - int len; - - if (IN6_IS_ADDR_UNSPECIFIED(&mask)) - return sizeof(long); - - sin6.sin6_addr = mask; - len = sizeof(struct sockaddr_in6); - - lim = (char *)&sin6.sin6_addr; - p = lim + sizeof(sin6.sin6_addr); - - while (*--p == 0 && p >= lim) - len--; - - return len; -} -#endif /* SIN6_LEN */ - /* Interface between zebra message and rtm message. */ static int kernel_rtm_ipv6(int cmd, const struct prefix *p, const struct nexthop_group *ng, uint32_t metric) From 08ea27d1121ef5989cdc54fb178c05a7efc4cd3e Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 17 Dec 2018 14:23:02 -0500 Subject: [PATCH 3/8] zebra: Refactor kernel_socket kernel_rtm_ipv4 and ipv6 functions Refactor both v4 and v6 functions down to 1 install function. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 275 ++++++++++++++++++---------------------------- 1 file changed, 109 insertions(+), 166 deletions(-) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index d5a4fcb116..54832c2688 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -114,8 +114,8 @@ static int sin6_masklen(struct in6_addr mask) #endif /* SIN6_LEN */ /* Interface between zebra message and rtm message. */ -static int kernel_rtm_ipv4(int cmd, const struct prefix *p, - const struct nexthop_group *ng, uint32_t metric) +static int kernel_rtm(int cmd, const struct prefix *p, + const struct nexthop_group *ng, uint32_t metric) { union sockunion *mask = NULL; @@ -134,20 +134,33 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, if (IS_ZEBRA_DEBUG_RIB) prefix2str(p, prefix_buf, sizeof(prefix_buf)); - memset(&sin_dest, 0, sizeof(sin_dest)); - sin_dest.sin.sin_family = AF_INET; -#ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_dest.sin.sin_len = sizeof(sin_dest); -#endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ - sin_dest.sin.sin_addr = p->u.prefix4; + memset(&sin_dest, 0, sizeof(sin_dest)); + memset(&sin_gate, 0, sizeof(sin_gate)); memset(&sin_mask, 0, sizeof(sin_mask)); - memset(&sin_gate, 0, sizeof(sin_gate)); - sin_gate.sin.sin_family = AF_INET; + switch (p->family) { + case AF_INET: + sin_dest.sin.sin_family = AF_INET; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_gate.sin.sin_len = sizeof(sin_gate); + sin_dest.sin.sin_len = sizeof(sin_dest); + sin_gate.sin.sin_len = sizeof(sin_gate); #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ + sin_dest.sin.sin_addr = p->u.prefix4; + sin_gate.sin.sin_family = AF_INET; + break; + case AF_INET6: + sin_dest.sin6.sin6_family = AF_INET6; +#ifdef SIN6_LEN + sin_dest.sin6.sin6_len = sizeof(sin_dest); +#endif /* SIN6_LEN */ + sin_dest.sin6.sin6_addr = p->u.prefix6; + sin_gate.sin6.sin6_family = AF_INET6; +#ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN + sin_gate.sin6.sin6_len = sizeof(sin_gate); +#endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ + break; + } /* Make gateway. */ for (ALL_NEXTHOPS_PTR(ng, nexthop)) { @@ -164,33 +177,85 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, */ if ((cmd == RTM_ADD && NEXTHOP_IS_ACTIVE(nexthop->flags)) || (cmd == RTM_DELETE)) { - if (nexthop->type == NEXTHOP_TYPE_IPV4 - || nexthop->type == NEXTHOP_TYPE_IPV4_IFINDEX) { + switch (nexthop->type) { + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: sin_gate.sin.sin_addr = nexthop->gate.ipv4; - gate = 1; - } - if (nexthop->type == NEXTHOP_TYPE_IFINDEX - || nexthop->type == NEXTHOP_TYPE_IPV4_IFINDEX) + sin_gate.sin.sin_family = AF_INET; ifindex = nexthop->ifindex; - if (nexthop->type == NEXTHOP_TYPE_BLACKHOLE) { - struct in_addr loopback; - loopback.s_addr = htonl(INADDR_LOOPBACK); - sin_gate.sin.sin_addr = loopback; - bh_type = nexthop->bh_type; gate = 1; + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: + sin_gate.sin6.sin6_addr = nexthop->gate.ipv6; + sin_gate.sin6.sin6_family = AF_INET6; + ifindex = nexthop->ifindex; +/* Under kame set interface index to link local address */ +#ifdef KAME + +#define SET_IN6_LINKLOCAL_IFINDEX(a, i) \ + do { \ + (a).s6_addr[2] = ((i) >> 8) & 0xff; \ + (a).s6_addr[3] = (i)&0xff; \ + } while (0) + + if (IN6_IS_ADDR_LINKLOCAL( + &sin_gate.sin6.sin6_addr)) + SET_IN6_LINKLOCAL_IFINDEX( + sin_gate.sin6.sin6_addr, + ifindex); +#endif /* KAME */ + + gate = 1; + break; + case NEXTHOP_TYPE_IFINDEX: + ifindex = nexthop->ifindex; + break; + case NEXTHOP_TYPE_BLACKHOLE: + bh_type = nexthop->bh_type; + switch (p->family) { + case AFI_IP: { + struct in_addr loopback; + loopback.s_addr = + htonl(INADDR_LOOPBACK); + sin_gate.sin.sin_addr = loopback; + gate = 1; + } + break; + case AFI_IP6: + break; + } } - if (gate && p->prefixlen == 32) - mask = NULL; - else { - masklen2ip(p->prefixlen, - &sin_mask.sin.sin_addr); - sin_mask.sin.sin_family = AF_INET; + switch (p->family) { + case AF_INET: + if (gate && p->prefixlen == 32) + mask = NULL; + else { + masklen2ip(p->prefixlen, + &sin_mask.sin.sin_addr); + sin_mask.sin.sin_family = AF_INET; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_mask.sin.sin_len = - sin_masklen(sin_mask.sin.sin_addr); + sin_mask.sin.sin_len = sin_masklen( + sin_mask.sin.sin_addr); #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ - mask = &sin_mask; + mask = &sin_mask; + } + break; + case AF_INET6: + if (gate && p->prefixlen == 128) + mask = NULL; + else { + masklen2ip6(p->prefixlen, + &sin_mask.sin6.sin6_addr); + sin_mask.sin6.sin6_family = AF_INET6; +#ifdef SIN6_LEN + sin_mask.sin6.sin6_len = sin6_masklen( + sin_mask.sin6.sin6_addr); +#endif /* SIN6_LEN */ + mask = &sin_mask; + } + break; } #ifdef __OpenBSD__ @@ -200,7 +265,6 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, continue; smplsp = (union sockunion *)&smpls; #endif - error = rtm_write(cmd, &sin_dest, mask, gate ? &sin_gate : NULL, smplsp, ifindex, bh_type, metric); @@ -211,33 +275,31 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, "%s: %s: attention! gate not found for re", __func__, prefix_buf); } else - inet_ntop(AF_INET, + inet_ntop(p->family == AFI_IP ? AF_INET + : AF_INET6, &sin_gate.sin.sin_addr, gate_buf, INET_ADDRSTRLEN); } - switch (error) { - /* We only flag nexthops as being in FIB if rtm_write() - * did its work. */ + /* We only flag nexthops as being in FIB if + * rtm_write() did its work. */ case ZEBRA_ERR_NOERROR: nexthop_num++; if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug( "%s: %s: successfully did NH %s", __func__, prefix_buf, gate_buf); - if (cmd == RTM_ADD) - SET_FLAG(nexthop->flags, - NEXTHOP_FLAG_FIB); - + SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); break; - /* The only valid case for this error is kernel's - * failure to install - * a multipath route, which is common for FreeBSD. This - * should be - * ignored silently, but logged as an error otherwise. - */ + /* The only valid case for this error is + * kernel's failure to install a multipath + * route, which is common for FreeBSD. This + * should be + * ignored silently, but logged as an error + * otherwise. + */ case ZEBRA_ERR_RTEXIST: if (cmd != RTM_ADD) flog_err( @@ -246,7 +308,7 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, __func__, error, cmd); continue; - /* Note any unexpected status returns */ + /* Note any unexpected status returns */ default: flog_err( EC_LIB_SYSTEM_CALL, @@ -277,125 +339,6 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, return 0; /*XXX*/ } -/* Interface between zebra message and rtm message. */ -static int kernel_rtm_ipv6(int cmd, const struct prefix *p, - const struct nexthop_group *ng, uint32_t metric) -{ - union sockunion *mask; - union sockunion sin_dest, sin_mask, sin_gate; -#ifdef __OpenBSD__ - struct sockaddr_mpls smpls; -#endif - union sockunion *smplsp = NULL; - struct nexthop *nexthop; - int nexthop_num = 0; - ifindex_t ifindex = 0; - int gate = 0; - int error; - enum blackhole_type bh_type = BLACKHOLE_UNSPEC; - - memset(&sin_dest, 0, sizeof(sin_dest)); - sin_dest.sin6.sin6_family = AF_INET6; -#ifdef SIN6_LEN - sin_dest.sin6.sin6_len = sizeof(sin_dest); -#endif /* SIN6_LEN */ - sin_dest.sin6.sin6_addr = p->u.prefix6; - - memset(&sin_mask, 0, sizeof(sin_mask)); - - memset(&sin_gate, 0, sizeof(sin_gate)); - sin_gate.sin6.sin6_family = AF_INET6; -#ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_gate.sin6.sin6_len = sizeof(sin_gate); -#endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ - - /* Make gateway. */ - for (ALL_NEXTHOPS_PTR(ng, nexthop)) { - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) - continue; - - gate = 0; - - if ((cmd == RTM_ADD && NEXTHOP_IS_ACTIVE(nexthop->flags)) - || (cmd == RTM_DELETE)) { - if (nexthop->type == NEXTHOP_TYPE_IPV6 - || nexthop->type == NEXTHOP_TYPE_IPV6_IFINDEX) { - sin_gate.sin6.sin6_addr = nexthop->gate.ipv6; - gate = 1; - } - if (nexthop->type == NEXTHOP_TYPE_IFINDEX - || nexthop->type == NEXTHOP_TYPE_IPV6_IFINDEX) - ifindex = nexthop->ifindex; - - if (nexthop->type == NEXTHOP_TYPE_BLACKHOLE) - bh_type = nexthop->bh_type; - } - -/* Under kame set interface index to link local address. */ -#ifdef KAME - -#define SET_IN6_LINKLOCAL_IFINDEX(a, i) \ - do { \ - (a).s6_addr[2] = ((i) >> 8) & 0xff; \ - (a).s6_addr[3] = (i)&0xff; \ - } while (0) - - if (gate && IN6_IS_ADDR_LINKLOCAL(&sin_gate.sin6.sin6_addr)) - SET_IN6_LINKLOCAL_IFINDEX(sin_gate.sin6.sin6_addr, - ifindex); -#endif /* KAME */ - - if (gate && p->prefixlen == 128) - mask = NULL; - else { - masklen2ip6(p->prefixlen, &sin_mask.sin6.sin6_addr); - sin_mask.sin6.sin6_family = AF_INET6; -#ifdef SIN6_LEN - sin_mask.sin6.sin6_len = - sin6_masklen(sin_mask.sin6.sin6_addr); -#endif /* SIN6_LEN */ - mask = &sin_mask; - } - -#ifdef __OpenBSD__ - if (nexthop->nh_label - && !kernel_rtm_add_labels(nexthop->nh_label, &smpls)) - continue; - smplsp = (union sockunion *)&smpls; -#endif - - error = rtm_write(cmd, &sin_dest, mask, gate ? &sin_gate : NULL, - smplsp, ifindex, bh_type, metric); - - /* Update installed nexthop info on success */ - if ((cmd == RTM_ADD) && (error == ZEBRA_ERR_NOERROR)) - SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); - - nexthop_num++; - } - - /* If there is no useful nexthop then return. */ - if (nexthop_num == 0) { - if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("kernel_rtm_ipv6(): No useful nexthop."); - return 1; - } - - return 0; /*XXX*/ -} - -static int kernel_rtm(int cmd, const struct prefix *p, - const struct nexthop_group *ng, uint32_t metric) -{ - switch (PREFIX_FAMILY(p)) { - case AF_INET: - return kernel_rtm_ipv4(cmd, p, ng, metric); - case AF_INET6: - return kernel_rtm_ipv6(cmd, p, ng, metric); - } - return 0; -} - /* * Update or delete a prefix from the kernel, * using info from a dataplane context struct. From 86afd5292f1769c6c1c1db7a1e65f09fed27b87d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 17 Dec 2018 18:21:38 -0500 Subject: [PATCH 4/8] zebra: Refactor kernel_rtm to be a bit smarter about how it handles options The ADD/DELETE messages are the only ones we support, so leave early from the function, in other words don't check it every nexthop loop. Additionally nexthops only care about non recursive active flags. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 266 +++++++++++++++++++++++----------------------- 1 file changed, 131 insertions(+), 135 deletions(-) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 54832c2688..0336d79ee4 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -135,6 +135,19 @@ static int kernel_rtm(int cmd, const struct prefix *p, if (IS_ZEBRA_DEBUG_RIB) prefix2str(p, prefix_buf, sizeof(prefix_buf)); + /* + * We only have the ability to ADD or DELETE at this point + * in time. + */ + if (cmd != RTM_ADD && cmd != RTM_DELETE) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: %s odd command %s for flags %d", + __func__, prefix_buf, + lookup_msg(rtm_type_str, cmd, NULL), + nexthop->flags); + return 0; + } + memset(&sin_dest, 0, sizeof(sin_dest)); memset(&sin_gate, 0, sizeof(sin_gate)); memset(&sin_mask, 0, sizeof(sin_mask)); @@ -164,32 +177,29 @@ static int kernel_rtm(int cmd, const struct prefix *p, /* Make gateway. */ for (ALL_NEXTHOPS_PTR(ng, nexthop)) { - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) + /* + * We only want to use the actual good nexthops + */ + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE) || + !CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) continue; gate = 0; char gate_buf[INET_ADDRSTRLEN] = "NULL"; - /* - * XXX We need to refrain from kernel operations in some cases, - * but this if statement seems overly cautious - what about - * other than ADD and DELETE? - */ - if ((cmd == RTM_ADD && NEXTHOP_IS_ACTIVE(nexthop->flags)) - || (cmd == RTM_DELETE)) { - switch (nexthop->type) { - case NEXTHOP_TYPE_IPV4: - case NEXTHOP_TYPE_IPV4_IFINDEX: - sin_gate.sin.sin_addr = nexthop->gate.ipv4; - sin_gate.sin.sin_family = AF_INET; - ifindex = nexthop->ifindex; - gate = 1; - break; - case NEXTHOP_TYPE_IPV6: - case NEXTHOP_TYPE_IPV6_IFINDEX: - sin_gate.sin6.sin6_addr = nexthop->gate.ipv6; - sin_gate.sin6.sin6_family = AF_INET6; - ifindex = nexthop->ifindex; + switch (nexthop->type) { + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: + sin_gate.sin.sin_addr = nexthop->gate.ipv4; + sin_gate.sin.sin_family = AF_INET; + ifindex = nexthop->ifindex; + gate = 1; + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: + sin_gate.sin6.sin6_addr = nexthop->gate.ipv6; + sin_gate.sin6.sin6_family = AF_INET6; + ifindex = nexthop->ifindex; /* Under kame set interface index to link local address */ #ifdef KAME @@ -199,132 +209,118 @@ static int kernel_rtm(int cmd, const struct prefix *p, (a).s6_addr[3] = (i)&0xff; \ } while (0) - if (IN6_IS_ADDR_LINKLOCAL( - &sin_gate.sin6.sin6_addr)) - SET_IN6_LINKLOCAL_IFINDEX( - sin_gate.sin6.sin6_addr, - ifindex); + if (IN6_IS_ADDR_LINKLOCAL(&sin_gate.sin6.sin6_addr)) + SET_IN6_LINKLOCAL_IFINDEX( + sin_gate.sin6.sin6_addr, + ifindex); #endif /* KAME */ - gate = 1; - break; - case NEXTHOP_TYPE_IFINDEX: - ifindex = nexthop->ifindex; - break; - case NEXTHOP_TYPE_BLACKHOLE: - bh_type = nexthop->bh_type; - switch (p->family) { - case AFI_IP: { - struct in_addr loopback; - loopback.s_addr = - htonl(INADDR_LOOPBACK); - sin_gate.sin.sin_addr = loopback; - gate = 1; - } - break; - case AFI_IP6: - break; - } - } - + gate = 1; + break; + case NEXTHOP_TYPE_IFINDEX: + ifindex = nexthop->ifindex; + break; + case NEXTHOP_TYPE_BLACKHOLE: + bh_type = nexthop->bh_type; switch (p->family) { - case AF_INET: - if (gate && p->prefixlen == 32) - mask = NULL; - else { - masklen2ip(p->prefixlen, - &sin_mask.sin.sin_addr); - sin_mask.sin.sin_family = AF_INET; -#ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_mask.sin.sin_len = sin_masklen( - sin_mask.sin.sin_addr); -#endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ - mask = &sin_mask; - } + case AFI_IP: { + struct in_addr loopback; + loopback.s_addr = htonl(INADDR_LOOPBACK); + sin_gate.sin.sin_addr = loopback; + gate = 1; + } break; - case AF_INET6: - if (gate && p->prefixlen == 128) - mask = NULL; - else { - masklen2ip6(p->prefixlen, - &sin_mask.sin6.sin6_addr); - sin_mask.sin6.sin6_family = AF_INET6; -#ifdef SIN6_LEN - sin_mask.sin6.sin6_len = sin6_masklen( - sin_mask.sin6.sin6_addr); -#endif /* SIN6_LEN */ - mask = &sin_mask; - } + case AFI_IP6: break; } + } + + switch (p->family) { + case AF_INET: + if (gate && p->prefixlen == 32) + mask = NULL; + else { + masklen2ip(p->prefixlen, + &sin_mask.sin.sin_addr); + sin_mask.sin.sin_family = AF_INET; +#ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN + sin_mask.sin.sin_len = sin_masklen( + sin_mask.sin.sin_addr); +#endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ + mask = &sin_mask; + } + break; + case AF_INET6: + if (gate && p->prefixlen == 128) + mask = NULL; + else { + masklen2ip6(p->prefixlen, + &sin_mask.sin6.sin6_addr); + sin_mask.sin6.sin6_family = AF_INET6; +#ifdef SIN6_LEN + sin_mask.sin6.sin6_len = sin6_masklen( + sin_mask.sin6.sin6_addr); +#endif /* SIN6_LEN */ + mask = &sin_mask; + } + break; + } #ifdef __OpenBSD__ - if (nexthop->nh_label - && !kernel_rtm_add_labels(nexthop->nh_label, - &smpls)) - continue; - smplsp = (union sockunion *)&smpls; + if (nexthop->nh_label + && !kernel_rtm_add_labels(nexthop->nh_label, &smpls)) + continue; + smplsp = (union sockunion *)&smpls; #endif - error = rtm_write(cmd, &sin_dest, mask, - gate ? &sin_gate : NULL, smplsp, - ifindex, bh_type, metric); + error = rtm_write(cmd, &sin_dest, mask, + gate ? &sin_gate : NULL, smplsp, + ifindex, bh_type, metric); - if (IS_ZEBRA_DEBUG_KERNEL) { - if (!gate) { - zlog_debug( - "%s: %s: attention! gate not found for re", - __func__, prefix_buf); - } else - inet_ntop(p->family == AFI_IP ? AF_INET - : AF_INET6, - &sin_gate.sin.sin_addr, - gate_buf, INET_ADDRSTRLEN); - } - switch (error) { - /* We only flag nexthops as being in FIB if - * rtm_write() did its work. */ - case ZEBRA_ERR_NOERROR: - nexthop_num++; - if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug( - "%s: %s: successfully did NH %s", - __func__, prefix_buf, gate_buf); - if (cmd == RTM_ADD) - SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); - break; + if (IS_ZEBRA_DEBUG_KERNEL) { + if (!gate) { + zlog_debug("%s: %s: attention! gate not found for re", + __func__, prefix_buf); + } else + inet_ntop(p->family == AFI_IP ? AF_INET + : AF_INET6, + &sin_gate.sin.sin_addr, + gate_buf, INET_ADDRSTRLEN); + } + switch (error) { + /* We only flag nexthops as being in FIB if + * rtm_write() did its work. */ + case ZEBRA_ERR_NOERROR: + nexthop_num++; + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: %s: successfully did NH %s", + __func__, prefix_buf, gate_buf); + if (cmd == RTM_ADD) + SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); + break; - /* The only valid case for this error is - * kernel's failure to install a multipath - * route, which is common for FreeBSD. This - * should be - * ignored silently, but logged as an error - * otherwise. - */ - case ZEBRA_ERR_RTEXIST: - if (cmd != RTM_ADD) - flog_err( - EC_LIB_SYSTEM_CALL, - "%s: rtm_write() returned %d for command %d", - __func__, error, cmd); - continue; + /* The only valid case for this error is + * kernel's failure to install a multipath + * route, which is common for FreeBSD. This + * should be ignored silently, but logged as an error + * otherwise. + */ + case ZEBRA_ERR_RTEXIST: + if (cmd != RTM_ADD) + flog_err(EC_LIB_SYSTEM_CALL, + "%s: rtm_write() returned %d for command %d", + __func__, error, cmd); + continue; - /* Note any unexpected status returns */ - default: - flog_err( - EC_LIB_SYSTEM_CALL, - "%s: %s: rtm_write() unexpectedly returned %d for command %s", - __func__, - prefix2str(p, prefix_buf, - sizeof(prefix_buf)), - error, - lookup_msg(rtm_type_str, cmd, NULL)); - break; - } - } /* if (cmd and flags make sense) */ - else if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("%s: odd command %s for flags %d", __func__, - lookup_msg(rtm_type_str, cmd, NULL), - nexthop->flags); + /* Note any unexpected status returns */ + default: + flog_err(EC_LIB_SYSTEM_CALL, + "%s: %s: rtm_write() unexpectedly returned %d for command %s", + __func__, + prefix2str(p, prefix_buf, + sizeof(prefix_buf)), + error, lookup_msg(rtm_type_str, cmd, NULL)); + break; + } } /* for (ALL_NEXTHOPS(...))*/ /* If there was no useful nexthop, then complain. */ From 18d10d885461eaf44fba87ed57a347b565e94f55 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 17 Dec 2018 18:31:09 -0500 Subject: [PATCH 5/8] zebra: The mask and sin_mask are a bit redundant for kernel_rtm The test we were using to ensure that a mask was sent in is a bit redundant, let's just always send it in. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 0336d79ee4..539370a498 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -118,7 +118,6 @@ static int kernel_rtm(int cmd, const struct prefix *p, const struct nexthop_group *ng, uint32_t metric) { - union sockunion *mask = NULL; union sockunion sin_dest, sin_mask, sin_gate; #ifdef __OpenBSD__ struct sockaddr_mpls smpls; @@ -237,32 +236,20 @@ static int kernel_rtm(int cmd, const struct prefix *p, switch (p->family) { case AF_INET: - if (gate && p->prefixlen == 32) - mask = NULL; - else { - masklen2ip(p->prefixlen, - &sin_mask.sin.sin_addr); - sin_mask.sin.sin_family = AF_INET; + masklen2ip(p->prefixlen, &sin_mask.sin.sin_addr); + sin_mask.sin.sin_family = AF_INET; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN - sin_mask.sin.sin_len = sin_masklen( - sin_mask.sin.sin_addr); + sin_mask.sin.sin_len = sin_masklen( + sin_mask.sin.sin_addr); #endif /* HAVE_STRUCT_SOCKADDR_IN_SIN_LEN */ - mask = &sin_mask; - } break; case AF_INET6: - if (gate && p->prefixlen == 128) - mask = NULL; - else { - masklen2ip6(p->prefixlen, - &sin_mask.sin6.sin6_addr); - sin_mask.sin6.sin6_family = AF_INET6; + masklen2ip6(p->prefixlen, &sin_mask.sin6.sin6_addr); + sin_mask.sin6.sin6_family = AF_INET6; #ifdef SIN6_LEN - sin_mask.sin6.sin6_len = sin6_masklen( - sin_mask.sin6.sin6_addr); + sin_mask.sin6.sin6_len = sin6_masklen( + sin_mask.sin6.sin6_addr); #endif /* SIN6_LEN */ - mask = &sin_mask; - } break; } @@ -272,7 +259,7 @@ static int kernel_rtm(int cmd, const struct prefix *p, continue; smplsp = (union sockunion *)&smpls; #endif - error = rtm_write(cmd, &sin_dest, mask, + error = rtm_write(cmd, &sin_dest, &sin_mask, gate ? &sin_gate : NULL, smplsp, ifindex, bh_type, metric); From 4dd39a0ec46d3298a8c7fcda3cb94a50d0f0e533 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 17 Dec 2018 18:34:26 -0500 Subject: [PATCH 6/8] zebra: Convert gate in kernel_rtm to a bool Convert the gate test int to a bool as that we use it this way. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 539370a498..23beeaebb7 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -126,7 +126,7 @@ static int kernel_rtm(int cmd, const struct prefix *p, struct nexthop *nexthop; int nexthop_num = 0; ifindex_t ifindex = 0; - int gate = 0; + bool gate = false; int error; char prefix_buf[PREFIX_STRLEN]; enum blackhole_type bh_type = BLACKHOLE_UNSPEC; @@ -183,7 +183,7 @@ static int kernel_rtm(int cmd, const struct prefix *p, !CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) continue; - gate = 0; + gate = false; char gate_buf[INET_ADDRSTRLEN] = "NULL"; switch (nexthop->type) { @@ -192,7 +192,7 @@ static int kernel_rtm(int cmd, const struct prefix *p, sin_gate.sin.sin_addr = nexthop->gate.ipv4; sin_gate.sin.sin_family = AF_INET; ifindex = nexthop->ifindex; - gate = 1; + gate = true; break; case NEXTHOP_TYPE_IPV6: case NEXTHOP_TYPE_IPV6_IFINDEX: @@ -214,7 +214,7 @@ static int kernel_rtm(int cmd, const struct prefix *p, ifindex); #endif /* KAME */ - gate = 1; + gate = true; break; case NEXTHOP_TYPE_IFINDEX: ifindex = nexthop->ifindex; @@ -226,7 +226,7 @@ static int kernel_rtm(int cmd, const struct prefix *p, struct in_addr loopback; loopback.s_addr = htonl(INADDR_LOOPBACK); sin_gate.sin.sin_addr = loopback; - gate = 1; + gate = true; } break; case AFI_IP6: From c2519893e0240d06c90bc7efa825875d69bdf148 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 18 Dec 2018 19:34:22 -0500 Subject: [PATCH 7/8] zebra: Make label processing guaranteed to be unique The label processing for socket installs was not ensuring that each nexthop would not accidently use the last nexthops value. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 1 + 1 file changed, 1 insertion(+) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 23beeaebb7..5ce1ddfd49 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -183,6 +183,7 @@ static int kernel_rtm(int cmd, const struct prefix *p, !CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) continue; + smplsp = NULL; gate = false; char gate_buf[INET_ADDRSTRLEN] = "NULL"; From c9277ebb410ccfad0f7c2d0104bbcb0f7407ec9a Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 18 Dec 2018 19:32:14 -0500 Subject: [PATCH 8/8] zebra: Fixup spaces/tabs issue found by CI in rt_socket.c Cleanup the space/tabs issues found by CI in rt_socket.c so it stops complaining at us. Signed-off-by: Donald Sharp --- zebra/rt_socket.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 5ce1ddfd49..29e9bf82f0 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -204,10 +204,10 @@ static int kernel_rtm(int cmd, const struct prefix *p, #ifdef KAME #define SET_IN6_LINKLOCAL_IFINDEX(a, i) \ - do { \ - (a).s6_addr[2] = ((i) >> 8) & 0xff; \ - (a).s6_addr[3] = (i)&0xff; \ - } while (0) + do { \ + (a).s6_addr[2] = ((i) >> 8) & 0xff; \ + (a).s6_addr[3] = (i)&0xff; \ + } while (0) if (IN6_IS_ADDR_LINKLOCAL(&sin_gate.sin6.sin6_addr)) SET_IN6_LINKLOCAL_IFINDEX(