[PATCH net-next 0/4] netdev: address a handful of nit picks

[PATCH net-next 0/4] netdev: address a handful of nit picks

[Jakub Kicinski]

I got slightly concerned about the state of the netdev-genl implementation
after Gemini found the bug fixed by commit c849de7d8757 ("netdev: fix
double-free in netdev_nl_bind_rx_doit()"). I fed the file into a couple
of models to make sure we're not sitting on more bugs. The models haven't
found anything worthy of a Fixes tag but some of the nit picks are
borderline worth addressing.

Jakub Kicinski (4):
  netdev: check for nla_put_u32() failures
  netdev: correct error code in netdev_nl_queue_fill_lease()
  netdev: avoid skipping objects on race with device disappearance
  netdev: don't use dev->flags for IFF_UP

 net/core/netdev-genl.c | 47 +++++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 10 deletions(-)

-- 
2.54.0

[PATCH net-next 1/4] netdev: check for nla_put_u32() failures

[Jakub Kicinski]

Make sure we check if nla_put_u32(id) was successful after creating
objects. This is theoretical today, the skbs are large enough to
always fit the ID.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/core/netdev-genl.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index b4d48f3672a5..a10d353d5eab 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -1091,7 +1091,10 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 			goto err_unbind;
 	}
 
-	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
+	err = nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
+	if (err)
+		goto err_unbind;
+
 	genlmsg_end(rsp, hdr);
 
 	err = genlmsg_reply(rsp, info);
@@ -1227,7 +1230,10 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
 		goto err_unlock_bind_dev;
 	}
 
-	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
+	err = nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
+	if (err)
+		goto err_unbind;
+
 	genlmsg_end(rsp, hdr);
 
 	if (bind_dev != netdev)
@@ -1237,6 +1243,8 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
 
 	return genlmsg_reply(rsp, info);
 
+err_unbind:
+	net_devmem_unbind_dmabuf(binding);
 err_unlock_bind_dev:
 	if (bind_dev != netdev)
 		netdev_unlock(bind_dev);
@@ -1394,7 +1402,12 @@ int netdev_nl_queue_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	netdev_rx_queue_lease(rxq, rxq_lease);
 
-	nla_put_u32(rsp, NETDEV_A_QUEUE_ID, queue_id);
+	err = nla_put_u32(rsp, NETDEV_A_QUEUE_ID, queue_id);
+	if (err) {
+		WARN_ON_ONCE(1); /* we can't delete the queue, ID must fit in */
+		goto err_unlease;
+	}
+
 	genlmsg_end(rsp, hdr);
 
 	netdev_unlock(dev_lease);
@@ -1404,6 +1417,8 @@ int netdev_nl_queue_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	return genlmsg_reply(rsp, info);
 
+err_unlease:
+	netdev_rx_queue_unlease(rxq, rxq_lease);
 err_unlock_dev_lease:
 	netdev_unlock(dev_lease);
 err_put_netns:
-- 
2.54.0

[PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()

[Jakub Kicinski]

netdev_nl_queue_fill_lease() returns ENOMEM on nla_put failures.
This is wrong, the error code should be EMSGSIZE. But it doesn't
matter, caller of netdev_nl_queue_fill_lease() just checks if
the retcode is zero or not, and uses EMSGSIZE.

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

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index a10d353d5eab..d35af460e886 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -432,7 +432,7 @@ netdev_nl_queue_fill_lease(struct sk_buff *rsp, struct net_device *netdev,
 nla_put_failure_unlock:
 	rcu_read_unlock();
 nla_put_failure:
-	return -ENOMEM;
+	return -EMSGSIZE;
 }
 
 static int
-- 
2.54.0

[PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance

[Jakub Kicinski]

If the currently dumped device disappears while we were mid-dump
we will get the next device without resetting the sub-object ID.
This is quite unlikely, it was reported by an AI tool not a real
user. Let's fix it for better dump consistency.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/core/netdev-genl.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index d35af460e886..18046ad0f883 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -310,11 +310,14 @@ int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 			err = -ENODEV;
 		}
 	} else {
+		unsigned long start_ifindex = ctx->ifindex;
+
 		for_each_netdev_lock_scoped(net, netdev, ctx->ifindex) {
+			if (ctx->ifindex != start_ifindex)
+				ctx->napi_id = 0;
 			err = netdev_nl_napi_dump_one(netdev, skb, info, ctx);
 			if (err < 0)
 				break;
-			ctx->napi_id = 0;
 		}
 	}
 
