* [PATCH net-next 1/4] netdev: check for nla_put_u32() failures
2026-06-09 19:08 [PATCH net-next 0/4] netdev: address a handful of nit picks Jakub Kicinski
@ 2026-06-09 19:08 ` Jakub Kicinski
2026-06-09 19:24 ` Bobby Eshleman
` (2 more replies)
2026-06-09 19:08 ` [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease() Jakub Kicinski
` (2 subsequent siblings)
3 siblings, 3 replies; 19+ messages in thread
From: Jakub Kicinski @ 2026-06-09 19:08 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel, razor,
sdf, dtatulea, bobbyeshleman, 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net-next 1/4] netdev: check for nla_put_u32() failures
2026-06-09 19:08 ` [PATCH net-next 1/4] netdev: check for nla_put_u32() failures Jakub Kicinski
@ 2026-06-09 19:24 ` Bobby Eshleman
2026-06-09 19:52 ` Daniel Borkmann
2026-06-09 19:56 ` Joe Damato
2 siblings, 0 replies; 19+ messages in thread
From: Bobby Eshleman @ 2026-06-09 19:24 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net-next 1/4] netdev: check for nla_put_u32() failures
2026-06-09 19:08 ` [PATCH net-next 1/4] netdev: check for nla_put_u32() failures Jakub Kicinski
2026-06-09 19:24 ` Bobby Eshleman
@ 2026-06-09 19:52 ` Daniel Borkmann
2026-06-09 19:56 ` Joe Damato
2 siblings, 0 replies; 19+ messages in thread
From: Daniel Borkmann @ 2026-06-09 19:52 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, razor, sdf,
dtatulea, bobbyeshleman
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:
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net-next 1/4] netdev: check for nla_put_u32() failures
2026-06-09 19:08 ` [PATCH net-next 1/4] netdev: check for nla_put_u32() failures Jakub Kicinski
2026-06-09 19:24 ` Bobby Eshleman
2026-06-09 19:52 ` Daniel Borkmann
@ 2026-06-09 19:56 ` Joe Damato
2 siblings, 0 replies; 19+ messages in thread
From: Joe Damato @ 2026-06-09 19:56 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()
2026-06-09 19:08 [PATCH net-next 0/4] netdev: address a handful of nit picks Jakub Kicinski
2026-06-09 19:08 ` [PATCH net-next 1/4] netdev: check for nla_put_u32() failures Jakub Kicinski
@ 2026-06-09 19:08 ` Jakub Kicinski
2026-06-09 19:16 ` Bobby Eshleman
` (2 more replies)
2026-06-09 19:08 ` [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance Jakub Kicinski
2026-06-09 19:08 ` [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP Jakub Kicinski
3 siblings, 3 replies; 19+ messages in thread
From: Jakub Kicinski @ 2026-06-09 19:08 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel, razor,
sdf, dtatulea, bobbyeshleman, 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()
2026-06-09 19:08 ` [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease() Jakub Kicinski
@ 2026-06-09 19:16 ` Bobby Eshleman
2026-06-09 19:48 ` Daniel Borkmann
2026-06-09 19:57 ` Joe Damato
2 siblings, 0 replies; 19+ messages in thread
From: Bobby Eshleman @ 2026-06-09 19:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()
2026-06-09 19:08 ` [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease() Jakub Kicinski
2026-06-09 19:16 ` Bobby Eshleman
@ 2026-06-09 19:48 ` Daniel Borkmann
2026-06-09 19:57 ` Joe Damato
2 siblings, 0 replies; 19+ messages in thread
From: Daniel Borkmann @ 2026-06-09 19:48 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, razor, sdf,
dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease()
2026-06-09 19:08 ` [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease() Jakub Kicinski
2026-06-09 19:16 ` Bobby Eshleman
2026-06-09 19:48 ` Daniel Borkmann
@ 2026-06-09 19:57 ` Joe Damato
2 siblings, 0 replies; 19+ messages in thread
From: Joe Damato @ 2026-06-09 19:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance
2026-06-09 19:08 [PATCH net-next 0/4] netdev: address a handful of nit picks Jakub Kicinski
2026-06-09 19:08 ` [PATCH net-next 1/4] netdev: check for nla_put_u32() failures Jakub Kicinski
2026-06-09 19:08 ` [PATCH net-next 2/4] netdev: correct error code in netdev_nl_queue_fill_lease() Jakub Kicinski
@ 2026-06-09 19:08 ` Jakub Kicinski
2026-06-09 19:48 ` Daniel Borkmann
2026-06-09 19:59 ` Bobby Eshleman
2026-06-09 19:08 ` [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP Jakub Kicinski
3 siblings, 2 replies; 19+ messages in thread
From: Jakub Kicinski @ 2026-06-09 19:08 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel, razor,
sdf, dtatulea, bobbyeshleman, 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance
2026-06-09 19:08 ` [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance Jakub Kicinski
@ 2026-06-09 19:48 ` Daniel Borkmann
2026-06-09 19:59 ` Bobby Eshleman
1 sibling, 0 replies; 19+ messages in thread
From: Daniel Borkmann @ 2026-06-09 19:48 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, razor, sdf,
dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance
2026-06-09 19:08 ` [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance Jakub Kicinski
2026-06-09 19:48 ` Daniel Borkmann
@ 2026-06-09 19:59 ` Bobby Eshleman
2026-06-09 20:15 ` Bobby Eshleman
1 sibling, 1 reply; 19+ messages in thread
From: Bobby Eshleman @ 2026-06-09 19:59 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance
2026-06-09 19:59 ` Bobby Eshleman
@ 2026-06-09 20:15 ` Bobby Eshleman
2026-06-09 21:19 ` Jakub Kicinski
0 siblings, 1 reply; 19+ messages in thread
From: Bobby Eshleman @ 2026-06-09 20:15 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance
2026-06-09 20:15 ` Bobby Eshleman
@ 2026-06-09 21:19 ` Jakub Kicinski
2026-06-09 22:04 ` Bobby Eshleman
0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2026-06-09 21:19 UTC (permalink / raw)
To: Bobby Eshleman
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance
2026-06-09 21:19 ` Jakub Kicinski
@ 2026-06-09 22:04 ` Bobby Eshleman
0 siblings, 0 replies; 19+ messages in thread
From: Bobby Eshleman @ 2026-06-09 22:04 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
On Tue, Jun 09, 2026 at 02:19:14PM -0700, Jakub Kicinski wrote:
> 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.
gotcha, thanks for the clarification.
-Bobby
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP
2026-06-09 19:08 [PATCH net-next 0/4] netdev: address a handful of nit picks Jakub Kicinski
` (2 preceding siblings ...)
2026-06-09 19:08 ` [PATCH net-next 3/4] netdev: avoid skipping objects on race with device disappearance Jakub Kicinski
@ 2026-06-09 19:08 ` Jakub Kicinski
2026-06-09 19:49 ` Daniel Borkmann
` (2 more replies)
3 siblings, 3 replies; 19+ messages in thread
From: Jakub Kicinski @ 2026-06-09 19:08 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel, razor,
sdf, dtatulea, bobbyeshleman, 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP
2026-06-09 19:08 ` [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP Jakub Kicinski
@ 2026-06-09 19:49 ` Daniel Borkmann
2026-06-09 19:54 ` Joe Damato
2026-06-09 20:00 ` Bobby Eshleman
2 siblings, 0 replies; 19+ messages in thread
From: Daniel Borkmann @ 2026-06-09 19:49 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, dw, razor, sdf,
dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP
2026-06-09 19:08 ` [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP Jakub Kicinski
2026-06-09 19:49 ` Daniel Borkmann
@ 2026-06-09 19:54 ` Joe Damato
2026-06-09 20:00 ` Bobby Eshleman
2 siblings, 0 replies; 19+ messages in thread
From: Joe Damato @ 2026-06-09 19:54 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP
2026-06-09 19:08 ` [PATCH net-next 4/4] netdev: don't use dev->flags for IFF_UP Jakub Kicinski
2026-06-09 19:49 ` Daniel Borkmann
2026-06-09 19:54 ` Joe Damato
@ 2026-06-09 20:00 ` Bobby Eshleman
2 siblings, 0 replies; 19+ messages in thread
From: Bobby Eshleman @ 2026-06-09 20:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dw, daniel,
razor, sdf, dtatulea, bobbyeshleman
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>
^ permalink raw reply [flat|nested] 19+ messages in thread