frr/SOURCES/0006-noprefixroute-network-manager.patch

From f450e1cda41f1b7576094a0b3017ba9849cd55ae Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Fri, 7 Jun 2024 12:30:59 -0400
Subject: [PATCH 1/5] zebra: Make p and src_p const for rib_delete

The prefix'es p and src_p are not const.  Let's make
them so.  Useful to signal that we will not change this
data.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
 zebra/rib.h       | 2 +-
 zebra/zebra_rib.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/zebra/rib.h b/zebra/rib.h
index 84ea766c4733..7f4e3949e02d 100644
--- a/zebra/rib.h
+++ b/zebra/rib.h
@@ -395,7 +395,7 @@ extern int rib_add_multipath_nhe(afi_t afi, safi_t safi, struct prefix *p,
 
 extern void rib_delete(afi_t afi, safi_t safi, vrf_id_t vrf_id, int type,
 		       unsigned short instance, uint32_t flags,
-		       struct prefix *p, struct prefix_ipv6 *src_p,
+		       const struct prefix *p, const struct prefix_ipv6 *src_p,
 		       const struct nexthop *nh, uint32_t nhe_id,
 		       uint32_t table_id, uint32_t metric, uint8_t distance,
 		       bool fromkernel);
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 59190e9dd330..c1bd61e1db31 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -4393,8 +4393,8 @@ int rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p,
 }
 
 void rib_delete(afi_t afi, safi_t safi, vrf_id_t vrf_id, int type,
-		unsigned short instance, uint32_t flags, struct prefix *p,
-		struct prefix_ipv6 *src_p, const struct nexthop *nh,
+		unsigned short instance, uint32_t flags, const struct prefix *p,
+		const struct prefix_ipv6 *src_p, const struct nexthop *nh,
 		uint32_t nhe_id, uint32_t table_id, uint32_t metric,
 		uint8_t distance, bool fromkernel)
 {

From bdfccf69fa128c51c45bbd3528788f72ac17d854 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Fri, 7 Jun 2024 12:56:35 -0400
Subject: [PATCH 2/5] zebra: Expose rib_update_handle_vrf_all

This function will be used on interface down
events to allow for kernel routes to be cleaned
up.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
 zebra/rib.h       | 2 ++
 zebra/zebra_rib.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/zebra/rib.h b/zebra/rib.h
index 7f4e3949e02d..8792fb7908ac 100644
--- a/zebra/rib.h
+++ b/zebra/rib.h
@@ -477,6 +477,8 @@ extern uint8_t route_distance(int type);
 extern void zebra_rib_evaluate_rn_nexthops(struct route_node *rn, uint32_t seq,
 					   bool rt_delete);
 
+extern void rib_update_handle_vrf_all(enum rib_update_event event, int rtype);
+
 /*
  * rib_find_rn_from_ctx
  *
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index c1bd61e1db31..649450b5c63c 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -4543,7 +4543,7 @@ void rib_update_table(struct route_table *table, enum rib_update_event event,
 	}
 }
 
-static void rib_update_handle_vrf_all(enum rib_update_event event, int rtype)
+void rib_update_handle_vrf_all(enum rib_update_event event, int rtype)
 {
 	struct zebra_router_table *zrt;
 

From d528c02a204086da0d542d5655b8724de681a65c Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Fri, 7 Jun 2024 13:50:07 -0400
Subject: [PATCH 3/5] zebra: Handle kernel routes appropriately

Current code intentionally ignores kernel routes.  Modify
zebra to allow these routes to be read in on linux.  Also
modify zebra to look to see if a route should be treated
as a connected and mark it as such.

Additionally this should properly handle some of the issues
being seen with NOPREFIXROUTE.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
 zebra/interface.c  |   2 +
 zebra/rib.h        |   1 +
 zebra/rt_netlink.c |   2 -
 zebra/zebra_rib.c  | 105 +++++++++++++++++++++++++++++++++++++++------
 4 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/zebra/interface.c b/zebra/interface.c
index 03b710e1a0f9..d146004781a5 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -1058,6 +1058,8 @@ void if_down(struct interface *ifp)
 
 	/* Delete all neighbor addresses learnt through IPv6 RA */
 	if_down_del_nbr_connected(ifp);
+
+	rib_update_handle_vrf_all(RIB_UPDATE_INTERFACE_DOWN, ZEBRA_ROUTE_KERNEL);
 }
 
 void if_refresh(struct interface *ifp)