@@ -634,13 +637,17 @@ int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 			err = -ENODEV;
 		}
 	} else {
+		unsigned long start_ifindex = ctx->ifindex;
+
 		for_each_netdev_lock_ops_compat_scoped(net, netdev,
 						       ctx->ifindex) {
+			if (ctx->ifindex != start_ifindex) {
+				ctx->rxq_idx = 0;
+				ctx->txq_idx = 0;
+			}
 			err = netdev_nl_queue_dump_one(netdev, skb, info, ctx);
 			if (err < 0)
 				break;
-			ctx->rxq_idx = 0;
-			ctx->txq_idx = 0;
 		}
 	}
 
@@ -786,8 +793,6 @@ netdev_nl_stats_by_queue(struct net_device *netdev, struct sk_buff *rsp,
 		ctx->txq_idx = ++i;
 	}
 
-	ctx->rxq_idx = 0;
-	ctx->txq_idx = 0;
 	return 0;
 }
 
@@ -902,6 +907,7 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
 	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
 	const struct genl_info *info = genl_info_dump(cb);
 	struct net *net = sock_net(skb->sk);
+	unsigned long start_ifindex;
 	struct net_device *netdev;
 	unsigned int ifindex;
 	unsigned int scope;
@@ -934,7 +940,13 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
 		return err;
 	}
 
