Re: [PATCH net-next v2 09/12] net-sysfs: remove the rtnl lock when accessing the xps maps

Quoting Alexander Duyck (2021-02-08 23:20:58)

On Mon, Feb 8, 2021 at 9:19 AM Antoine Tenart [off-list ref] wrote:

quoted

@@ -1328,17 +1328,12 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue,

        index = get_netdev_queue_index(queue);

-       if (!rtnl_trylock())
-               return restart_syscall();
-
        /* If queue belongs to subordinate dev use its map */
        dev = netdev_get_tx_queue(dev, index)->sb_dev ? : dev;

        tc = netdev_txq_to_tc(dev, index);
-       if (tc < 0) {
-               ret = -EINVAL;
-               goto err_rtnl_unlock;
-       }
+       if (tc < 0)
+               return -EINVAL;

        rcu_read_lock();
        dev_maps = rcu_dereference(dev->xps_maps[XPS_CPUS]);

So I think we hit a snag here. The sb_dev pointer is protected by the
rtnl_lock. So I don't think we can release the rtnl_lock until after
we are done with the dev pointer.

Also I am not sure it is safe to use netdev_txq_to_tc without holding
the lock. I don't know if we ever went through and guaranteed that it
will always work if the lock isn't held since in theory the device
could reprogram all the map values out from under us.

Odds are we should probably fix traffic_class_show as I suspect it
probably also needs to be holding the rtnl_lock to prevent any
possible races. I'll submit a patch for that.

Yet another possible race :-) Good catch, I thought about the one we
fixed already but not this one.

As the sb_dev pointer is protected by the rtnl lock, we'll have to keep
the lock. I'll move that patch at the end of the series (it'll be easier
to add the get_device/put_device logic with the xps_queue_show
function). I'll also move netdev_txq_to_tc out of xps_queue_show, to
call it under the rtnl lock taken.

Thanks,
Antoine