]> git.proxmox.com Git - mirror_frr.git/commitdiff
zebra: Refactor kernel_rtm to be a bit smarter about how it handles options
authorDonald Sharp <sharpd@cumulusnetworks.com>
Mon, 17 Dec 2018 23:21:38 +0000 (18:21 -0500)
committerDonald Sharp <sharpd@cumulusnetworks.com>
Wed, 19 Dec 2018 13:58:33 +0000 (08:58 -0500)
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>
zebra/rt_socket.c

index 54832c26886c9af4ade7934ea9cb226f99945a77..0336d79ee4d0ca6749de80af75c5cdef69ec994c 100644 (file)
@@ -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. */