+	start_ifindex = ctx->ifindex;
+
 	for_each_netdev_lock_ops_compat_scoped(net, netdev, ctx->ifindex) {
+		if (ctx->ifindex != start_ifindex) {
+			ctx->rxq_idx = 0;
+			ctx->txq_idx = 0;
+		}
 		err = netdev_nl_qstats_get_dump_one(netdev, scope, skb,
 						    info, ctx);
 		if (err < 0)
-- 
2.54.0

[PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP

[Jakub Kicinski]

dev->flags are not technically ops lock protected. The IFF_UP flag
will not change when dev->lock is held, but other flags may change
so KCSAN would probably not be impressed. Because of this we added
a dedicated dev->up which is safe to read under dev->lock.

qstats want to make sure device is up, use dev->up.

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

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 18046ad0f883..745e2eb6c9af 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -773,7 +773,7 @@ netdev_nl_stats_by_queue(struct net_device *netdev, struct sk_buff *rsp,
 	const struct netdev_stat_ops *ops = netdev->stat_ops;
 	int i, err;
 
-	if (!(netdev->flags & IFF_UP))
+	if (!netdev->up)
 		return 0;
 
 	i = ctx->rxq_idx;
-- 
2.54.0

Re: [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()

[Bobby Eshleman]

On Tue, Jun 09, 2026 at 12:08:02PM -0700, Jakub Kicinski wrote:
> netdev_nl_queue_fill_lease() returns ENOMEM on nla_put failures.
> This is wrong, the error code should be EMSGSIZE. But it doesn't
> matter, caller of netdev_nl_queue_fill_lease() just checks if
> the retcode is zero or not, and uses EMSGSIZE.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index a10d353d5eab..d35af460e886 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -432,7 +432,7 @@ netdev_nl_queue_fill_lease(struct sk_buff *rsp, struct net_device *netdev,
>  nla_put_failure_unlock:
>  	rcu_read_unlock();
>  nla_put_failure:
> -	return -ENOMEM;
> +	return -EMSGSIZE;
>  }
>  
>  static int
> -- 
> 2.54.0
> 
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net-next 1/4] netdev: check for nla_put_u32() failures

[Bobby Eshleman]

On Tue, Jun 09, 2026 at 12:08:01PM -0700, Jakub Kicinski wrote:
> Make sure we check if nla_put_u32(id) was successful after creating
> objects. This is theoretical today, the skbs are large enough to
> always fit the ID.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index b4d48f3672a5..a10d353d5eab 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -1091,7 +1091,10 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
>  			goto err_unbind;
>  	}
>  
> -	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	err = nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	if (err)
> +		goto err_unbind;
> +
>  	genlmsg_end(rsp, hdr);
>  
>  	err = genlmsg_reply(rsp, info);
> @@ -1227,7 +1230,10 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
>  		goto err_unlock_bind_dev;
>  	}
>  
> -	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	err = nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	if (err)
> +		goto err_unbind;
> +
>  	genlmsg_end(rsp, hdr);
>  
>  	if (bind_dev != netdev)
> @@ -1237,6 +1243,8 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
>  
>  	return genlmsg_reply(rsp, info);
>  
> +err_unbind:
> +	net_devmem_unbind_dmabuf(binding);
>  err_unlock_bind_dev:
>  	if (bind_dev != netdev)
>  		netdev_unlock(bind_dev);
> @@ -1394,7 +1402,12 @@ int netdev_nl_queue_create_doit(struct sk_buff *skb, struct genl_info *info)
>  
>  	netdev_rx_queue_lease(rxq, rxq_lease);
>  
> -	nla_put_u32(rsp, NETDEV_A_QUEUE_ID, queue_id);
> +	err = nla_put_u32(rsp, NETDEV_A_QUEUE_ID, queue_id);
> +	if (err) {
> +		WARN_ON_ONCE(1); /* we can't delete the queue, ID must fit in */
> +		goto err_unlease;
> +	}
> +
>  	genlmsg_end(rsp, hdr);
>  
>  	netdev_unlock(dev_lease);
> @@ -1404,6 +1417,8 @@ int netdev_nl_queue_create_doit(struct sk_buff *skb, struct genl_info *info)
>  
>  	return genlmsg_reply(rsp, info);
>  
> +err_unlease:
> +	netdev_rx_queue_unlease(rxq, rxq_lease);
>  err_unlock_dev_lease:
>  	netdev_unlock(dev_lease);
>  err_put_netns:
> -- 
> 2.54.0
> 
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()

[Daniel Borkmann]

On 6/9/26 9:08 PM, Jakub Kicinski wrote:
> netdev_nl_queue_fill_lease() returns ENOMEM on nla_put failures.
> This is wrong, the error code should be EMSGSIZE. But it doesn't
> matter, caller of netdev_nl_queue_fill_lease() just checks if
> the retcode is zero or not, and uses EMSGSIZE.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance

[Daniel Borkmann]

On 6/9/26 9:08 PM, Jakub Kicinski wrote:
> If the currently dumped device disappears while we were mid-dump
> we will get the next device without resetting the sub-object ID.
> This is quite unlikely, it was reported by an AI tool not a real
> user. Let's fix it for better dump consistency.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP

[Daniel Borkmann]

On 6/9/26 9:08 PM, Jakub Kicinski wrote:
> dev->flags are not technically ops lock protected. The IFF_UP flag
> will not change when dev->lock is held, but other flags may change
> so KCSAN would probably not be impressed. Because of this we added
> a dedicated dev->up which is safe to read under dev->lock.
> 
> qstats want to make sure device is up, use dev->up.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [PATCH net-next 1/4] netdev: check for nla_put_u32() failures

[Daniel Borkmann]

On 6/9/26 9:08 PM, Jakub Kicinski wrote:
> Make sure we check if nla_put_u32(id) was successful after creating
> objects. This is theoretical today, the skbs are large enough to
> always fit the ID.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>   net/core/netdev-genl.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index b4d48f3672a5..a10d353d5eab 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -1091,7 +1091,10 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
>   			goto err_unbind;
>   	}
>   
> -	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	err = nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	if (err)
> +		goto err_unbind;
> +

