lkml.org 
[lkml]   [2024]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver modules' firmware notifications ability
Date
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, 30 April 2024 6:12
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; edumazet@google.com;
> pabeni@redhat.com; corbet@lwn.net; linux@armlinux.org.uk;
> sdf@google.com; kory.maincent@bootlin.com;
> maxime.chevallier@bootlin.com; vladimir.oltean@nxp.com;
> przemyslaw.kitszel@intel.com; ahmed.zaki@intel.com;
> richardcochran@gmail.com; shayagr@amazon.com;
> paul.greenwalt@intel.com; jiri@resnulli.us; linux-doc@vger.kernel.org; linux-
> kernel@vger.kernel.org; mlxsw <mlxsw@nvidia.com>; Petr Machata
> <petrm@nvidia.com>; Ido Schimmel <idosch@nvidia.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);
\
 
 \ /
  Last update: 2024-05-27 18:09    [W:0.266 / U:0.720 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site