Skip to content

Commit

Permalink
net: dsa: drop rtnl_lock from dsa_slave_switchdev_event_work
Browse files Browse the repository at this point in the history
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
c9eb3e0 ("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
b117e1e ("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>
  • Loading branch information
vladimiroltean authored and davem330 committed Oct 25, 2021
1 parent 338a3a4 commit 0faf890
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions net/dsa/slave.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 0faf890

Please sign in to comment.