Not sure about these three tbh... if the skbs are guaranteed to be
large enough, would a more straight forward ...

   WARN_ON_ONCE(err);

... suffice? Unwinding at this point feels a bit excessive. Is there
any other way we could assert this earlier before we do all the ops
that need to be unwound?

>   	genlmsg_end(rsp, hdr);
>   
>   	err = genlmsg_reply(rsp, info);
> @@ -1227,7 +1230,10 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
>   		goto err_unlock_bind_dev;
>   	}
>   
> -	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	err = nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
> +	if (err)
> +		goto err_unbind;
> +
>   	genlmsg_end(rsp, hdr);
>   
>   	if (bind_dev != netdev)
> @@ -1237,6 +1243,8 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
>   
>   	return genlmsg_reply(rsp, info);
>   
> +err_unbind:
> +	net_devmem_unbind_dmabuf(binding);
>   err_unlock_bind_dev:
>   	if (bind_dev != netdev)
>   		netdev_unlock(bind_dev);
> @@ -1394,7 +1402,12 @@ int netdev_nl_queue_create_doit(struct sk_buff *skb, struct genl_info *info)
>   
>   	netdev_rx_queue_lease(rxq, rxq_lease);
>   
> -	nla_put_u32(rsp, NETDEV_A_QUEUE_ID, queue_id);
> +	err = nla_put_u32(rsp, NETDEV_A_QUEUE_ID, queue_id);
> +	if (err) {
> +		WARN_ON_ONCE(1); /* we can't delete the queue, ID must fit in */
> +		goto err_unlease;
> +	}

.. in particular here :/ would be nicer if we could solve it differently

> +
>   	genlmsg_end(rsp, hdr);
>   
>   	netdev_unlock(dev_lease);
> @@ -1404,6 +1417,8 @@ int netdev_nl_queue_create_doit(struct sk_buff *skb, struct genl_info *info)
>   
>   	return genlmsg_reply(rsp, info);
>   
> +err_unlease:
> +	netdev_rx_queue_unlease(rxq, rxq_lease);
>   err_unlock_dev_lease:
>   	netdev_unlock(dev_lease);
>   err_put_netns:

Re: [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP

[Joe Damato]

On Tue, Jun 09, 2026 at 12:08:04PM -0700, Jakub Kicinski wrote:
> dev->flags are not technically ops lock protected. The IFF_UP flag
> will not change when dev->lock is held, but other flags may change
> so KCSAN would probably not be impressed. Because of this we added
> a dedicated dev->up which is safe to read under dev->lock.
> 
> qstats want to make sure device is up, use dev->up.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net-next 1/4] netdev: check for nla_put_u32() failures

[Joe Damato]

On Tue, Jun 09, 2026 at 12:08:01PM -0700, Jakub Kicinski wrote:
> Make sure we check if nla_put_u32(id) was successful after creating
> objects. This is theoretical today, the skbs are large enough to
> always fit the ID.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()

[Joe Damato]

On Tue, Jun 09, 2026 at 12:08:02PM -0700, Jakub Kicinski wrote:
> netdev_nl_queue_fill_lease() returns ENOMEM on nla_put failures.
> This is wrong, the error code should be EMSGSIZE. But it doesn't
> matter, caller of netdev_nl_queue_fill_lease() just checks if
> the retcode is zero or not, and uses EMSGSIZE.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance

[Bobby Eshleman]

