Re: [PATCH net-next v13 02/13] net: pse-pd: Add support for reporting events

Le Mon, 16 Jun 2025 13:57:22 +0200,
Kory Maincent [off-list ref] a écrit :

Le Sat, 14 Jun 2025 12:18:43 -0700,
Jakub Kicinski [off-list ref] a écrit :

quoted

On Tue, 10 Jun 2025 10:11:36 +0200 Kory Maincent wrote:  

quoted

+static struct net_device *
+pse_control_find_net_by_id(struct pse_controller_dev *pcdev, int id,
+			   netdevice_tracker *tracker)
+{
+	struct pse_control *psec, *next;
+
+	mutex_lock(&pse_list_mutex);
+	list_for_each_entry_safe(psec, next, &pcdev->pse_control_head,
list) {    

nit: _safe is not necessary here, the body of the if always exits after
dropping the lock  

Indeed, I will drop it.

quoted

Do you plan to add more callers for this function?
Maybe it's better if it returns the psec pointer with the refcount
elevated. Because it would be pretty neat if we could move the 
ethnl_pse_send_ntf(netdev, notifs, &extack); that  pse_isr() does
right after calling this function under the rtnl_lock.
I don't think calling ethnl_pse_send_ntf() may crash the kernel as is,
but it feels like a little bit of a trap to have ethtool code called
outside of any networking lock.  

Ok. My aim was to put the less amount of code inside the rtnl lock but if you
prefer I will call ethnl_pse_send_ntf() with the lock acquired.

psec pointer is private to pse so we will have something like the following.
Is it ok for you ?

psec = pse_control_find_by_id(pcdev, i, &tracker);
rtnl_lock();
if (psec && psec->attached_phydev &&
    psec->attached_phydev->attached_dev)
	ethnl_pse_send_ntf(psec->attached_phydev->attached_dev, notifs,
			   &extack);
rtnl_unlock();
pse_control_put(psec);


-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com