Next-hop check not needed?

21 Jun 2017

      Hey

in netlink.c, there is this piece of code:

#v+
      if (a[RTA_GATEWAY])
	{
	  neighbor *ng;
	  ra->dest = RTD_ROUTER;
	  memcpy(&ra->gw, RTA_DATA(a[RTA_GATEWAY]), sizeof(ra->gw));
	  ipa_ntoh(ra->gw);

#ifdef IPV6
	  /* Silently skip strange 6to4 routes */
	  if (ipa_in_net(ra->gw, IPA_NONE, 96))
	    return;
#endif

	  ng = neigh_find2(&p->p, &ra->gw, ra->iface,
			   (i->rtm_flags & RTNH_F_ONLINK) ? NEF_ONLINK : 0);
	  if (!ng || (ng->scope == SCOPE_HOST))
	    {
	      log(L_ERR "KRT: Received route %I/%d with strange next-hop %I",
		  net->n.prefix, net->n.pxlen, ra->gw);
	      return;
	    }
	}
#v-

In turns, it calls neigh_find2() which delegates the decision to
if_connected(). if_connected() will return an error if it thinks that
the gateway is not part of the prefix of one of the configured IP
addresses.

This means that if I have the following setup:

ip route add 203.0.113.4/32 dev vnet2
ip route add 203.0.113.15/32 via 203.0.113.4 dev vnet2

BIRD will complain about "strange next-hop" while the setup is perfectly
valid. I don't need an IP address on an interface to route traffic on
it.

I would think the check should just be removed. If the kernel says this
is a valid gateway, it likely is. However, I can't find the reason the
check was added.

In the past, this was just a warning:

#v+
+	  ng = neigh_find(&p->p, &ra.gw, 0);
+	  if (ng)
+	    ra.iface = ng->iface;
+	  else
+	    /* FIXME: Remove this warning? */
+	    log(L_WARN "Kernel told us to use non-neighbor %I for %I/%d", ra.gw, net->n.prefix, net->n.pxlen);
#v-

(commit aa64578641c15b137172acc927d9d7af5914576b)

But this was made an error in commit
9d4d38d1a5d67f5485d2b2fa439c879583dfdcb0.

#v+
-	  ng = neigh_find(&p->p, &ra.gw, 0);
+	  ng = neigh_find2(&p->p, &ra.gw, ifa, 0);
 	  if (ng && ng->scope)
+	  {
+	    if (ng->iface != ifa)
+	      log(L_WARN "KRT: Route with unexpected iface for %I/%d", net->n.prefix, net->n.pxlen);
 	    ra.iface = ng->iface;
+	  }
 	  else
-	    /* FIXME: Remove this warning? Handle it somehow... */
+	  {
 	    log(L_WARN "Kernel told us to use non-neighbor %I for %I/%d", ra.gw, net->n.prefix, net->n.pxlen);
+	    return;
+	  }
#v-

Commit message says "Fixes some problems related to link-local routes in
KRT interface". Does this ring any bell?
-- 
Avoid temporary variables.
            - The Elements of Programming Style (Kernighan & Plauger)

Next-hop check not needed?

Vincent Bernat