[PATCH 1/2] fib6: fix memory leak on delete operation

[PATCH 1/2] fib6: fix memory leak on delete operation

[Vladimir Medvedkin]

When deleting a prefix, the first attempt to get next prefix cannot
return NULL as we will at a minimum get the prefix we are trying to
delete, whereas we were rather interested in whether there are other
prefixes within the same subtree, not counting the prefix we are
deleting. To address this, we check if we have found the exact prefix we
started with, and perform another search to see if there are more
prefixes to be found.

In addition to that, doing the searches with _COVER rather than _ALL is
incorrect, because if we are doing search with _COVER rather than _ALL, we
do not dive into the tree to find more specific prefixes within the
prefix we are going to delete, and thus will not notice if our subtree
also has more specific prefixes. To fix it, perform first (and
subsequent) searches with _ALL rather than _COVER.

Finally, when we hit the "tmp == NULL" branch (meaning, when we are
deleting the only node that exists in our subtree), we know that the
rib6_lookup will always return us the node that we are trying to delete,
but this is incorrect because further code will consider this to be our
parent node. Address this by doing another search to find the parent of
the current node.

Fixes: c3e12e0f0354 ("fib: add dataplane algorithm for IPv6")
Cc: stable@dpdk.org

Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
 lib/fib/trie.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/fib/trie.c b/lib/fib/trie.c
index ff4c750952..5a9978b4ca 100644
--- a/lib/fib/trie.c
+++ b/lib/fib/trie.c
@@ -516,7 +516,7 @@ trie_modify(struct rte_fib6 *fib, const struct rte_ipv6_addr *ip,
 	struct rte_rib6_node *tmp = NULL;
 	struct rte_rib6_node *node;
 	struct rte_rib6_node *parent;
-	struct rte_ipv6_addr ip_masked;
+	struct rte_ipv6_addr ip_masked, tmp_ip;
 	int ret = 0;
 	uint64_t par_nh, node_nh;
 	uint8_t tmp_depth, depth_diff = 0, parent_depth = 24;
@@ -535,9 +535,25 @@ trie_modify(struct rte_fib6 *fib, const struct rte_ipv6_addr *ip,
 	if (depth > 24) {
 		tmp = rte_rib6_get_nxt(rib, &ip_masked,
 			RTE_ALIGN_FLOOR(depth, 8), NULL,
-			RTE_RIB6_GET_NXT_COVER);
+			RTE_RIB6_GET_NXT_ALL);
+		if (tmp && op == RTE_FIB6_DEL) {
+			/* in case of delete operation, skip the prefix we are going to delete */
+			rte_rib6_get_ip(tmp, &tmp_ip);
+			rte_rib6_get_depth(tmp, &tmp_depth);
+			if (rte_ipv6_addr_eq(&ip_masked, &tmp_ip) && depth == tmp_depth)
+				tmp = rte_rib6_get_nxt(rib, &ip_masked,
+					RTE_ALIGN_FLOOR(depth, 8), tmp, RTE_RIB6_GET_NXT_ALL);
+		}
+
 		if (tmp == NULL) {
 			tmp = rte_rib6_lookup(rib, ip);
+			/**
+			 * in case of delete operation, lookup returns the prefix
+			 * we are going to delete. Find the parent.
+			 */
+			if (tmp && op == RTE_FIB6_DEL)
+				tmp = rte_rib6_lookup_parent(tmp);
+
 			if (tmp != NULL) {
 				rte_rib6_get_depth(tmp, &tmp_depth);
 				parent_depth = RTE_MAX(tmp_depth, 24);
-- 
2.43.0

[PATCH 2/2] fib6: fix tbl8 allocation check logic

[Vladimir Medvedkin]

Currently if there were 'n' preallocated tbl8 entries only 'n - 1' were
able to be used. Fix the logic allowing to use all preallocated tbl8
entries.

Fixes: c3e12e0f0354 ("fib: add dataplane algorithm for IPv6")
Cc: stable@dpdk.org

Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
 lib/fib/trie.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/fib/trie.c b/lib/fib/trie.c
index 5a9978b4ca..6427920c58 100644
--- a/lib/fib/trie.c
+++ b/lib/fib/trie.c
@@ -576,8 +576,7 @@ trie_modify(struct rte_fib6 *fib, const struct rte_ipv6_addr *ip,
 			return 0;
 		}
 
-		if ((depth > 24) && (dp->rsvd_tbl8s >=
-				dp->number_tbl8s - depth_diff))
+		if ((depth > 24) && (dp->rsvd_tbl8s + depth_diff > dp->number_tbl8s))
 			return -ENOSPC;
 
 		node = rte_rib6_insert(rib, &ip_masked, depth);
-- 
2.43.0

Re: [PATCH 2/2] fib6: fix tbl8 allocation check logic

[Robin Jarry]

Vladimir Medvedkin, Oct 14, 2025 at 20:17:
> Currently if there were 'n' preallocated tbl8 entries only 'n - 1' were
> able to be used. Fix the logic allowing to use all preallocated tbl8
> entries.
>
> Fixes: c3e12e0f0354 ("fib: add dataplane algorithm for IPv6")
> Cc: stable@dpdk.org
>
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---

Tested-by: Robin Jarry <rjarry@redhat.com>

-- 
Robin

> This is not an offer to sell securities.

Re: [PATCH 1/2] fib6: fix memory leak on delete operation

[Robin Jarry]

Vladimir Medvedkin, Oct 14, 2025 at 20:17:
> When deleting a prefix, the first attempt to get next prefix cannot
> return NULL as we will at a minimum get the prefix we are trying to
> delete, whereas we were rather interested in whether there are other
> prefixes within the same subtree, not counting the prefix we are
> deleting. To address this, we check if we have found the exact prefix we
> started with, and perform another search to see if there are more
> prefixes to be found.
>
> In addition to that, doing the searches with _COVER rather than _ALL is
> incorrect, because if we are doing search with _COVER rather than _ALL, we
> do not dive into the tree to find more specific prefixes within the
> prefix we are going to delete, and thus will not notice if our subtree
> also has more specific prefixes. To fix it, perform first (and
> subsequent) searches with _ALL rather than _COVER.
>
> Finally, when we hit the "tmp == NULL" branch (meaning, when we are
> deleting the only node that exists in our subtree), we know that the
> rib6_lookup will always return us the node that we are trying to delete,
> but this is incorrect because further code will consider this to be our
> parent node. Address this by doing another search to find the parent of
> the current node.
>
> Fixes: c3e12e0f0354 ("fib: add dataplane algorithm for IPv6")
> Cc: stable@dpdk.org
>
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>

Tested-by: Robin Jarry <rjarry@redhat.com>

-- 
Robin

> Use extra care when cleaning on stairs.