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

From: Danielle Ratson
Date: Wed May 22 2024 - 09:59:41 EST


> > 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.
>
> Makes sense.
>
> > 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.
>
> What's the "relevant event" in this case? Closing of the socket that user had
> issued the command on?

The event should match the below:
event == NETLINK_URELEASE && notify->protocol == NETLINK_GENERIC

Then iterate over the list to look for work that matches the dev and portid.
The socket doesn’t close until the work is done in that case.

>
> Easiest way to "notice" the socket got closed would probably be to add some
> info to genl_sk_priv_*(). ->sock_priv_destroy() will get called. But you can also
> get a close notification in the family
> ->unbind callback.
>
> I'm on the fence whether we should cancel the work. We could just mark the
> command as 'no socket present' and stop sending notifications.
> Not sure which is better..

Is there a scenario that we hit this event and won't intend to cancel the work?

Thanks,
Danielle