On Tue, Apr 30, 2024 at 01:03:02PM -0700, Jakub Kicinski wrote: > On Tue, 30 Apr 2024 18:11:18 +0000 Danielle Ratson wrote: > > > 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? > > Make sure you have read the netlink intro, it should help fill in some > gaps I won't explicitly cover: > https://docs.kernel.org/next/userspace-api/netlink/intro.html > > "True" notifications will have pid = 0 and seq = 0, while replies to > commands have those fields populated based on the request. > > pid identifies the socket where the message should be delivered. > ethnl_multicast() assumes that it's zero (since it's designed to work > for notifications) and sends the message to all sockets subscribed to > a multicast / notification group (ETHNL_MCGRP_MONITOR). > > So that's the background. What you're doing isn't incorrect but I think > it'd be better if we didn't use the multicast group here, and sent the > messages as a reply - just to the socket which requested the flashing. > Still asynchronously, we just need to save the right pid and seq to use. > > Two reasons for this: > 1) convenience, the user space socket won't have to subscribe to > the multicast group > 2) the multicast group is really intended for notifying about > _configuration changes_ done to the system. If there is a daemon > listening on that group, there's a very high chance it won't care > about progress of the flashing. Maybe we can send a single > notification that flashing has been completed but not "progress > updates" > > I think it should work. 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?