diff --git a/zebra/rib.h b/zebra/rib.h
index 8792fb7908ac..cd6efbfb36dd 100644
--- a/zebra/rib.h
+++ b/zebra/rib.h
@@ -326,6 +326,7 @@ typedef struct rib_tables_iter_t_ {
 
 /* Events/reasons triggering a RIB update. */
 enum rib_update_event {
+	RIB_UPDATE_INTERFACE_DOWN,
 	RIB_UPDATE_KERNEL,
 	RIB_UPDATE_RMAP_CHANGE,
 	RIB_UPDATE_OTHER,
diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index c22145be693b..ddcb83cd8ce7 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -799,8 +799,6 @@ int netlink_route_change_read_unicast_internal(struct nlmsghdr *h,
 		return 0;
 	if (rtm->rtm_protocol == RTPROT_REDIRECT)
 		return 0;
-	if (rtm->rtm_protocol == RTPROT_KERNEL)
-		return 0;
 
 	selfroute = is_selfroute(rtm->rtm_protocol);
 
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 649450b5c63c..2d6c5148833a 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -1619,6 +1619,10 @@ static bool rib_compare_routes(const struct route_entry *re1,
 	 * v6 link-locals, and we also support multiple addresses in the same
 	 * subnet on a single interface.
 	 */
+	if (re1->type == ZEBRA_ROUTE_CONNECT &&
+	    (re1->nhe->nhg.nexthop->ifindex == re2->nhe->nhg.nexthop->ifindex))
+		return true;
+
 	if (re1->type != ZEBRA_ROUTE_CONNECT && re1->type != ZEBRA_ROUTE_LOCAL)
 		return true;
 
@@ -2863,10 +2867,11 @@ static void process_subq_early_route_add(struct zebra_early_route *ere)
 
 	/* Link new re to node.*/
 	if (IS_ZEBRA_DEBUG_RIB) {
-		rnode_debug(
-			rn, re->vrf_id,
-			"Inserting route rn %p, re %p (%s) existing %p, same_count %d",
-			rn, re, zebra_route_string(re->type), same, same_count);
+		rnode_debug(rn, re->vrf_id,
+			    "Inserting route rn %p, re %p (%s/%s/%s) existing %p, same_count %d",
+			    rn, re, zebra_route_string(re->type),
+			    afi2str(ere->afi), safi2str(ere->safi), same,
+			    same_count);
 
 		if (IS_ZEBRA_DEBUG_RIB_DETAILED)
 			route_entry_dump(
@@ -4383,6 +4388,34 @@ int rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p,
 		nhe.id = re->nhe_id;
 
 	n = zebra_nhe_copy(&nhe, 0);
+
+	if (re->type == ZEBRA_ROUTE_KERNEL) {
+		struct interface *ifp;
+		struct connected *connected;
+
+		if (p->family == AF_INET6 &&
+		    IN6_IS_ADDR_LINKLOCAL(&p->u.prefix6)) {
+			zebra_nhg_free(n);
+			zebra_rib_route_entry_free(re);
+			return -1;
+		}
+
+		ifp = if_lookup_prefix(p, re->vrf_id);
+		if (ifp) {
+			connected = connected_lookup_prefix(ifp, p);
+
+			if (connected && !CHECK_FLAG(connected->flags,
+						     ZEBRA_IFA_NOPREFIXROUTE)) {
+				zebra_nhg_free(n);
+				zebra_rib_route_entry_free(re);
+				return -1;
+			}
+
+			if (ifp->ifindex == ng->nexthop->ifindex)
+				re->type = ZEBRA_ROUTE_CONNECT;
+		}
+	}
+
 	ret = rib_add_multipath_nhe(afi, safi, p, src_p, re, n, startup);
 
 	/* In error cases, free the route also */
@@ -4458,6 +4491,9 @@ static const char *rib_update_event2str(enum rib_update_event event)
 	const char *ret = "UNKNOWN";
 
 	switch (event) {
+	case RIB_UPDATE_INTERFACE_DOWN:
+		ret = "RIB_UPDATE_INTERFACE_DOWN";
+		break;
 	case RIB_UPDATE_KERNEL:
 		ret = "RIB_UPDATE_KERNEL";
 		break;
@@ -4474,15 +4510,56 @@ static const char *rib_update_event2str(enum rib_update_event event)
 	return ret;
 }
 
+/*
+ * We now keep kernel routes, but we don't have any
+ * trigger events for them when they are implicitly
+ * deleted.  Since we are already walking the
+ * entire table on a down event let's look at
+ * the few kernel routes we may have
+ */
+static void
+rib_update_handle_kernel_route_down_possibility(struct route_node *rn,
+						struct route_entry *re)
+{
+	struct nexthop *nexthop = NULL;
+	bool alive = false;
+
+	for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) {
+		struct interface *ifp = if_lookup_by_index(nexthop->ifindex,
+							   nexthop->vrf_id);
+
+		if (ifp && if_is_up(ifp)) {
+			alive = true;
+			break;
+		}
+	}
+
+	if (!alive) {
+		struct rib_table_info *rib_table = srcdest_rnode_table_info(rn);
+		const struct prefix *p;
+		const struct prefix_ipv6 *src_p;
+
+		srcdest_rnode_prefixes(rn, &p, (const struct prefix **)&src_p);
+
+		rib_delete(rib_table->afi, rib_table->safi, re->vrf_id,
+			   re->type, re->instance, re->flags, p, src_p, NULL, 0,
+			   re->table, re->metric, re->distance, true);
+	}
+}
+
 
 /* Schedule route nodes to be processed if they match the type */
-static void rib_update_route_node(struct route_node *rn, int type)
+static void rib_update_route_node(struct route_node *rn, int type,
+				  enum rib_update_event event)
 {
 	struct route_entry *re, *next;
 	bool re_changed = false;
 
 	RNODE_FOREACH_RE_SAFE (rn, re, next) {
-		if (type == ZEBRA_ROUTE_ALL || type == re->type) {
+		if (event == RIB_UPDATE_INTERFACE_DOWN && type == re->type &&
+		    type == ZEBRA_ROUTE_KERNEL)
+			rib_update_handle_kernel_route_down_possibility(rn, re);
+		else if (type == ZEBRA_ROUTE_ALL || type == re->type) {
 			SET_FLAG(re->status, ROUTE_ENTRY_CHANGED);
 			re_changed = true;
 		}
@@ -4522,20 +4599,24 @@ void rib_update_table(struct route_table *table, enum rib_update_event event,
 		/*
 		 * If we are looking at a route node and the node
 		 * has already been queued  we don't
-		 * need to queue it up again
+		 * need to queue it up again, unless it is
+		 * an interface down event as that we need
+		 * to process this no matter what.
 		 */
-		if (rn->info
-		    && CHECK_FLAG(rib_dest_from_rnode(rn)->flags,
-				  RIB_ROUTE_ANY_QUEUED))
+		if (rn->info &&
+		    CHECK_FLAG(rib_dest_from_rnode(rn)->flags,
+			       RIB_ROUTE_ANY_QUEUED) &&
+		    event != RIB_UPDATE_INTERFACE_DOWN)
 			continue;
 
 		switch (event) {
+		case RIB_UPDATE_INTERFACE_DOWN:
 		case RIB_UPDATE_KERNEL:
-			rib_update_route_node(rn, ZEBRA_ROUTE_KERNEL);
+			rib_update_route_node(rn, ZEBRA_ROUTE_KERNEL, event);
 			break;
 		case RIB_UPDATE_RMAP_CHANGE:
 		case RIB_UPDATE_OTHER:
-			rib_update_route_node(rn, rtype);
+			rib_update_route_node(rn, rtype, event);
 			break;
 		case RIB_UPDATE_MAX:
 			break;

From 9bc0cd8241f39e4fd751edfa52c09fae6db2db1c Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 26 Jun 2024 13:21:38 -0400
Subject: [PATCH 4/5] zebra: Prevent accidental re memory leak in odd case

There exists a path in rib_add_multipath where if a decision
is made to not use the passed in re, we just drop the memory
instead of freeing it.  Let's free it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
 zebra/zebra_rib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 2d6c5148833a..b4baee148aef 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -4375,8 +4375,10 @@ int rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p,
 		return -1;
 
 	/* We either need nexthop(s) or an existing nexthop id */
-	if (ng == NULL && re->nhe_id == 0)
+	if (ng == NULL && re->nhe_id == 0) {
+		zebra_rib_route_entry_free(re);
 		return -1;
+	}
 
 	/*
 	 * Use a temporary nhe to convey info to the common/main api.

From 37dd51867f2b98f0fb616fc3cf9922240346fd19 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Thu, 15 Aug 2024 16:02:55 -0400
Subject: [PATCH 5/5] tests: Add some tests to show new behavior works as
 expected

a) A noprefix address by itself should not create a connected route.
   This was pre-existing.
b) A noprefix address with a corresponding route should result in a
   connected route.  This is how NetworkManager appears to work.
   This is new behavior, so a new test.
c) A route is added to the system from someone else.
   This is new behavior, so a new test.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
 .../r1/ip_route_connected.json                | 24 +++++++++++
 .../r1/ip_route_kernel.json                   | 24 +++++++++++
 .../test_zebra_multiple_connected.py          | 43 +++++++++++++++++++
 3 files changed, 91 insertions(+)
 create mode 100644 tests/topotests/zebra_multiple_connected/r1/ip_route_connected.json
 create mode 100644 tests/topotests/zebra_multiple_connected/r1/ip_route_kernel.json

diff --git a/tests/topotests/zebra_multiple_connected/r1/ip_route_connected.json b/tests/topotests/zebra_multiple_connected/r1/ip_route_connected.json
new file mode 100644
index 000000000000..db03ce84a6a4
--- /dev/null
+++ b/tests/topotests/zebra_multiple_connected/r1/ip_route_connected.json
@@ -0,0 +1,24 @@
+{
+ "192.168.44.0/24":[
+    {
+      "prefix":"192.168.44.0/24",
+      "prefixLen":24,
+      "protocol":"connected",
+      "vrfName":"default",
+      "selected":true,
+      "destSelected":true,
+      "distance":0,
+      "metric":0,
+      "installed":true,
+      "table":254,
+      "nexthops":[
+        {
+            "fib":true,
+            "directlyConnected":true,
+            "interfaceName":"r1-eth1",
+            "active":true
+        }
+      ]
+    }
+  ]
+}
diff --git a/tests/topotests/zebra_multiple_connected/r1/ip_route_kernel.json b/tests/topotests/zebra_multiple_connected/r1/ip_route_kernel.json
new file mode 100644
index 000000000000..22465cb477d3
--- /dev/null
+++ b/tests/topotests/zebra_multiple_connected/r1/ip_route_kernel.json
@@ -0,0 +1,24 @@
+{
+  "4.5.6.7/32":[
+    {
+      "prefix":"4.5.6.7/32",
+      "prefixLen":32,
+      "protocol":"kernel",
+      "vrfName":"default",
+      "selected":true,
+      "destSelected":true,
+      "distance":0,
+      "metric":0,
+      "installed":true,
+      "table":254,
+      "nexthops":[
+        {
+          "fib":true,
+          "directlyConnected":true,
+          "interfaceName":"r1-eth1",
+          "active":true
+        }
+      ]
+    }
+  ]
+}
diff --git a/tests/topotests/zebra_multiple_connected/test_zebra_multiple_connected.py b/tests/topotests/zebra_multiple_connected/test_zebra_multiple_connected.py
index dc47527c74c2..7dbeb6f1ccb3 100644
--- a/tests/topotests/zebra_multiple_connected/test_zebra_multiple_connected.py
+++ b/tests/topotests/zebra_multiple_connected/test_zebra_multiple_connected.py
@@ -19,6 +19,9 @@
 import pytest
 import json
 from functools import partial
+from lib.topolog import logger
+
+pytestmark = pytest.mark.random_order(disabled=True)
 
 # Save the Current Working Directory to find configuration files.
 CWD = os.path.dirname(os.path.realpath(__file__))
@@ -159,6 +162,46 @@ def test_zebra_noprefix_connected():
     assert result, "Connected Route should not have been added"
 
 
+def test_zebra_noprefix_connected_add():
+    "Test that a noprefixroute created with a manual route works as expected, this is for NetworkManager"
+
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears["r1"]
+    router.run("ip route add 192.168.44.0/24 dev r1-eth1")
+
+    connected = "{}/{}/ip_route_connected.json".format(CWD, router.name)
+    expected = json.loads(open(connected).read())
+
+    test_func = partial(
+        topotest.router_json_cmp, router, "show ip route 192.168.44.0/24 json", expected
+    )
+    result, _ = topotest.run_and_expect(test_func, None, count=20, wait=1)
+    assert result, "Connected Route should have been added\n{}".format(_)
+
+
+def test_zebra_kernel_route_add():
+    "Test that a random kernel route is properly handled as expected"
+
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    router = tgen.gears["r1"]
+    router.run("ip route add 4.5.6.7/32 dev r1-eth1")
+
+    kernel = "{}/{}/ip_route_kernel.json".format(CWD, router.name)
+    expected = json.loads(open(kernel).read())
+
+    test_func = partial(
+        topotest.router_json_cmp, router, "show ip route 4.5.6.7/32 json", expected
+    )
+    result, _ = topotest.run_and_expect(test_func, None, count=20, wait=1)
+    assert result, "Connected Route should have been added\n{}".format(_)
+
+
 if __name__ == "__main__":
     args = ["-s"] + sys.argv[1:]
     sys.exit(pytest.main(args))