[PATCH 6.6.y] Revert "mctp: no longer rely on net->dev_index_head[]"

[PATCH 6.6.y] Revert "mctp: no longer rely on net->dev_index_head[]"

[Jeremy Kerr]

This reverts commit 2d45eeb7d5d7019b623d513be813123cd048c059 from the
6.6 stable tree.

2d45eeb7d5d7 is the 6.6.y backport of mainline 2d20773aec14.

The switch to for_each_netdev_dump() was predicated on a change in
semantics for the netdev iterator, introduced by f22b4b55edb5 ("net:
make for_each_netdev_dump() a little more bug-proof"). Without that
prior change, we incorrectly repeat the last iteration indefinitely.

2d45eeb was pulled in to stable as context for acab78ae12c7 ("net: mctp:
Don't access ifa_index when missing"), but we're fine without it here,
with a small tweak to the variable declarations as updated patch
context.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
The 6.6.y branch is the only stable release that has the conversion to
for_each_netdev_dump() but not the prereq fix to for_each_netdev_dump(). 
---
 net/mctp/device.c | 50 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/net/mctp/device.c b/net/mctp/device.c
index 8d1386601bbe06487bea46eeae56733124c85098..27aee8b04055f0ad19b24b08117406c303a566cc 100644
--- a/net/mctp/device.c
+++ b/net/mctp/device.c
@@ -20,7 +20,8 @@
 #include <net/sock.h>
 
 struct mctp_dump_cb {
-	unsigned long ifindex;
+	int h;
+	int idx;
 	size_t a_idx;
 };
 
@@ -114,10 +115,12 @@ static int mctp_dump_addrinfo(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct mctp_dump_cb *mcb = (void *)cb->ctx;
 	struct net *net = sock_net(skb->sk);
+	struct hlist_head *head;
 	struct net_device *dev;
 	struct ifaddrmsg *hdr;
 	struct mctp_dev *mdev;
-	int ifindex = 0, rc;
+	int ifindex = 0;
+	int idx = 0, rc;
 
 	/* Filter by ifindex if a header is provided */
 	if (cb->nlh->nlmsg_len >= nlmsg_msg_size(sizeof(*hdr))) {
@@ -131,19 +134,31 @@ static int mctp_dump_addrinfo(struct sk_buff *skb, struct netlink_callback *cb)
 	}
 
 	rcu_read_lock();
-	for_each_netdev_dump(net, dev, mcb->ifindex) {
-		if (ifindex && ifindex != dev->ifindex)
-			continue;
-		mdev = __mctp_dev_get(dev);
-		if (!mdev)
-			continue;
-		rc = mctp_dump_dev_addrinfo(mdev, skb, cb);
-		mctp_dev_put(mdev);
-		if (rc < 0)
-			break;
-		mcb->a_idx = 0;
+	for (; mcb->h < NETDEV_HASHENTRIES; mcb->h++, mcb->idx = 0) {
+		idx = 0;
+		head = &net->dev_index_head[mcb->h];
+		hlist_for_each_entry_rcu(dev, head, index_hlist) {
+			if (idx >= mcb->idx &&
+			    (ifindex == 0 || ifindex == dev->ifindex)) {
+				mdev = __mctp_dev_get(dev);
+				if (mdev) {
+					rc = mctp_dump_dev_addrinfo(mdev,
+								    skb, cb);
+					mctp_dev_put(mdev);
+					// Error indicates full buffer, this
+					// callback will get retried.
+					if (rc < 0)
+						goto out;
+				}
+			}
+			idx++;
+			// reset for next iteration
+			mcb->a_idx = 0;
+		}
 	}
+out:
 	rcu_read_unlock();
+	mcb->idx = idx;
 
 	return skb->len;
 }
@@ -517,12 +532,9 @@ static struct notifier_block mctp_dev_nb = {
 };
 
 static const struct rtnl_msg_handler mctp_device_rtnl_msg_handlers[] = {
-	{.owner = THIS_MODULE, .protocol = PF_MCTP, .msgtype = RTM_NEWADDR,
-	 .doit = mctp_rtm_newaddr},
-	{.owner = THIS_MODULE, .protocol = PF_MCTP, .msgtype = RTM_DELADDR,
-	 .doit = mctp_rtm_deladdr},
-	{.owner = THIS_MODULE, .protocol = PF_MCTP, .msgtype = RTM_GETADDR,
-	 .dumpit = mctp_dump_addrinfo},
+	{THIS_MODULE, PF_MCTP, RTM_NEWADDR, mctp_rtm_newaddr, NULL, 0},
+	{THIS_MODULE, PF_MCTP, RTM_DELADDR, mctp_rtm_deladdr, NULL, 0},
+	{THIS_MODULE, PF_MCTP, RTM_GETADDR, NULL, mctp_dump_addrinfo, 0},
 };
 
 int __init mctp_device_init(void)

---
base-commit: c2603c511feb427b2b09f74b57816a81272932a1
change-id: 20250609-dev-mctp-nl-addrinfo-b23f157735c8

Best regards,
-- 
Jeremy Kerr <jk@codeconstruct.com.au>

Re: [PATCH 6.6.y] Revert "mctp: no longer rely on net->dev_index_head[]"

[Eric Dumazet]

On Sun, Jun 8, 2025 at 9:12 PM Jeremy Kerr <jk@codeconstruct.com.au> wrote:
>
> This reverts commit 2d45eeb7d5d7019b623d513be813123cd048c059 from the
> 6.6 stable tree.
>
> 2d45eeb7d5d7 is the 6.6.y backport of mainline 2d20773aec14.
>
> The switch to for_each_netdev_dump() was predicated on a change in
> semantics for the netdev iterator, introduced by f22b4b55edb5 ("net:
> make for_each_netdev_dump() a little more bug-proof"). Without that
> prior change, we incorrectly repeat the last iteration indefinitely.
>
> 2d45eeb was pulled in to stable as context for acab78ae12c7 ("net: mctp:
> Don't access ifa_index when missing"), but we're fine without it here,
> with a small tweak to the variable declarations as updated patch
> context.
>
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
> ---
> The 6.6.y branch is the only stable release that has the conversion to
> for_each_netdev_dump() but not the prereq fix to for_each_netdev_dump().

I would rather make sure f22b4b55edb5 ("net: make
for_each_netdev_dump() a little more bug-proof")
is backported to kernels using for_each_netdev_dump()

Re: [PATCH 6.6.y] Revert "mctp: no longer rely on net->dev_index_head[]"

[Jeremy Kerr]

Hi Eric,

> I would rather make sure f22b4b55edb5 ("net: make
> for_each_netdev_dump() a little more bug-proof")
> is backported to kernels using for_each_netdev_dump()

Either way works for me, but I assume that changing the semantics of
for_each_netdev_dump() would be fairly risky for a stable series,
especially given the amount of testing 6.6.y has had.

Jakub, as the author of that fix: any preferences?

Cheers,


Jeremy

Re: [PATCH 6.6.y] Revert "mctp: no longer rely on net->dev_index_head[]"

[Jakub Kicinski]

On Mon, 09 Jun 2025 22:13:52 +0800 Jeremy Kerr wrote:
> > I would rather make sure f22b4b55edb5 ("net: make
> > for_each_netdev_dump() a little more bug-proof")
> > is backported to kernels using for_each_netdev_dump()  
> 
> Either way works for me, but I assume that changing the semantics of
> for_each_netdev_dump() would be fairly risky for a stable series,
> especially given the amount of testing 6.6.y has had.
> 
> Jakub, as the author of that fix: any preferences?

We should backport it, I can't think of why someone would depend 
on the old behavior.

Re: [PATCH 6.6.y] Revert "mctp: no longer rely on net->dev_index_head[]"

[Jeremy Kerr]

Hi Jakub,

> > Either way works for me, but I assume that changing the semantics of
> > for_each_netdev_dump() would be fairly risky for a stable series,
> > especially given the amount of testing 6.6.y has had.
> > 
> > Jakub, as the author of that fix: any preferences?
> 
> We should backport it, I can't think of why someone would depend 
> on the old behavior.

OK, sounds good. Thanks for the input, I'll get a patch sent to stable@
shortly.

Cheers,


Jeremy