ASPA: Unified the ASPA_INVALID into one result

The _EMPTY and _CONFED variants are easy to spot bare-eyed from the AS path.
hack3ric · Nov 26, 2024 · 997d2f5 · 997d2f5
1 parent 0137759
commit 997d2f5
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 13 deletions.
diff --git a/doc/bird.sgml b/doc/bird.sgml
@@ -1961,6 +1961,9 @@ of a set" operation - it can be used on:
   and for <cf>aspa_check_upstream</cf> it is
   <cf>aspa_check(<m/table/, bgp_path, true)</cf>.
   Note: the ASPA check does not include the local ASN in the AS path.
+  Also, <cf>ASPA_INVALID</cf> is returned for an empty AS path
+  or for AS path containing <cf>CONFED_SET</cf> or <cf>CONFED_SEQUENCE</cf> blocks,
+  as the (draft) stipulates.
 </itemize>
 
 <p>The following example checks for ROA and ASPA on routes from a customer:

diff --git a/filter/test.conf b/filter/test.conf
@@ -2267,11 +2267,11 @@ function t_aspa_check()
 
 	p1.prepend(65542);
 	bt_assert(aspa_check(at, p1, false) = ASPA_VALID);
-	bt_assert(aspa_check(at, p1, true) = ASPA_INVALID_LEAK);
+	bt_assert(aspa_check(at, p1, true) = ASPA_INVALID);
 
 	p1.prepend(65555);
 	bt_assert(aspa_check(at, p1, false) = ASPA_UNKNOWN);
-	bt_assert(aspa_check(at, p1, true) = ASPA_INVALID_LEAK);
+	bt_assert(aspa_check(at, p1, true) = ASPA_INVALID);
 
 	bgppath p2 = +empty+;
 	p2.prepend(65554);
@@ -2282,13 +2282,13 @@ function t_aspa_check()
 
 	p2.prepend(65543);
 	bt_assert(aspa_check(at, p2, false) = ASPA_UNKNOWN);
-	bt_assert(aspa_check(at, p2, true) = ASPA_INVALID_LEAK);
+	bt_assert(aspa_check(at, p2, true) = ASPA_INVALID);
 
 	bgppath p3 = +empty+;
 	p3.prepend(65541);
 	p3.prepend(65544);
-	bt_assert(aspa_check(at, p3, false) = ASPA_INVALID_LEAK);
-	bt_assert(aspa_check(at, p3, true) = ASPA_INVALID_LEAK);
+	bt_assert(aspa_check(at, p3, false) = ASPA_INVALID);
+	bt_assert(aspa_check(at, p3, true) = ASPA_INVALID);
 }
 
 bt_test_suite(t_aspa_check, "Testing ASPA");

diff --git a/nest/config.Y b/nest/config.Y
@@ -139,7 +139,7 @@ CF_ENUM(T_ENUM_RTS, RTS_, STATIC, INHERIT, DEVICE, STATIC_DEVICE, REDIRECT,
 CF_ENUM(T_ENUM_SCOPE, SCOPE_, HOST, LINK, SITE, ORGANIZATION, UNIVERSE, UNDEFINED)
 CF_ENUM(T_ENUM_RTD, RTD_, UNICAST, BLACKHOLE, UNREACHABLE, PROHIBIT)
 CF_ENUM(T_ENUM_ROA, ROA_, UNKNOWN, VALID, INVALID)
-CF_ENUM(T_ENUM_ASPA, ASPA_, UNKNOWN, VALID, INVALID_EMPTY, INVALID_LEAK, INVALID_CONFED)
+CF_ENUM(T_ENUM_ASPA, ASPA_, UNKNOWN, VALID, INVALID)
 CF_ENUM_PX(T_ENUM_AF, AF_, AFI_, IPV4, IPV6)
 CF_ENUM(T_ENUM_MPLS_POLICY, MPLS_POLICY_, NONE, STATIC, PREFIX, AGGREGATE, VRF)
 

diff --git a/nest/route.h b/nest/route.h
@@ -786,9 +786,7 @@ int rt_flowspec_check(rtable *tab_ip, rtable *tab_flow, const net_addr *n, rta *
 enum aspa_result {
   ASPA_UNKNOWN = 0,
   ASPA_VALID,
-  ASPA_INVALID_EMPTY,
-  ASPA_INVALID_CONFED,
-  ASPA_INVALID_LEAK,
+  ASPA_INVALID,
 };
 
 #endif
diff --git a/nest/rt-table.c b/nest/rt-table.c
@@ -360,12 +360,12 @@ enum aspa_result aspa_check(rtable *tab, const adata *path, bool force_upstream)
 
   /* No support for confed paths */
   if (as_path_contains_confed(path))
-    return ASPA_INVALID_CONFED;
+    return ASPA_INVALID;
 
   /* Check path length */
   uint len = as_path_getlen(path);
   if (len == 0)
-    return ASPA_INVALID_EMPTY;
+    return ASPA_INVALID;
 
   /* Normalize the AS Path: drop stuffings */
   u32 *asns = alloca(sizeof(u32) * len);
@@ -420,7 +420,7 @@ end_of_aspa:;
 	min_up = ap;
       else if (ap && !up)
 	/* Exists but doesn't allow this upstream */
-	return ASPA_INVALID_LEAK;
+	return ASPA_INVALID;
     }
 
     /* Fast path for no ASPA here */
@@ -468,7 +468,7 @@ end_of_aspa:;
     return ASPA_UNKNOWN;
 
   /* Now there is surely a valley there. */
-  return ASPA_INVALID_LEAK;
+  return ASPA_INVALID;
 }
 
 /**