Nexthop: Fixed recursive route mpls label merging

conf/confbase.Y

@@ -65,7 +65,7 @@ CF_DECLS
   struct proto_spec ps;
   struct channel_limit cl;
   struct timeformat *tf;
-  u32 *lbl;
+  mpls_label_stack *mls;
 }
 
 %token END CLI_MARKER INVALID_TOKEN ELSECOL DDOT
@@ -85,7 +85,7 @@ CF_DECLS
 %type <a> ipa
 %type <net> net_ip4_ net_ip6_ net_ip6 net_ip_ net_ip net_or_ipa
 %type <net_ptr> net_ net_any net_vpn4_ net_vpn6_ net_vpn_ net_roa4_ net_roa6_ net_roa_
-%type <lbl> label_stack_start label_stack
+%type <mls> label_stack_start label_stack
 
 %type <t> text opttext
 
@@ -288,18 +288,17 @@ net_or_ipa:
 
 label_stack_start: NUM
 {
-  $$ = cfg_allocz(sizeof(u32) * (MPLS_MAX_LABEL_STACK+1));
+  $$ = cfg_allocz(sizeof(mpls_label_stack));
-  $$[0] = 1;
+  $$->len = 1;
-  $$[1] = $1;
+  $$->stack[0] = $1;
 };
 
 label_stack:
     label_stack_start
   | label_stack '/' NUM {
-    if ($1[0] >= MPLS_MAX_LABEL_STACK)
+    if ($1->len >= MPLS_MAX_LABEL_STACK)
       cf_error("Too many labels in stack");
-    $1[0]++;
+    $1->stack[$1->len++] = $3;
-    $1[*$1] = $3;
     $$ = $1;
   }
 ;

lib/ip.h

@@ -326,6 +326,11 @@ static inline ip6_addr ip6_ntoh(ip6_addr a)
 { return _MI6(ntohl(_I0(a)), ntohl(_I1(a)), ntohl(_I2(a)), ntohl(_I3(a))); }
 
 #define MPLS_MAX_LABEL_STACK 8
+typedef struct mpls_label_stack {
+  uint len;
+  u32 stack[MPLS_MAX_LABEL_STACK];
+} mpls_label_stack;
+
 static inline int
 mpls_get(const char *buf, int buflen, u32 *stack)
 {

nest/route.h

@@ -551,7 +551,7 @@ static inline rta * rta_cow(rta *r, linpool *lp) { return rta_is_cached(r) ? rta
 void rta_dump(rta *);
 void rta_dump_all(void);
 void rta_show(struct cli *, rta *, ea_list *);
-void rta_set_recursive_next_hop(rtable *dep, rta *a, rtable *tab, ip_addr gw, ip_addr ll);
+void rta_set_recursive_next_hop(rtable *dep, rta *a, rtable *tab, ip_addr gw, ip_addr ll, mpls_label_stack *mls);
 
 /*
  * rta_set_recursive_next_hop() acquires hostentry from hostcache and fills

nest/rt-table.c

@@ -1767,55 +1767,80 @@ rta_next_hop_outdated(rta *a)
 }
 
 static inline void
-rta_apply_hostentry(rta *a, struct hostentry *he)
+rta_apply_hostentry(rta *a, struct hostentry *he, mpls_label_stack *mls)
 {
   a->hostentry = he;
   a->dest = he->dest;
   a->igp_metric = he->igp_metric;
 
-  if ((a->nh.labels_orig == 0) && (!a->nh.next) && he->nexthop_linkable)
+  if (a->dest != RTD_UNICAST)
   {
-    a->nh = he->src->nh;
+    /* No nexthop */
+no_nexthop:
+    a->nh = (struct nexthop) {};
+    if (mls)
+    { /* Store the label stack for later changes */
+      a->nh.labels_orig = a->nh.labels = mls->len;
+      memcpy(a->nh.label, mls->stack, mls->len * sizeof(u32));
+    }
     return;
   }
 
-  struct nexthop *nhp = alloca(NEXTHOP_MAX_SIZE);
+  if (((!mls) || (!mls->len)) && he->nexthop_linkable)
+  { /* Just link the nexthop chain, no label append happens. */
+    memcpy(&(a->nh), &(he->src->nh), nexthop_size(&(he->src->nh)));
+    return;
+  }
 
-  for (struct nexthop *nhe = &(a->nh); nhe; nhe = nhe->next)
+  struct nexthop *nhp = NULL, *nhr = NULL;
+  int skip_nexthop = 0;
+  
+  for (struct nexthop *nh = &(he->src->nh); nh; nh = nh->next)
   {
-    int labels_orig = nhe->labels_orig;	    /* Number of labels (at the bottom of stack) */
+    if (skip_nexthop)
-    u32 label_stack[MPLS_MAX_LABEL_STACK];
+      skip_nexthop--;
-    memcpy(label_stack, nhe->label, labels_orig * sizeof(u32));
+    else
-
-    for (struct nexthop *nh = &(he->src->nh); nh; nh = nh->next)
     {
-      nhp->iface = nh->iface;
+      nhr = nhp;
-      nhp->weight = nh->weight; /* FIXME: Ignoring the recursive nexthop's weight */
+      nhp = (nhp ? (nhp->next = lp_allocz(rte_update_pool, NEXTHOP_MAX_SIZE)) : &(a->nh));
-      nhp->labels = nh->labels + labels_orig;
+    }
-      nhp->labels_orig = labels_orig;
+
+    nhp->iface = nh->iface;
+    nhp->weight = nh->weight;
+    if (mls)
+    {
+      nhp->labels = nh->labels + mls->len;
+      nhp->labels_orig = mls->len;
       if (nhp->labels <= MPLS_MAX_LABEL_STACK)
       {
 	memcpy(nhp->label, nh->label, nh->labels * sizeof(u32)); /* First the hostentry labels */
-	memcpy(&(nhp->label[nh->labels]), label_stack, labels_orig * sizeof(u32)); /* Then the bottom labels */
+	memcpy(&(nhp->label[nh->labels]), mls->stack, mls->len * sizeof(u32)); /* Then the bottom labels */
       }
       else
       {
 	log(L_WARN "Sum of label stack sizes %d + %d = %d exceedes allowed maximum (%d)",
-	    nh->labels, labels_orig, nhp->labels, MPLS_MAX_LABEL_STACK);
+	    nh->labels, mls->len, nhp->labels, MPLS_MAX_LABEL_STACK);
+	skip_nexthop++;
 	continue;
       }
-      if (ipa_nonzero(nh->gw))
-	nhp->gw = nh->gw;		/* Router nexthop */
-      else if (ipa_nonzero(he->link))
-	nhp->gw = he->link;		/* Device nexthop with link-local address known */
-      else
-	nhp->gw = he->addr;		/* Device nexthop with link-local address unknown */
-
-      nhp = (nhp->next = lp_alloc(rte_update_pool, NEXTHOP_MAX_SIZE));
     }
+    if (ipa_nonzero(nh->gw))
+      nhp->gw = nh->gw;		/* Router nexthop */
+    else if (ipa_nonzero(he->link))
+      nhp->gw = he->link;		/* Device nexthop with link-local address known */
+    else
+      nhp->gw = he->addr;		/* Device nexthop with link-local address unknown */
   }
 
