net: dsa: sja1105: serialize sja1105_port_mcast_flood() with other FDB accesses
sja1105_fdb_add() runs from the dsa_owq, and sja1105_port_mcast_flood() runs from switchdev_deferred_process_work(). Prior to the blamed commit, they used to be indirectly serialized through the rtnl_lock(), which no longer holds true because dsa_owq dropped that. So, it is now possible that we traverse the static config BLK_IDX_L2_LOOKUP elements concurrently compared to when we change them, in sja1105_static_fdb_change(). That is not ideal, since it might result in data corruption. Introduce a mutex which serializes accesses to the hardware FDB and to the static config elements for the L2 Address Lookup table. I can't find a good reason to add locking around sja1105_fdb_dump(). I'll add it later if needed. Fixes: 0faf890fc519 ("net: dsa: drop rtnl_lock from dsa_slave_switchdev_event_work") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
7cef293b9a
commit
ea32690daf
@ -266,6 +266,8 @@ struct sja1105_private {
|
||||
* the switch doesn't confuse them with one another.
|
||||
*/
|
||||
struct mutex mgmt_lock;
|
||||
/* Serializes accesses to the FDB */
|
||||
struct mutex fdb_lock;
|
||||
/* PTP two-step TX timestamp ID, and its serialization lock */
|
||||
spinlock_t ts_id_lock;
|
||||
u8 ts_id;
|
||||
|
@ -1798,6 +1798,7 @@ static int sja1105_fdb_add(struct dsa_switch *ds, int port,
|
||||
struct dsa_db db)
|
||||
{
|
||||
struct sja1105_private *priv = ds->priv;
|
||||
int rc;
|
||||
|
||||
if (!vid) {
|
||||
switch (db.type) {
|
||||
@ -1812,12 +1813,16 @@ static int sja1105_fdb_add(struct dsa_switch *ds, int port,
|
||||
}
|
||||
}
|
||||
|
||||
return priv->info->fdb_add_cmd(ds, port, addr, vid);
|
||||
mutex_lock(&priv->fdb_lock);
|
||||
rc = priv->info->fdb_add_cmd(ds, port, addr, vid);
|
||||
mutex_unlock(&priv->fdb_lock);
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
static int sja1105_fdb_del(struct dsa_switch *ds, int port,
|
||||
const unsigned char *addr, u16 vid,
|
||||
struct dsa_db db)
|
||||
static int __sja1105_fdb_del(struct dsa_switch *ds, int port,
|
||||
const unsigned char *addr, u16 vid,
|
||||
struct dsa_db db)
|
||||
{
|
||||
struct sja1105_private *priv = ds->priv;
|
||||
|
||||
@ -1837,6 +1842,20 @@ static int sja1105_fdb_del(struct dsa_switch *ds, int port,
|
||||
return priv->info->fdb_del_cmd(ds, port, addr, vid);
|
||||
}
|
||||
|
||||
static int sja1105_fdb_del(struct dsa_switch *ds, int port,
|
||||
const unsigned char *addr, u16 vid,
|
||||
struct dsa_db db)
|
||||
{
|
||||
struct sja1105_private *priv = ds->priv;
|
||||
int rc;
|
||||
|
||||
mutex_lock(&priv->fdb_lock);
|
||||
rc = __sja1105_fdb_del(ds, port, addr, vid, db);
|
||||
mutex_unlock(&priv->fdb_lock);
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
|
||||
dsa_fdb_dump_cb_t *cb, void *data)
|
||||
{
|
||||
@ -1899,6 +1918,8 @@ static void sja1105_fast_age(struct dsa_switch *ds, int port)
|
||||
};
|
||||
int i;
|
||||
|
||||
mutex_lock(&priv->fdb_lock);
|
||||
|
||||
for (i = 0; i < SJA1105_MAX_L2_LOOKUP_COUNT; i++) {
|
||||
struct sja1105_l2_lookup_entry l2_lookup = {0};
|
||||
u8 macaddr[ETH_ALEN];
|
||||
@ -1912,7 +1933,7 @@ static void sja1105_fast_age(struct dsa_switch *ds, int port)
|
||||
if (rc) {
|
||||
dev_err(ds->dev, "Failed to read FDB: %pe\n",
|
||||
ERR_PTR(rc));
|
||||
return;
|
||||
break;
|
||||
}
|
||||
|
||||
if (!(l2_lookup.destports & BIT(port)))
|
||||
@ -1924,14 +1945,16 @@ static void sja1105_fast_age(struct dsa_switch *ds, int port)
|
||||
|
||||
u64_to_ether_addr(l2_lookup.macaddr, macaddr);
|
||||
|
||||
rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
|
||||
rc = __sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
|
||||
if (rc) {
|
||||
dev_err(ds->dev,
|
||||
"Failed to delete FDB entry %pM vid %lld: %pe\n",
|
||||
macaddr, l2_lookup.vlanid, ERR_PTR(rc));
|
||||
return;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
mutex_unlock(&priv->fdb_lock);
|
||||
}
|
||||
|
||||
static int sja1105_mdb_add(struct dsa_switch *ds, int port,
|
||||
@ -2955,7 +2978,9 @@ static int sja1105_port_mcast_flood(struct sja1105_private *priv, int to,
|
||||
{
|
||||
struct sja1105_l2_lookup_entry *l2_lookup;
|
||||
struct sja1105_table *table;
|
||||
int match;
|
||||
int match, rc;
|
||||
|
||||
mutex_lock(&priv->fdb_lock);
|
||||
|
||||
table = &priv->static_config.tables[BLK_IDX_L2_LOOKUP];
|
||||
l2_lookup = table->entries;
|
||||
@ -2968,7 +2993,8 @@ static int sja1105_port_mcast_flood(struct sja1105_private *priv, int to,
|
||||
if (match == table->entry_count) {
|
||||
NL_SET_ERR_MSG_MOD(extack,
|
||||
"Could not find FDB entry for unknown multicast");
|
||||
return -ENOSPC;
|
||||
rc = -ENOSPC;
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (flags.val & BR_MCAST_FLOOD)
|
||||
@ -2976,10 +3002,13 @@ static int sja1105_port_mcast_flood(struct sja1105_private *priv, int to,
|
||||
else
|
||||
l2_lookup[match].destports &= ~BIT(to);
|
||||
|
||||
return sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
|
||||
l2_lookup[match].index,
|
||||
&l2_lookup[match],
|
||||
true);
|
||||
rc = sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
|
||||
l2_lookup[match].index,
|
||||
&l2_lookup[match], true);
|
||||
out:
|
||||
mutex_unlock(&priv->fdb_lock);
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
static int sja1105_port_pre_bridge_flags(struct dsa_switch *ds, int port,
|
||||
@ -3349,6 +3378,7 @@ static int sja1105_probe(struct spi_device *spi)
|
||||
mutex_init(&priv->ptp_data.lock);
|
||||
mutex_init(&priv->dynamic_config_lock);
|
||||
mutex_init(&priv->mgmt_lock);
|
||||
mutex_init(&priv->fdb_lock);
|
||||
spin_lock_init(&priv->ts_id_lock);
|
||||
|
||||
rc = sja1105_parse_dt(priv);
|
||||
|
Loading…
x
Reference in New Issue
Block a user