On Tue, 10 Mar 2026 11:47:31 +0100 Björn Töpel wrote:
> The per-PHY specific dump functions share a lot functionality of with
> the default dumpit infrastructure, but does not share the actual code.
> By introducing a new sub-iterator function, the two dumpit variants
> can be folded into one set of functions.
> 
> Add a new dump_one_dev callback in ethnl_request_ops. When
> ops->dump_one_dev is set, ethnl_default_start() saves the target
> device's ifindex for filtered dumps, and ethnl_default_dumpit()
> delegates per-device iteration to the callback instead of calling
> ethnl_default_dump_one() directly. No separate start/dumpit/done
> functions are needed.
> 
> For the existing per-PHY commands (PSE, PLCA, PHY, MSE), the shared
> ethnl_perphy_dump_one_dev helper provides the xa_for_each_start loop
> over the device's PHY topology.
> 
> This prepares the ethtool infrastructure for other commands that need
> similar per-device sub-iteration.

Feels like this could be split into two patches for ease of review.

Warning: net/ethtool/netlink.h:441 struct member 'dump_one_dev' not described 
in 'ethnl_request_ops'
Warning: net/ethtool/netlink.h:441 struct member 'dump_one_dev' not described 
in 'ethnl_request_ops'

> @@ -616,17 +580,41 @@ static int ethnl_default_dumpit(struct sk_buff *skb,
>                               struct netlink_callback *cb)
>  {
>       struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
> +     const struct genl_info *info = genl_info_dump(cb);
>       struct net *net = sock_net(skb->sk);
>       netdevice_tracker dev_tracker;
>       struct net_device *dev;
>       int ret = 0;
>  
> +     if (ctx->ops->dump_one_dev && ctx->ifindex) {
> +             dev = netdev_get_by_index(net, ctx->ifindex, &dev_tracker,
> +                                       GFP_KERNEL);
> +             if (!dev)
> +                     return -ENODEV;
> +
> +             ctx->req_info->dev = dev;
> +             ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub, info);
> +
> +             if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
> +                     ret = skb->len;
> +
> +             netdev_put(dev, &dev_tracker);
> +             return ret;
> +     }
> +
>       rcu_read_lock();
>       for_each_netdev_dump(net, dev, ctx->pos_ifindex) {
>               netdev_hold(dev, &dev_tracker, GFP_ATOMIC);
>               rcu_read_unlock();
>  
> -             ret = ethnl_default_dump_one(skb, dev, ctx, genl_info_dump(cb));
> +             if (ctx->ops->dump_one_dev) {
> +                     ctx->req_info->dev = dev;
> +                     ret = ctx->ops->dump_one_dev(skb, ctx, &ctx->pos_sub,
> +                                                  info);
> +                     ctx->req_info->dev = NULL;
> +             } else {
> +                     ret = ethnl_default_dump_one(skb, dev, ctx, info);
> +             }
>  
>               rcu_read_lock();
>               netdev_put(dev, &dev_tracker);

Not sure if it works here but another way to implementing single dump
and a loop dump concisely is to init both ifindex and pos_ifindex when
request is parsed and then add

        if (ctx->ifindex && ctx->ifindex != ctx->pos_ifindex)
                break;

at the start of the loop. That way the body of the loop doesn't have to
be repeated in a separate if 

Reply via email to