diff options
| author | David S. Miller <davem@davemloft.net> | 2021-10-24 13:47:45 +0100 |
|---|---|---|
| committer | David S. Miller <davem@davemloft.net> | 2021-10-24 13:47:45 +0100 |
| commit | 965e6b262f48257dbdb51b565ecfd84877a0ab5f (patch) | |
| tree | 06380d14ee187dd4154682bc8950ae211c162418 /net/dsa/switch.c | |
| parent | 4d98bb0d7ec2d0b417df6207b0bafe1868bad9f8 (diff) | |
| parent | edc90d15850c4812a6c72b004c9b81c37755b997 (diff) | |
Merge branch 'dsa-rtnl'
Vladimir Oltean says:
====================
Drop rtnl_lock from DSA .port_fdb_{add,del}
As mentioned in the RFC posted 2 months ago:
https://patchwork.kernel.org/project/netdevbpf/cover/20210824114049.3814660-1-vladimir.oltean@nxp.com/
DSA is transitioning to a driver API where the rtnl_lock is not held
when calling ds->ops->port_fdb_add() and ds->ops->port_fdb_del().
Drivers cannot take that lock privately from those callbacks either.
This change is required so that DSA can wait for switchdev FDB work
items to finish before leaving the bridge. That change will be made in a
future patch series.
A small selftest is provided with the patch set in the hope that
concurrency issues uncovered by this series, but not spotted by me by
code inspection, will be caught.
A status of the existing drivers:
- mv88e6xxx_port_fdb_add() and mv88e6xxx_port_fdb_del() take
mv88e6xxx_reg_lock() so they should be safe.
- qca8k_fdb_add() and qca8k_fdb_del() take mutex_lock(&priv->reg_mutex)
so they should be safe.
- hellcreek_fdb_add() and hellcreek_fdb_add() take mutex_lock(&hellcreek->reg_lock)
so they should be safe.
- ksz9477_port_fdb_add() and ksz9477_port_fdb_del() take mutex_lock(&dev->alu_mutex)
so they should be safe.
- b53_fdb_add() and b53_fdb_del() did not have locking, so I've added a
scheme based on my own judgement there (not tested).
- felix_fdb_add() and felix_fdb_del() did not have locking, I've added
and tested a locking scheme there.
- mt7530_port_fdb_add() and mt7530_port_fdb_del() take
mutex_lock(&priv->reg_mutex), so they should be safe.
- gswip_port_fdb() did not have locking, so I've added a non-expert
locking scheme based on my own judgement (not tested).
- lan9303_alr_add_port() and lan9303_alr_del_port() take
mutex_lock(&chip->alr_mutex) so they should be safe.
- sja1105_fdb_add() and sja1105_fdb_del() did not have locking, I've
added and tested a locking scheme.
====================
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/dsa/switch.c')
| -rw-r--r-- | net/dsa/switch.c | 76 |
1 files changed, 52 insertions, 24 deletions
diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 2b1b21bde830..6871e5f9b597 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -215,26 +215,30 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp, struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; int port = dp->index; - int err; + int err = 0; /* No need to bother with refcounting for user ports */ if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp))) return ds->ops->port_mdb_add(ds, port, mdb); + mutex_lock(&dp->addr_lists_lock); + a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid); if (a) { refcount_inc(&a->refcount); - return 0; + goto out; } a = kzalloc(sizeof(*a), GFP_KERNEL); - if (!a) - return -ENOMEM; + if (!a) { + err = -ENOMEM; + goto out; + } err = ds->ops->port_mdb_add(ds, port, mdb); if (err) { kfree(a); - return err; + goto out; } ether_addr_copy(a->addr, mdb->addr); @@ -242,7 +246,10 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp, refcount_set(&a->refcount, 1); list_add_tail(&a->list, &dp->mdbs); - return 0; +out: + mutex_unlock(&dp->addr_lists_lock); + + return err; } static int dsa_port_do_mdb_del(struct dsa_port *dp, @@ -251,29 +258,36 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp, struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; int port = dp->index; - int err; + int err = 0; /* No need to bother with refcounting for user ports */ if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp))) return ds->ops->port_mdb_del(ds, port, mdb); + mutex_lock(&dp->addr_lists_lock); + a = dsa_mac_addr_find(&dp->mdbs, mdb->addr, mdb->vid); - if (!a) - return -ENOENT; + if (!a) { + err = -ENOENT; + goto out; + } if (!refcount_dec_and_test(&a->refcount)) - return 0; + goto out; err = ds->ops->port_mdb_del(ds, port, mdb); if (err) { refcount_inc(&a->refcount); - return err; + goto out; } list_del(&a->list); kfree(a); - return 0; +out: + mutex_unlock(&dp->addr_lists_lock); + + return err; } static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr, @@ -282,26 +296,30 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr, struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; int port = dp->index; - int err; + int err = 0; /* No need to bother with refcounting for user ports */ if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp))) return ds->ops->port_fdb_add(ds, port, addr, vid); + mutex_lock(&dp->addr_lists_lock); + a = dsa_mac_addr_find(&dp->fdbs, addr, vid); if (a) { refcount_inc(&a->refcount); - return 0; + goto out; } a = kzalloc(sizeof(*a), GFP_KERNEL); - if (!a) - return -ENOMEM; + if (!a) { + err = -ENOMEM; + goto out; + } err = ds->ops->port_fdb_add(ds, port, addr, vid); if (err) { kfree(a); - return err; + goto out; } ether_addr_copy(a->addr, addr); @@ -309,7 +327,10 @@ static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr, refcount_set(&a->refcount, 1); list_add_tail(&a->list, &dp->fdbs); - return 0; +out: + mutex_unlock(&dp->addr_lists_lock); + + return err; } static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr, @@ -318,29 +339,36 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr, struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; int port = dp->index; - int err; + int err = 0; /* No need to bother with refcounting for user ports */ if (!(dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp))) return ds->ops->port_fdb_del(ds, port, addr, vid); + mutex_lock(&dp->addr_lists_lock); + a = dsa_mac_addr_find(&dp->fdbs, addr, vid); - if (!a) - return -ENOENT; + if (!a) { + err = -ENOENT; + goto out; + } if (!refcount_dec_and_test(&a->refcount)) - return 0; + goto out; err = ds->ops->port_fdb_del(ds, port, addr, vid); if (err) { refcount_inc(&a->refcount); - return err; + goto out; } list_del(&a->list); kfree(a); - return 0; +out: + mutex_unlock(&dp->addr_lists_lock); + + return err; } static int dsa_switch_host_fdb_add(struct dsa_switch *ds, |
