OSDN Git Service

net: dsa: drop rtnl_lock from dsa_slave_switchdev_event_work
authorVladimir Oltean <vladimir.oltean@nxp.com>
Sun, 24 Oct 2021 17:17:55 +0000 (20:17 +0300)
committerDavid S. Miller <davem@davemloft.net>
Mon, 25 Oct 2021 11:59:42 +0000 (12:59 +0100)
After talking with Ido Schimmel, it became clear that rtnl_lock is not
actually required for anything that is done inside the
SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE deferred work handlers.

The reason why it was probably added by Arkadi Sharshevsky in commit
c9eb3e0f8701 ("net: dsa: Add support for learning FDB through
notification") was to offer the same locking/serialization guarantees as
.ndo_fdb_{add,del} and avoid reworking any drivers.

DSA has implemented .ndo_fdb_add and .ndo_fdb_del until commit
b117e1e8a86d ("net: dsa: delete dsa_legacy_fdb_add and
dsa_legacy_fdb_del") - that is to say, until fairly recently.

But those methods have been deleted, so now we are free to drop the
rtnl_lock as well.

Note that exposing DSA switch drivers to an unlocked method which was
previously serialized by the rtnl_mutex is a potentially dangerous
affair. Driver writers couldn't ensure that their internal locking
scheme does the right thing even if they wanted.

We could err on the side of paranoia and introduce a switch-wide lock
inside the DSA framework, but that seems way overreaching. Instead, we
could check as many drivers for regressions as we can, fix those first,
then let this change go in once it is assumed to be fairly safe.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/dsa/slave.c

index 9d9fef6..adcfb2c 100644 (file)
@@ -2413,7 +2413,6 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
 
        dp = dsa_to_port(ds, switchdev_work->port);
 
-       rtnl_lock();
        switch (switchdev_work->event) {
        case SWITCHDEV_FDB_ADD_TO_DEVICE:
                if (switchdev_work->host_addr)
@@ -2448,7 +2447,6 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
 
                break;
        }
-       rtnl_unlock();
 
        dev_put(switchdev_work->dev);
        kfree(switchdev_work);