-  memcpy(&(a->nh), nhp, nexthop_size(nhp));
+  if (skip_nexthop)
+    if (nhr)
+      nhr->next = NULL;
+    else
+    {
+      a->dest = RTD_UNREACHABLE;
+      log(L_WARN "No valid nexthop remaining, setting route unreachable");
+      goto no_nexthop;
+    }
 }
 
 static inline rte *
@@ -1823,7 +1848,11 @@ rt_next_hop_update_rte(rtable *tab UNUSED, rte *old)
 {
   rta *a = alloca(RTA_MAX_SIZE);
   memcpy(a, old->attrs, rta_size(old->attrs));
-  rta_apply_hostentry(a, old->attrs->hostentry);
+
+  mpls_label_stack mls = { .len = a->nh.labels_orig };
+  memcpy(mls.stack, &a->nh.label[a->nh.labels - mls.len], mls.len * sizeof(u32));
+
+  rta_apply_hostentry(a, old->attrs->hostentry, &mls);
   a->aflags = 0;
 
   rte *e = sl_alloc(rte_slab);
@@ -2387,13 +2416,25 @@ rt_update_hostentry(rtable *tab, struct hostentry *he)
 	  goto done;
 	}
 
+      he->dest = a->dest;
       he->nexthop_linkable = 1;
-      for (struct nexthop *nh = &(a->nh); nh; nh = nh->next)
+      if (he->dest == RTD_UNICAST)
-	if (ipa_zero(nh->gw))
+	{
-	  {
+	  for (struct nexthop *nh = &(a->nh); nh; nh = nh->next)
-	    he->nexthop_linkable = 0;
+	    if (ipa_zero(nh->gw))
-	    break;
+	      {
-	  }
+		if (if_local_addr(he->addr, nh->iface))
+		  {
+		    /* The host address is a local address, this is not valid */
+		    log(L_WARN "Next hop address %I is a local address of iface %s",
+			he->addr, nh->iface->name);
+		    goto done;
+		  }
+
+		he->nexthop_linkable = 0;
+		break;
+	      }
+	}
 
       he->src = rta_clone(a);
       he->igp_metric = rt_get_igp_metric(e);
@@ -2454,9 +2495,9 @@ rt_get_hostentry(rtable *tab, ip_addr a, ip_addr ll, rtable *dep)
 }
 
 void
-rta_set_recursive_next_hop(rtable *dep, rta *a, rtable *tab, ip_addr gw, ip_addr ll)
+rta_set_recursive_next_hop(rtable *dep, rta *a, rtable *tab, ip_addr gw, ip_addr ll, mpls_label_stack *mls)
 {
-  rta_apply_hostentry(a, rt_get_hostentry(tab, gw, ipa_zero(ll) ? gw : ll, dep));
+  rta_apply_hostentry(a, rt_get_hostentry(tab, gw, ipa_zero(ll) ? gw : ll, dep), mls);
 }
 
 

