Messages in this thread | | | From | Danielle Ratson <> | Subject | RE: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver modules' firmware notifications ability | Date | Tue, 30 Apr 2024 18:11:18 +0000 |
| |
> -----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);
| |