On Tue, Jun 09, 2026 at 12:08:03PM -0700, Jakub Kicinski wrote:
> If the currently dumped device disappears while we were mid-dump
> we will get the next device without resetting the sub-object ID.
> This is quite unlikely, it was reported by an AI tool not a real
> user. Let's fix it for better dump consistency.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index d35af460e886..18046ad0f883 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -310,11 +310,14 @@ int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
>  			err = -ENODEV;
>  		}
>  	} else {
> +		unsigned long start_ifindex = ctx->ifindex;
> +
>  		for_each_netdev_lock_scoped(net, netdev, ctx->ifindex) {
> +			if (ctx->ifindex != start_ifindex)
> +				ctx->napi_id = 0;
>  			err = netdev_nl_napi_dump_one(netdev, skb, info, ctx);
>  			if (err < 0)
>  				break;
> -			ctx->napi_id = 0;
>  		}
>  	}
>  
> @@ -634,13 +637,17 @@ int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
>  			err = -ENODEV;
>  		}
>  	} else {
> +		unsigned long start_ifindex = ctx->ifindex;
> +
>  		for_each_netdev_lock_ops_compat_scoped(net, netdev,
>  						       ctx->ifindex) {
> +			if (ctx->ifindex != start_ifindex) {
> +				ctx->rxq_idx = 0;
> +				ctx->txq_idx = 0;
> +			}
>  			err = netdev_nl_queue_dump_one(netdev, skb, info, ctx);
>  			if (err < 0)
>  				break;
> -			ctx->rxq_idx = 0;
> -			ctx->txq_idx = 0;
>  		}
>  	}
>  
> @@ -786,8 +793,6 @@ netdev_nl_stats_by_queue(struct net_device *netdev, struct sk_buff *rsp,
>  		ctx->txq_idx = ++i;
>  	}
>  
> -	ctx->rxq_idx = 0;
> -	ctx->txq_idx = 0;
>  	return 0;
>  }
>  
> @@ -902,6 +907,7 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
>  	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
>  	const struct genl_info *info = genl_info_dump(cb);
>  	struct net *net = sock_net(skb->sk);
> +	unsigned long start_ifindex;
>  	struct net_device *netdev;
>  	unsigned int ifindex;
>  	unsigned int scope;
> @@ -934,7 +940,13 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
>  		return err;
>  	}
>  
> +	start_ifindex = ctx->ifindex;
> +
>  	for_each_netdev_lock_ops_compat_scoped(net, netdev, ctx->ifindex) {
> +		if (ctx->ifindex != start_ifindex) {
> +			ctx->rxq_idx = 0;
> +			ctx->txq_idx = 0;
> +		}
>  		err = netdev_nl_qstats_get_dump_one(netdev, scope, skb,
>  						    info, ctx);
>  		if (err < 0)
> -- 
> 2.54.0
> 
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP

[Bobby Eshleman]

On Tue, Jun 09, 2026 at 12:08:04PM -0700, Jakub Kicinski wrote:
> dev->flags are not technically ops lock protected. The IFF_UP flag
> will not change when dev->lock is held, but other flags may change
> so KCSAN would probably not be impressed. Because of this we added
> a dedicated dev->up which is safe to read under dev->lock.
> 
> qstats want to make sure device is up, use dev->up.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 18046ad0f883..745e2eb6c9af 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -773,7 +773,7 @@ netdev_nl_stats_by_queue(struct net_device *netdev, struct sk_buff *rsp,
>  	const struct netdev_stat_ops *ops = netdev->stat_ops;
>  	int i, err;
>  
> -	if (!(netdev->flags & IFF_UP))
> +	if (!netdev->up)
>  		return 0;
>  
>  	i = ctx->rxq_idx;
> -- 
> 2.54.0
> 
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance

[Bobby Eshleman]

On Tue, Jun 09, 2026 at 12:59:16PM -0700, Bobby Eshleman wrote:
> On Tue, Jun 09, 2026 at 12:08:03PM -0700, Jakub Kicinski wrote:
> > If the currently dumped device disappears while we were mid-dump
> > we will get the next device without resetting the sub-object ID.
> > This is quite unlikely, it was reported by an AI tool not a real
> > user. Let's fix it for better dump consistency.
> > 
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  net/core/netdev-genl.c | 22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > index d35af460e886..18046ad0f883 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -310,11 +310,14 @@ int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
> >  			err = -ENODEV;
> >  		}
> >  	} else {
> > +		unsigned long start_ifindex = ctx->ifindex;
> > +
> >  		for_each_netdev_lock_scoped(net, netdev, ctx->ifindex) {
> > +			if (ctx->ifindex != start_ifindex)
> > +				ctx->napi_id = 0;
> >  			err = netdev_nl_napi_dump_one(netdev, skb, info, ctx);
> >  			if (err < 0)
> >  				break;
> > -			ctx->napi_id = 0;
> >  		}
> >  	}
> >  
> > @@ -634,13 +637,17 @@ int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
> >  			err = -ENODEV;
> >  		}
> >  	} else {
> > +		unsigned long start_ifindex = ctx->ifindex;
> > +
> >  		for_each_netdev_lock_ops_compat_scoped(net, netdev,
> >  						       ctx->ifindex) {
> > +			if (ctx->ifindex != start_ifindex) {
> > +				ctx->rxq_idx = 0;
> > +				ctx->txq_idx = 0;
> > +			}
> >  			err = netdev_nl_queue_dump_one(netdev, skb, info, ctx);
> >  			if (err < 0)
> >  				break;
> > -			ctx->rxq_idx = 0;
> > -			ctx->txq_idx = 0;
> >  		}
> >  	}
> >  
> > @@ -786,8 +793,6 @@ netdev_nl_stats_by_queue(struct net_device *netdev, struct sk_buff *rsp,
> >  		ctx->txq_idx = ++i;
> >  	}
> >  
> > -	ctx->rxq_idx = 0;
> > -	ctx->txq_idx = 0;
> >  	return 0;
> >  }
> >  
> > @@ -902,6 +907,7 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
> >  	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
> >  	const struct genl_info *info = genl_info_dump(cb);
> >  	struct net *net = sock_net(skb->sk);
> > +	unsigned long start_ifindex;
> >  	struct net_device *netdev;
> >  	unsigned int ifindex;
> >  	unsigned int scope;
> > @@ -934,7 +940,13 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
> >  		return err;
> >  	}
> >  
> > +	start_ifindex = ctx->ifindex;
> > +
> >  	for_each_netdev_lock_ops_compat_scoped(net, netdev, ctx->ifindex) {
> > +		if (ctx->ifindex != start_ifindex) {
> > +			ctx->rxq_idx = 0;
> > +			ctx->txq_idx = 0;
> > +		}
> >  		err = netdev_nl_qstats_get_dump_one(netdev, scope, skb,
> >  						    info, ctx);
> >  		if (err < 0)
> > -- 
> > 2.54.0
> > 
> > 
> 
> Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

I just noticed we handle this differently in rtnetlink with
cb->seq/dev_base_seq/nl_dump_check_consistent(), but it seems like
netdev-genl.c intentionally does not use NLM_F_DUMP_INTR?

Best,
Bobby

Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance

[Jakub Kicinski]

On Tue, 9 Jun 2026 13:15:05 -0700 Bobby Eshleman wrote:
> I just noticed we handle this differently in rtnetlink with
> cb->seq/dev_base_seq/nl_dump_check_consistent(), but it seems like
> netdev-genl.c intentionally does not use NLM_F_DUMP_INTR?

Intentionally is a strong word, but yes, it's "known".

My mental model is that DUMP_INTR is required when we are dumping
unordered lists. In those cases we just count "how many objects
were already dumped", so if we dumped 4 objects then object #1 is
removed the dump will resume from object which was previously #6
thinking that it's #5 and #5 will never be dumped.

In netdev objects have ordered IDs, we don't count.

But in theory we probably maybe should also indicate consistency.
Nobody seems to care in practice.