Re: Question about MPLS L3VPN

[Ondrej Zajicek]

On Sat, Jan 27, 2024 at 02:59:57PM +0100, Ondrej Zajicek wrote:
> On Sat, Jan 27, 2024 at 01:42:57PM +0100, Marcel Menzel via Bird-users wrote:
> > Hello List,
> > 
> > I am doing a test setup with the new MPLS L3VPN feature (for background
> > information: I am running IPsec with L2TP on top, with OSPF as IGP and iBGP
> > between loopbacks)
> > with one router receiving an IPv6 fulltable via eBGP and the other one
> > establishing the BGP session through a tunnel described above, and I noticed
> > periodic high CPU usage on the router receiving the routes at the end of the
> > tunnel.
> > 
> > It seems, that BIRD keeps exporting the whole table to the kernel every 30
> > seconds, as it is configured in the kernel protocol scan time with the "vrf"
> > directive set:
> 
> Hello
> 
> It is possible that BIRD exports some route but receives back from kernel
> something slightly different than it expected, so it will try to re-send
> it during the scan.
>
> ... and i just noticed that the bug manifests even in my test case, so
> i will check it.

Fixed:

https://gitlab.nic.cz/labs/bird/-/commit/f40e2bc270d3635be518ae80251ce0b5c519c6f4

You can try attached patch.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
"To err is human -- to blame it on a computer is even more so."

commit f40e2bc270d3635be518ae80251ce0b5c519c6f4
Author: Ondrej Zajicek <santi...@crfreenet.org>
Date:   Sat Jan 27 17:38:06 2024 +0100

    Nest: Fix bug in recursive routes with MPLS-labeled nexthops
    
    When a recursive route with MPLS-labeled nexthop was exported to kernel
    and read back, the nexthop_same() failed due to different labels_orig
    field and kernel protocol reinstalled it unnecessarily.
    
    For comparing hext hops, route cache has to distinguish ones with
    different labels_orig, but KRT has to ignore that, so we need two
    nexthop compare functions.
    
    Thanks to Marcel Menzel for the bugreport.

diff --git a/nest/route.h b/nest/route.h
index d26a4b8c..e6f6c64a 100644
--- a/nest/route.h
+++ b/nest/route.h
@@ -694,6 +694,9 @@ static inline size_t nexthop_size(const struct nexthop *nh)
 int nexthop__same(struct nexthop *x, struct nexthop *y); /* Compare multipath nexthops */
 static inline int nexthop_same(struct nexthop *x, struct nexthop *y)
 { return (x == y) || nexthop__same(x, y); }
+int nexthop_equal_(struct nexthop *x, struct nexthop *y); /* Compare multipath nexthops, ignore labels_orig */
+static inline int nexthop_equal(struct nexthop *x, struct nexthop *y)
+{ return (x == y) || nexthop_equal_(x, y); }
 struct nexthop *nexthop_merge(struct nexthop *x, struct nexthop *y, int rx, int ry, int max, linpool *lp);
 struct nexthop *nexthop_sort(struct nexthop *x);
 static inline void nexthop_link(struct rta *a, const struct nexthop *from)
diff --git a/nest/rt-attr.c b/nest/rt-attr.c
index 7f3645ee..af864bdf 100644
--- a/nest/rt-attr.c
+++ b/nest/rt-attr.c
@@ -185,20 +185,40 @@ nexthop_hash(struct nexthop *x)
   return h;
 }
 
+static inline int
+nexthop_equal_1(struct nexthop *x, struct nexthop *y)
+{
+  if (!ipa_equal(x->gw, y->gw) || (x->iface != y->iface) ||
+      (x->flags != y->flags) || (x->weight != y->weight) ||
+      (x->labels != y->labels))
+    return 0;
+
+  for (int i = 0; i < x->labels; i++)
+    if (x->label[i] != y->label[i])
+      return 0;
+
+  return 1;
+}
+
 int
-nexthop__same(struct nexthop *x, struct nexthop *y)
+nexthop_equal_(struct nexthop *x, struct nexthop *y)
 {
+  /* Like nexthop_same(), but ignores difference between local labels and labels from hostentry */
+
   for (; x && y; x = x->next, y = y->next)
-  {
-    if (!ipa_equal(x->gw, y->gw) || (x->iface != y->iface) ||
-	(x->flags != y->flags) || (x->weight != y->weight) ||
-	(x->labels_orig != y->labels_orig) || (x->labels != y->labels))
+    if (!nexthop_equal_1(x, y))
       return 0;
 
-    for (int i = 0; i < x->labels; i++)
-      if (x->label[i] != y->label[i])
-	return 0;
-  }
+  return x == y;
+}
+
+int
+nexthop__same(struct nexthop *x, struct nexthop *y)
+{
+  for (; x && y; x = x->next, y = y->next)
+    if (!nexthop_equal_1(x, y) ||
+	(x->labels_orig != y->labels_orig))
+      return 0;
 
   return x == y;
 }
diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c
index 3a4b24dc..7a078fb9 100644
--- a/sysdep/unix/krt.c
+++ b/sysdep/unix/krt.c
@@ -619,7 +619,7 @@ krt_same_dest(rte *k, rte *e)
     return 0;
 
   if (ka->dest == RTD_UNICAST)
-    return nexthop_same(&(ka->nh), &(ea->nh));
+    return nexthop_equal(&(ka->nh), &(ea->nh));
 
   return 1;
 }