> -----Original Message----- > From: Jakub Kicinski <kuba@xxxxxxxxxx> > Sent: Wednesday, 1 May 2024 17:38 > To: Ido Schimmel <idosch@xxxxxxxxxx> > Cc: Danielle Ratson <danieller@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; > davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; pabeni@xxxxxxxxxx; > corbet@xxxxxxx; linux@xxxxxxxxxxxxxxx; sdf@xxxxxxxxxx; > kory.maincent@xxxxxxxxxxx; maxime.chevallier@xxxxxxxxxxx; > vladimir.oltean@xxxxxxx; przemyslaw.kitszel@xxxxxxxxx; > ahmed.zaki@xxxxxxxxx; richardcochran@xxxxxxxxx; shayagr@xxxxxxxxxx; > paul.greenwalt@xxxxxxxxx; jiri@xxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; mlxsw <mlxsw@xxxxxxxxxx>; Petr Machata > <petrm@xxxxxxxxxx> > Subject: Re: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver > modules' firmware notifications ability > > On Wed, 1 May 2024 10:53:48 +0300 Ido Schimmel wrote: > > We can try to use unicast, but the current design is influenced by > > devlink firmware flash (see __devlink_flash_update_notify()) and > > ethtool cable testing (see ethnl_cable_test_started() and > > ethnl_cable_test_finished()), both of which use multicast > > notifications although the latter does not update about progress. > > > > Do you want us to try the unicast approach or be consistent with the > > above examples? > > We are charting a bit of a new territory here, you're right that the precedents > point in the direction of multicast. > The unicast is harder to get done on the kernel side (we should probably also > check that the socket pid didn't get reused, stop sending the notifications > when original socket gets closed?) It will require using pretty much all the > pieces of advanced netlink infra we have, I'm happy to explain more, but I'll > also understand if you prefer to stick to multicast. Hi Jakub, Following our discussion, I wanted to see if you are ok with the idea below: 1. Add a new unicast function to netlink.c: void *ethnl_unicast_put(struct sk_buff *skb, u32 portid, u32 seq, u8 cmd) 2. Use it in the notification function instead of the multicast previously used along with genlmsg_unicast(). 'portid' and 'seq' taken from genl_info(), are added to the struct ethtool_module_fw_flash, which is accessible from the work item. 3. Create a global list that holds nodes from type struct ethtool_module_fw_flash() and add it as a field in the struct ethtool_module_fw_flash. Before scheduling a work, a new node is added to the list. 4. Add a new netlink notifier that when the relevant event takes place, deletes the node from the list, wait until the end of the work item, with cancel_work_sync() and free allocations. Thanks, Danielle
