Fixing FlowSpec validation for v3 internal API

Validation is called internally from route table at the same place where
nexthop resolution is done. Also accounting for rte->sender semantics
change (not a channel but the import hook instead).

3 files changed, 61 insertions, 44 deletions

diff --git a/nest/rt-table.c b/nest/rt-table.c
index 8677c177..491ae1d9 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -122,6 +122,7 @@ static void rt_notify_hostcache(rtable *tab, net *net);
 static void rt_update_hostcache(rtable *tab);
 static void rt_next_hop_update(rtable *tab);
 static inline void rt_next_hop_resolve_rte(rte *r);
+static inline void rt_flowspec_resolve_rte(rte *r, struct channel *c);
 static inline void rt_prune_table(rtable *tab);
 static inline void rt_schedule_notify(rtable *tab);
 static void rt_flowspec_notify(rtable *tab, net *net);
@@ -1570,7 +1571,10 @@ rte_update_direct(struct channel *c, const net_addr *n, rte *new, struct rte_src
 	}
 
       if (new)
-	rt_next_hop_resolve_rte(new);
+	if (net_is_flow(n))
+	  rt_flowspec_resolve_rte(new, c);
+	else
+	  rt_next_hop_resolve_rte(new);
 
       if (new && !rte_validate(c, new))
 	{
@@ -2804,7 +2808,7 @@ rt_flowspec_update_rte(rtable *tab, net *n, rte *r)
   if (rt_get_source_attr(r) != RTS_BGP)
     return NULL;
 
-  struct bgp_channel *bc = (struct bgp_channel *) r->sender;
+  struct bgp_channel *bc = (struct bgp_channel *) SKIP_BACK(struct channel, in_req, r->sender->req);
   if (!bc->base_table)
     return NULL;
 
@@ -2816,7 +2820,7 @@ rt_flowspec_update_rte(rtable *tab, net *n, rte *r)
     return NULL;
 
   rta *a = alloca(RTA_MAX_SIZE);
-  memcpy(a, r->attrs, rta_size(r->attrs));
+  *a = *r->attrs;
   a->dest = dest;
   a->cached = 0;
 
@@ -2830,6 +2834,35 @@ rt_flowspec_update_rte(rtable *tab, net *n, rte *r)
 #endif
 }
 
+static inline void
+rt_flowspec_resolve_rte(rte *r, struct channel *c)
+{
+#ifdef CONFIG_BGP
+  if (rt_get_source_attr(r) != RTS_BGP)
+    return;
+
+  struct bgp_channel *bc = (struct bgp_channel *) c;
+  if (!bc->base_table)
+    return;
+
+  struct bgp_proto *p = (void *) r->src->proto;
+  int valid = rt_flowspec_check(bc->base_table, c->in_req.hook->table, r->net, r->attrs, p->is_interior);
+  int dest = valid ? RTD_NONE : RTD_UNREACHABLE;
+
+  if (dest == r->attrs->dest)
+    return;
+
+  if (r->attrs->cached)
+  {
+    rta *a = tmp_alloc(RTA_MAX_SIZE);
+    *a = *r->attrs;
+    a->cached = 0;
+    r->attrs = a;
+  }
+
+  r->attrs->dest = dest;
+#endif
+}
 
 static inline int
 rt_next_hop_update_net(rtable *tab, net *n)
@@ -2861,6 +2894,9 @@ rt_next_hop_update_net(rtable *tab, net *n)
 	  ? rt_flowspec_update_rte(tab, n, &e->rte)
 	  : rt_next_hop_update_rte(tab, n, &e->rte);
 
+	if (!new)
+	  continue;
+
 	/* Call a pre-comparison hook */
 	/* Not really an efficient way to compute this */
 	if (e->rte.src->proto->rte_recalculate)
@@ -2876,7 +2912,8 @@ rt_next_hop_update_net(rtable *tab, net *n)
 	*k = e = new;
       }
 
