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 <sharpd@cumulusnetworks.com>

1 files changed, 127 insertions, 131 deletions

diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c
index 54832c268..0336d79ee 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 NEXTHOP_TYPE_IFINDEX:
-				ifindex = nexthop->ifindex;
+			case AFI_IP6:
 				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;
-				}
 			}
+		}
 
-			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;
+		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;
-				}
-				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;
+				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);
+				sin_mask.sin6.sin6_len = sin6_masklen(
+					sin_mask.sin6.sin6_addr);
 #endif /* SIN6_LEN */
-					mask = &sin_mask;
-				}
-				break;
+				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);
-
-			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;
+		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;
+
+			/* 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. */