proto/bgp/bgp.h

@@ -337,6 +337,8 @@ struct bgp_parse_state {
   u32 mp_reach_af;
   u32 mp_unreach_af;
 
+  mpls_label_stack mls;
+
   uint attr_len;
   uint ip_reach_len;
   uint ip_unreach_len;

proto/bgp/packets.c

@@ -753,7 +753,7 @@ bgp_apply_next_hop(struct bgp_parse_state *s, rta *a, ip_addr gw, ip_addr ll)
     if (ipa_zero(gw))
       WITHDRAW(BAD_NEXT_HOP);
 
-    rta_set_recursive_next_hop(c->c.table, a, c->igp_table, gw, ll);
+    rta_set_recursive_next_hop(c->c.table, a, c->igp_table, gw, ll, &(s->mls));
   }
 }
 

proto/static/config.Y

@@ -75,8 +75,7 @@ stat_nexthop:
       this_snh->iface = if_get_by_name($2);
     }
   | stat_nexthop MPLS label_stack {
-    this_snh->label_count = $3[0];
+    this_snh->mls = $3;
-    this_snh->label_stack = &($3[1]);
   }
   | stat_nexthop WEIGHT expr {
     this_snh->weight = $3 - 1;
@@ -111,8 +110,7 @@ stat_route:
  | stat_route0 RECURSIVE ipa MPLS label_stack {
       this_srt->dest = RTDX_RECURSIVE;
       this_srt->via = $3;
-      this_srt->label_count = $5[0];
+      this_srt->mls = $5;
-      this_srt->label_stack = &($5[1]);
    }
  | stat_route0 DROP		{ this_srt->dest = RTD_BLACKHOLE; }
  | stat_route0 REJECT		{ this_srt->dest = RTD_UNREACHABLE; }

proto/static/static.c

@@ -79,8 +79,11 @@ static_announce_rte(struct static_proto *p, struct static_route *r)
       nh->gw = r2->via;
       nh->iface = r2->neigh->iface;
       nh->weight = r2->weight;
-      nh->labels = r2->label_count;
+      if (r2->mls)
-      memcpy(nh->label, r2->label_stack, r2->label_count * sizeof(u32));
+      {
+	nh->labels = r2->mls->len;
+	memcpy(nh->label, r2->mls->stack, r2->mls->len * sizeof(u32));
+      }
 
       nexthop_insert(&nhs, nh);
     }
@@ -92,11 +95,7 @@ static_announce_rte(struct static_proto *p, struct static_route *r)
   }
 
   if (r->dest == RTDX_RECURSIVE)
-  {
+    rta_set_recursive_next_hop(p->p.main_channel->table, a, p_igp_table(p), r->via, IPA_NONE, r->mls);
-    a->nh.labels_orig = a->nh.labels = r->label_count;
-    memcpy(a->nh.label, r->label_stack, r->label_count * sizeof(u32));
-    rta_set_recursive_next_hop(p->p.main_channel->table, a, p_igp_table(p), r->via, IPA_NONE);
-  }
 
   /* Already announced */
   if (r->state == SRS_CLEAN)
@@ -274,17 +273,33 @@ static_same_dest(struct static_route *x, struct static_route *y)
 	  (x->iface != y->iface) ||
 	  (x->use_bfd != y->use_bfd) ||
 	  (x->weight != y->weight) ||
-	  (x->label_count != y->label_count))
+	  (!x->mls != !y->mls) ||
+	  ((x->mls) && (y->mls) && (x->mls->len != y->mls->len)))
 	return 0;
 
-      for (int i = 0; i < x->label_count; i++)
+      if (!x->mls)
-	if (x->label_stack[i] != y->label_stack[i])
+	continue;
+
+      for (uint i = 0; i < x->mls->len; i++)
+	if (x->mls->stack[i] != y->mls->stack[i])
 	  return 0;
     }
     return !x && !y;
 
   case RTDX_RECURSIVE:
-    return ipa_equal(x->via, y->via);
+    if (!ipa_equal(x->via, y->via) ||
+	(!x->mls != !y->mls) ||
+	((x->mls) && (y->mls) && (x->mls->len != y->mls->len)))
+      return 0;
+
+    if (!x->mls)
+      return 1;
+
+    for (uint i = 0; i < x->mls->len; i++)
+      if (x->mls->stack[i] != y->mls->stack[i])
+	return 0;
+
+    return 1;
 
   default:
     return 1;

proto/static/static.h

@@ -42,9 +42,8 @@ struct static_route {
   byte active;				/* Next hop is active (nbr/iface/BFD available) */
   byte weight;				/* Multipath next hop weight */
   byte use_bfd;				/* Configured to use BFD */
-  byte label_count;			/* Number of labels in stack */
   struct bfd_request *bfd_req;		/* BFD request, if BFD is used */
-  u32 *label_stack;			/* Label stack if label_count > 0 */
+  mpls_label_stack *mls;		/* MPLS label stack; may be NULL */
 };
 
 /*