netdev - Re: [PATCH net-next 5/9] net: ethtool: copy req

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250622140020.3dcc6814@fedora.home>
Date: Sun, 22 Jun 2025 14:00:20 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
 donald.hunter@...il.com, sdf@...ichev.me, jdamato@...tly.com,
 ecree.xilinx@...il.com
Subject: Re: [PATCH net-next 5/9] net: ethtool: copy req_info from SET to
 NTF

Hi Jakub,

On Sat, 21 Jun 2025 10:19:40 -0700
Jakub Kicinski <kuba@...nel.org> wrote:

> Copy information parsed for SET with .req_parse to NTF handling
> and therefore the GET-equivalent that it ends up executing.
> This way if the SET was on a sub-object (like RSS context)
> the notification will also be appropriately scoped.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>

Thanks for that !

> ---
>  net/ethtool/netlink.h |  5 ++++-
>  net/ethtool/netlink.c | 14 +++++++++-----
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
> index 4a061944a3aa..373a8d5e86ae 100644
> --- a/net/ethtool/netlink.h
> +++ b/net/ethtool/netlink.h
> @@ -23,7 +23,8 @@ void *ethnl_dump_put(struct sk_buff *skb, struct netlink_callback *cb, u8 cmd);
>  void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd);
>  void *ethnl_unicast_put(struct sk_buff *skb, u32 portid, u32 seq, u8 cmd);
>  int ethnl_multicast(struct sk_buff *skb, struct net_device *dev);
> -void ethnl_notify(struct net_device *dev, unsigned int cmd, const void *data);
> +void ethnl_notify(struct net_device *dev, unsigned int cmd,
> +		  const struct ethnl_req_info *req_info);
>  
>  /**
>   * ethnl_strz_size() - calculate attribute length for fixed size string
> @@ -338,6 +339,8 @@ int ethnl_sock_priv_set(struct sk_buff *skb, struct net_device *dev, u32 portid,
>   *	header is already filled on entry, the rest up to @repdata_offset
>   *	is zero initialized. This callback should only modify type specific
>   *	request info by parsed attributes from request message.
> + *	Called for both GET and SET. Information parsed for SET will
> + *	be conveyed to the req_info used during NTF generation.
>   * @prepare_data:
>   *	Retrieve and prepare data needed to compose a reply message. Calls to
>   *	ethtool_ops handlers are limited to this callback. Common reply data
> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
> index 129f9d56ac65..f4a61016b364 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -911,7 +911,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
>  	swap(dev->cfg, dev->cfg_pending);
>  	if (!ret)
>  		goto out_ops;
> -	ethtool_notify(dev, ops->set_ntf_cmd);
> +	ethnl_notify(dev, ops->set_ntf_cmd, req_info);
>  
>  	ret = 0;
>  out_ops:
> @@ -950,7 +950,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
>  
>  /* default notification handler */
>  static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
> -				 const void *data)
> +				 const struct ethnl_req_info *orig_req_info)
>  {
>  	struct ethnl_reply_data *reply_data;
>  	const struct ethnl_request_ops *ops;
> @@ -979,6 +979,9 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
>  
>  	req_info->dev = dev;
>  	req_info->flags |= ETHTOOL_FLAG_COMPACT_BITSETS;
> +	if (orig_req_info)
> +		memcpy(&req_info[1], &orig_req_info[1],
> +		       ops->req_info_size - sizeof(*req_info));

Is there any chance we can also carry orig_req_info->phy_index into
req_info ? That's a bit of sub-command context that is also useful for
notifications, especially PLCA. As of today, the PLCA notif after a SET
isn't generated at all as the phy_index isn't passed to the ethnl
notification code.

Maxime