Re: [PATCH 6/11] ethtool: Add MAC loopback support via ethtool_ops

[Naveen Mamindlapalli <naveenm@marvell.com>]

On 2026-03-10 at 16:17:36, Björn Töpel (bjorn@kernel.org) wrote:
> Extend struct ethtool_ops with three loopback callbacks that drivers
> can implement for MAC-level loopback control:
> 
>  - get_loopback(dev, name, id, entry): exact lookup by name and
>    instance id, used by doit (single-entry GET) requests.
> 
>  - get_loopback_by_index(dev, index, entry): flat enumeration by
>    index, used by dumpit (multi-entry GET) requests to iterate all
>    loopback points on a device.
> 
>  - set_loopback(dev, entry, extack): apply a loopback configuration
>    change. Returns 1 if hardware state changed, 0 if no-op.
> 
> Wire the MAC component into loopback.c's dispatch functions. For dump
> enumeration, MAC entries are tried first via the driver's
> get_loopback_by_index() op, then MODULE/CMIS entries follow at the
> next index offset.
> 
> Signed-off-by: Björn Töpel <bjorn@kernel.org>
> ---
>  include/linux/ethtool.h |  7 ++++++
>  net/ethtool/loopback.c  | 56 ++++++++++++++++++++++++++++++++---------
>  2 files changed, 51 insertions(+), 12 deletions(-)
>

The MAC loopback callbacks look good.
Reviewed-by: Naveen Mamindlapalli <naveenm@marvell.com>
 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index c9beca11fc40..8893e732f930 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -1321,6 +1321,13 @@ struct ethtool_ops {
>  	int	(*set_mm)(struct net_device *dev, struct ethtool_mm_cfg *cfg,
>  			  struct netlink_ext_ack *extack);
>  	void	(*get_mm_stats)(struct net_device *dev, struct ethtool_mm_stats *stats);
> +	int	(*get_loopback)(struct net_device *dev, const char *name,
> +				u32 id, struct ethtool_loopback_entry *entry);
> +	int	(*get_loopback_by_index)(struct net_device *dev, u32 index,
> +					 struct ethtool_loopback_entry *entry);
> +	int	(*set_loopback)(struct net_device *dev,
> +				const struct ethtool_loopback_entry *entry,
> +				struct netlink_ext_ack *extack);
>  };
>  
>  int ethtool_check_ops(const struct ethtool_ops *ops);
> diff --git a/net/ethtool/loopback.c b/net/ethtool/loopback.c
> index ed184f45322e..9c5834be743f 100644
> --- a/net/ethtool/loopback.c
> +++ b/net/ethtool/loopback.c
> @@ -86,6 +86,10 @@ static int loopback_get(struct net_device *dev,
>  			struct ethtool_loopback_entry *entry)
>  {
>  	switch (component) {
> +	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
> +		if (!dev->ethtool_ops->get_loopback)
> +			return -EOPNOTSUPP;
> +		return dev->ethtool_ops->get_loopback(dev, name, id, entry);
>  	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
>  		return ethtool_cmis_get_loopback(dev, name, entry);
>  	default:
> @@ -93,10 +97,22 @@ static int loopback_get(struct net_device *dev,
>  	}
>  }
>  
> -static int loopback_get_by_index(struct net_device *dev, u32 index,
> +static int loopback_get_by_index(struct net_device *dev,
> +				 enum ethtool_loopback_component component,
> +				 u32 index,
>  				 struct ethtool_loopback_entry *entry)
>  {
> -	return ethtool_cmis_get_loopback_by_index(dev, index, entry);
> +	switch (component) {
> +	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
> +		if (!dev->ethtool_ops->get_loopback_by_index)
> +			return -EOPNOTSUPP;
> +		return dev->ethtool_ops->get_loopback_by_index(dev, index,
> +							       entry);
> +	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
> +		return ethtool_cmis_get_loopback_by_index(dev, index, entry);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
>  }
>  
>  static int loopback_prepare_data(const struct ethnl_req_info *req_base,
> @@ -116,7 +132,8 @@ static int loopback_prepare_data(const struct ethnl_req_info *req_base,
>  		ret = loopback_get(dev, req_info->component, req_info->id,
>  				   req_info->name, &data->entry);
>  	else
> -		ret = loopback_get_by_index(dev, req_info->index, &data->entry);
> +		ret = loopback_get_by_index(dev, req_info->component,
> +					    req_info->index, &data->entry);
>  
>  	ethnl_ops_complete(dev);
>  
> @@ -225,6 +242,10 @@ static int __loopback_set(struct net_device *dev,
>  			  struct netlink_ext_ack *extack)
>  {
>  	switch (entry->component) {
> +	case ETHTOOL_LOOPBACK_COMPONENT_MAC:
> +		if (!dev->ethtool_ops->set_loopback)
> +			return -EOPNOTSUPP;
> +		return dev->ethtool_ops->set_loopback(dev, entry, extack);
>  	case ETHTOOL_LOOPBACK_COMPONENT_MODULE:
>  		return ethtool_cmis_set_loopback(dev, entry, extack);
>  	default:
> @@ -274,20 +295,31 @@ static int loopback_dump_one_dev(struct sk_buff *skb,
>  {
>  	struct loopback_req_info *req_info =
>  		container_of(ctx->req_info, struct loopback_req_info, base);
> +	/* pos_sub encodes: upper 16 bits = component phase, lower 16 = index
> +	 * within that component. dump_one_dev is called repeatedly with
> +	 * increasing pos_sub until all components are exhausted.
> +	 */
> +	enum ethtool_loopback_component phase = *pos_sub >> 16;
> +	u32 idx = *pos_sub & 0xffff;
>  	int ret;
>  
> -	for (;; (*pos_sub)++) {
> -		req_info->index = *pos_sub;
> -		ret = ethnl_default_dump_one(skb, ctx->req_info->dev, ctx,
> -					     info);
> -		if (ret == -EOPNOTSUPP)
> -			break;
> -		if (ret)
> -			return ret;
> +	for (; phase <= ETHTOOL_LOOPBACK_COMPONENT_MODULE; phase++) {
> +		for (;; idx++) {
> +			req_info->component = phase;
> +			req_info->index = idx;
> +			ret = ethnl_default_dump_one(skb, ctx->req_info->dev,
> +						     ctx, info);
> +			if (ret == -EOPNOTSUPP)
> +				break;
> +			if (ret) {
> +				*pos_sub = ((unsigned long)phase << 16) | idx;
> +				return ret;
> +			}
> +		}
> +		idx = 0;
>  	}
>  
>  	*pos_sub = 0;
> -
>  	return 0;
>  }
>  
> 
> -- 
> 2.53.0
>