-  ASSERT_DIE(pos == count);
+  ASSERT_DIE(pos <= count);
+  count = pos;
 
   /* Find the new best route */
   struct rte_storage **new_best = NULL;
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 11b1c728..ccfda4df 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -2390,25 +2390,28 @@ bgp_get_route_info(rte *e, byte *buf)
 
   buf += bsprintf(buf, " (%d", rt_get_preference(e));
 
-  if (e->pflags & BGP_REF_SUPPRESSED)
-    buf += bsprintf(buf, "-");
+  if (!net_is_flow(e->net))
+  {
+    if (e->pflags & BGP_REF_SUPPRESSED)
+      buf += bsprintf(buf, "-");
 
-  if (rte_stale(e))
-    buf += bsprintf(buf, "s");
+    if (rte_stale(e))
+      buf += bsprintf(buf, "s");
 
-  u64 metric = bgp_total_aigp_metric(e);
-  if (metric < BGP_AIGP_MAX)
-  {
-    buf += bsprintf(buf, "/%lu", metric);
-  }
-  else if (metric = rt_get_igp_metric(e))
-  {
-    if (!rta_resolvable(e->attrs))
-      buf += bsprintf(buf, "/-");
-    else if (metric >= IGP_METRIC_UNKNOWN)
-      buf += bsprintf(buf, "/?");
-    else
-      buf += bsprintf(buf, "/%d", metric);
+    u64 metric = bgp_total_aigp_metric(e);
+    if (metric < BGP_AIGP_MAX)
+    {
+      buf += bsprintf(buf, "/%lu", metric);
+    }
+    else if (metric = rt_get_igp_metric(e))
+    {
+      if (!rta_resolvable(e->attrs))
+	buf += bsprintf(buf, "/-");
+      else if (metric >= IGP_METRIC_UNKNOWN)
+	buf += bsprintf(buf, "/?");
+      else
+	buf += bsprintf(buf, "/%d", metric);
+    }
   }
   buf += bsprintf(buf, ") [");
 
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index ce2848c0..06b563b0 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -1035,21 +1035,6 @@ bgp_apply_mpls_labels(struct bgp_parse_state *s, rta *a, u32 lnum, u32 labels[ln
   }
 }
 
-static void
-bgp_apply_flow_validation(struct bgp_parse_state *s, const net_addr *n, rta *a)
-{
-  struct bgp_channel *c = s->channel;
-  int valid = rt_flowspec_check(c->base_table, c->c.table, n, a, s->proto->is_interior);
-  a->dest = valid ? RTD_NONE : RTD_UNREACHABLE;
-
-  /* Invalidate cached rta if dest changes */
-  if (s->cached_rta && (s->cached_rta->dest != a->dest))
-  {
-    rta_free(s->cached_rta);
-    s->cached_rta = NULL;
-  }
-}
-
 static int
 bgp_match_src(struct bgp_export_state *s, int mode)
 {
@@ -1928,10 +1913,6 @@ bgp_decode_nlri_flow4(struct bgp_parse_state *s, byte *pos, uint len, rta *a)
     net_fill_flow4(n, px, pxlen, pos, flen);
     ADVANCE(pos, len, flen);
 
-    /* Apply validation procedure per RFC 8955 (6) */
-    if (a && s->channel->cf->validate)
-      bgp_apply_flow_validation(s, n, a);
-
     bgp_rte_update(s, n, path_id, a);
   }
 }
@@ -2020,10 +2001,6 @@ bgp_decode_nlri_flow6(struct bgp_parse_state *s, byte *pos, uint len, rta *a)
     net_fill_flow6(n, px, pxlen, pos, flen);
     ADVANCE(pos, len, flen);
 
-    /* Apply validation procedure per RFC 8955 (6) */
-    if (a && s->channel->cf->validate)
-      bgp_apply_flow_validation(s, n, a);
-
     bgp_rte_update(s, n, path_id, a);
   }
 }