netdev - RE: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver modules' firmware notifications ability

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:
 <DM6PR12MB45168DC7D9D9D7A5AE3E2B2DD81A2@DM6PR12MB4516.namprd12.prod.outlook.com>
Date: Tue, 30 Apr 2024 18:11:18 +0000
From: Danielle Ratson <danieller@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "corbet@....net" <corbet@....net>,
	"linux@...linux.org.uk" <linux@...linux.org.uk>, "sdf@...gle.com"
	<sdf@...gle.com>, "kory.maincent@...tlin.com" <kory.maincent@...tlin.com>,
	"maxime.chevallier@...tlin.com" <maxime.chevallier@...tlin.com>,
	"vladimir.oltean@....com" <vladimir.oltean@....com>,
	"przemyslaw.kitszel@...el.com" <przemyslaw.kitszel@...el.com>,
	"ahmed.zaki@...el.com" <ahmed.zaki@...el.com>, "richardcochran@...il.com"
	<richardcochran@...il.com>, "shayagr@...zon.com" <shayagr@...zon.com>,
	"paul.greenwalt@...el.com" <paul.greenwalt@...el.com>, "jiri@...nulli.us"
	<jiri@...nulli.us>, "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, mlxsw
	<mlxsw@...dia.com>, Petr Machata <petrm@...dia.com>, Ido Schimmel
	<idosch@...dia.com>
Subject: RE: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver
 modules' firmware notifications ability

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Tuesday, 30 April 2024 6:12
> To: Danielle Ratson <danieller@...dia.com>
> Cc: netdev@...r.kernel.org; davem@...emloft.net; edumazet@...gle.com;
> pabeni@...hat.com; corbet@....net; linux@...linux.org.uk;
> sdf@...gle.com; kory.maincent@...tlin.com;
> maxime.chevallier@...tlin.com; vladimir.oltean@....com;
> przemyslaw.kitszel@...el.com; ahmed.zaki@...el.com;
> richardcochran@...il.com; shayagr@...zon.com;
> paul.greenwalt@...el.com; jiri@...nulli.us; linux-doc@...r.kernel.org; linux-
> kernel@...r.kernel.org; mlxsw <mlxsw@...dia.com>; Petr Machata
> <petrm@...dia.com>; Ido Schimmel <idosch@...dia.com>
> Subject: Re: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver
> modules' firmware notifications ability
> 
> On Wed, 24 Apr 2024 16:30:17 +0300 Danielle Ratson wrote:
> > +	hdr = ethnl_bcastmsg_put(skb,
> ETHTOOL_MSG_MODULE_FW_FLASH_NTF);
> > +	if (!hdr)
> > +		goto err_skb;
> 
> Do we want to blast it to all listeners or treat it as an async reply?
> We can save the seq and portid of the original requester and use reply, I
> think.

I am sorry, I am not sure I understood what you meant here... it should be an async reply, but not sure I understood your suggestion.
Can you explain please?
Thanks!
 
> 
> > +	ret = ethnl_fill_reply_header(skb, dev,
> > +
> ETHTOOL_A_MODULE_FW_FLASH_HEADER);
> > +	if (ret < 0)
> > +		goto err_skb;
> > +
> > +	if (nla_put_u32(skb, ETHTOOL_A_MODULE_FW_FLASH_STATUS,
> status))
> > +		goto err_skb;
> > +
> > +	if (status_msg &&
> > +	    nla_put_string(skb,
> ETHTOOL_A_MODULE_FW_FLASH_STATUS_MSG,
> > +			   status_msg))
> > +		goto err_skb;
> > +
> > +	if (nla_put_u64_64bit(skb, ETHTOOL_A_MODULE_FW_FLASH_DONE,
> done,
> > +			      ETHTOOL_A_MODULE_FW_FLASH_PAD))
> 
> nla_put_uint()
> 
> > +		goto err_skb;
> > +
> > +	if (nla_put_u64_64bit(skb, ETHTOOL_A_MODULE_FW_FLASH_TOTAL,
> total,
> > +			      ETHTOOL_A_MODULE_FW_FLASH_PAD))
> 
> nla_put_uint()
> 
> > +		goto err_skb;
> > +
> > +	genlmsg_end(skb, hdr);
> > +	ethnl_multicast(skb, dev);
> > +	return;
> > +
> > +err_skb:
> > +	nlmsg_free(skb);
> > +}
> > +
> > +void ethnl_module_fw_flash_ntf_err(struct net_device *dev,
> > +				   char *err_msg, char *sub_err_msg) {
> > +	char status_msg[120];
> > +
> > +	if (sub_err_msg)
> > +		sprintf(status_msg, "%s, %s.", err_msg, sub_err_msg);
> > +	else
> > +		sprintf(status_msg, "%s.", err_msg);
> 
> Hm, printing in the dot, and assuming sizeof err_msg + sub_err < 116 is a bit
> surprising. But I guess you have a reason...
> 
> Maybe pass them separately to ethnl_module_fw_flash_ntf() then you can
> nla_reserve() the right amount of space and sprintf() directly into the skb?

I can get rid of the dot actually, would it be ok like that?

> 
> > +	ethnl_module_fw_flash_ntf(dev,
> ETHTOOL_MODULE_FW_FLASH_STATUS_ERROR,
> > +				  status_msg, 0, 0);