[PATCH net-next 0/4] net: shaper: follow ups to recent fixes

[PATCH net-next 0/4] net: shaper: follow ups to recent fixes

[Jakub Kicinski]

As discussed previously on the patch set with real fixes the xa_locking
in shapers is a little confusing, remove it.
Link: https://lore.kernel.org/20260506000628.1501691-2-kuba@kernel.org

The remaining three patches are an attempt to silence AI reviewers,
I believe Sashiko was complaining about these non-issues. Not adding
Reported tags since these are false positives.

Jakub Kicinski (4):
  net: shaper: drop redundant xa_lock() bracketing
  net: shaper: drop unnecessary kfree_rcu in pre_insert
  net: shaper: add a comment why we don't need kfree_rcu() in flush
  net: shaper: add a note that we expect cap dumps to be tiny

 net/shaper/shaper.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

-- 
2.54.0

[PATCH net-next 1/4] net: shaper: drop redundant xa_lock() bracketing

[Jakub Kicinski]

The shaper insertion and update code takes xa_lock() explicitly.
Paolo explained that the locking was purely to avoid re-taking
the lock in loops. But it may be mis-read as if it was expecting
readers to be fenced off by xa_lock. Readers of XArray are purely
under RCU. Remove explicit taking of xa_lock().

All writers to hierarchy->shapers are serialized by the netdev
instance lock (or run after netdev is made inaccessible to readers).

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/shaper/shaper.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index dea9270f3e57..a5b42b697a93 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -429,7 +429,6 @@ static void net_shaper_commit(struct net_shaper_binding *binding,
 	int index;
 	int i;
 
-	xa_lock(&hierarchy->shapers);
 	for (i = 0; i < nr_shapers; ++i) {
 		index = net_shaper_handle_to_index(&shapers[i].handle);
 
@@ -442,7 +441,6 @@ static void net_shaper_commit(struct net_shaper_binding *binding,
 		/* ... publish to lockless readers. */
 		smp_store_release(&cur->valid, true);
 	}
-	xa_unlock(&hierarchy->shapers);
 }
 
 /* Rollback all the tentative inserts from the hierarchy. */
@@ -455,14 +453,12 @@ static void net_shaper_rollback(struct net_shaper_binding *binding)
 	if (!hierarchy)
 		return;
 
-	xa_lock(&hierarchy->shapers);
 	xa_for_each(&hierarchy->shapers, index, cur) {
 		if (cur->valid)
 			continue;
-		__xa_erase(&hierarchy->shapers, index);
+		xa_erase(&hierarchy->shapers, index);
 		kfree_rcu(cur, rcu);
 	}
-	xa_unlock(&hierarchy->shapers);
 }
 
 static int net_shaper_parse_handle(const struct nlattr *attr,
@@ -1472,12 +1468,10 @@ static void net_shaper_flush(struct net_shaper_binding *binding)
 	if (!hierarchy)
 		return;
 
-	xa_lock(&hierarchy->shapers);
 	xa_for_each(&hierarchy->shapers, index, cur) {
-		__xa_erase(&hierarchy->shapers, index);
+		xa_erase(&hierarchy->shapers, index);
 		kfree(cur);
 	}
-	xa_unlock(&hierarchy->shapers);
 
 	kfree(hierarchy);
 }
-- 
2.54.0

[PATCH net-next 2/4] net: shaper: drop unnecessary kfree_rcu in pre_insert

[Jakub Kicinski]

If we fail to insert a node into the XArray in net_shaper_pre_insert()
we can free it directly - it was never visible to the RCU readers.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/shaper/shaper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index a5b42b697a93..33958462e5e9 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -406,7 +406,7 @@ static int net_shaper_pre_insert(struct net_shaper_binding *binding,
 	prev = xa_store(&hierarchy->shapers, index, cur, GFP_KERNEL);
 	if (xa_err(prev)) {
 		NL_SET_ERR_MSG(extack, "Can't insert shaper into device store");
-		kfree_rcu(cur, rcu);
+		kfree(cur);
 		ret = xa_err(prev);
 		goto free_id;
 	}
-- 
2.54.0

[PATCH net-next 3/4] net: shaper: add a comment why we don't need kfree_rcu() in flush

[Jakub Kicinski]

We keep getting misguided patches to fix the flush.
Add a comment.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/shaper/shaper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index 33958462e5e9..5a3b44c5d10f 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -1470,6 +1470,10 @@ static void net_shaper_flush(struct net_shaper_binding *binding)
 
 	xa_for_each(&hierarchy->shapers, index, cur) {
 		xa_erase(&hierarchy->shapers, index);
+		/* No need to use kfree_rcu(), netdev is already unpublished,
+		 * and synchronize_rcu() has been run as part of
+		 * unregister_netdevice().
+		 */
 		kfree(cur);
 	}
 
-- 
2.54.0

[PATCH net-next 4/4] net: shaper: add a note that we expect cap dumps to be tiny

[Jakub Kicinski]

Various AI scan tools may complain that we don't support resuming
the cap dump. This is true, but the cap dumps are tiny.
net_shaper_nl_cap_pre_dumpit() sets up the dump for just
one device, so the size of the dump scales with NET_SHAPER_SCOPE_MAX (3).

We don't expect them to ever need more than a 4kB page.
Document this.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/shaper/shaper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index 5a3b44c5d10f..b65b356da16b 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -1452,6 +1452,8 @@ int net_shaper_nl_cap_get_dumpit(struct sk_buff *skb,
 
 		ret = net_shaper_cap_fill_one(skb, binding, scope, flags,
 					      info);
+		/* cap dumps are tiny, we expect them to fit in a single skb */
+		WARN_ON_ONCE(ret == -EMSGSIZE);
 		if (ret)
 			return ret;
 	}
-- 
2.54.0