mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-08-23 01:10:04 +00:00
![Vladimir Oltean](/assets/img/avatar_default.png)
When using the felix driver (the only one which supports UC filtering and MC filtering) as a DSA master for a random other DSA switch, one can see the following stack trace when the downstream switch ports join a VLAN-aware bridge: ============================= WARNING: suspicious RCU usage ----------------------------- net/8021q/vlan_core.c:238 suspicious rcu_dereference_protected() usage! stack backtrace: Workqueue: dsa_ordered dsa_slave_switchdev_event_work Call trace: lockdep_rcu_suspicious+0x170/0x210 vlan_for_each+0x8c/0x188 dsa_slave_sync_uc+0x128/0x178 __hw_addr_sync_dev+0x138/0x158 dsa_slave_set_rx_mode+0x58/0x70 __dev_set_rx_mode+0x88/0xa8 dev_uc_add+0x74/0xa0 dsa_port_bridge_host_fdb_add+0xec/0x180 dsa_slave_switchdev_event_work+0x7c/0x1c8 process_one_work+0x290/0x568 What it's saying is that vlan_for_each() expects rtnl_lock() context and it's not getting it, when it's called from the DSA master's ndo_set_rx_mode(). The caller of that - dsa_slave_set_rx_mode() - is the slave DSA interface's dsa_port_bridge_host_fdb_add() which comes from the deferred dsa_slave_switchdev_event_work(). We went to great lengths to avoid the rtnl_lock() context in that call path in commit0faf890fc5
("net: dsa: drop rtnl_lock from dsa_slave_switchdev_event_work"), and calling rtnl_lock() is simply not an option due to the possibility of deadlocking when calling dsa_flush_workqueue() from the call paths that do hold rtnl_lock() - basically all of them. So, when the DSA master calls vlan_for_each() from its ndo_set_rx_mode(), the state of the 8021q driver on this device is really not protected from concurrent access by anything. Looking at net/8021q/, I don't think that vlan_info->vid_list was particularly designed with RCU traversal in mind, so introducing an RCU read-side form of vlan_for_each() - vlan_for_each_rcu() - won't be so easy, and it also wouldn't be exactly what we need anyway. In general I believe that the solution isn't in net/8021q/ anyway; vlan_for_each() is not cut out for this task. DSA doesn't need rtnl_lock() to be held per se - since it's not a netdev state change that we're blocking, but rather, just concurrent additions/removals to a VLAN list. We don't even need sleepable context - the callback of vlan_for_each() just schedules deferred work. The proposed escape is to remove the dependency on vlan_for_each() and to open-code a non-sleepable, rtnl-free alternative to that, based on copies of the VLAN list modified from .ndo_vlan_rx_add_vid() and .ndo_vlan_rx_kill_vid(). Fixes:64fdc5f341
("net: dsa: sync unicast and multicast addresses for VLAN filters too") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Link: https://lore.kernel.org/r/20230626154402.3154454-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
123 lines
2.7 KiB
C
123 lines
2.7 KiB
C
/* SPDX-License-Identifier: GPL-2.0-or-later */
|
|
|
|
#ifndef __DSA_SWITCH_H
|
|
#define __DSA_SWITCH_H
|
|
|
|
#include <net/dsa.h>
|
|
|
|
struct netlink_ext_ack;
|
|
|
|
enum {
|
|
DSA_NOTIFIER_AGEING_TIME,
|
|
DSA_NOTIFIER_BRIDGE_JOIN,
|
|
DSA_NOTIFIER_BRIDGE_LEAVE,
|
|
DSA_NOTIFIER_FDB_ADD,
|
|
DSA_NOTIFIER_FDB_DEL,
|
|
DSA_NOTIFIER_HOST_FDB_ADD,
|
|
DSA_NOTIFIER_HOST_FDB_DEL,
|
|
DSA_NOTIFIER_LAG_FDB_ADD,
|
|
DSA_NOTIFIER_LAG_FDB_DEL,
|
|
DSA_NOTIFIER_LAG_CHANGE,
|
|
DSA_NOTIFIER_LAG_JOIN,
|
|
DSA_NOTIFIER_LAG_LEAVE,
|
|
DSA_NOTIFIER_MDB_ADD,
|
|
DSA_NOTIFIER_MDB_DEL,
|
|
DSA_NOTIFIER_HOST_MDB_ADD,
|
|
DSA_NOTIFIER_HOST_MDB_DEL,
|
|
DSA_NOTIFIER_VLAN_ADD,
|
|
DSA_NOTIFIER_VLAN_DEL,
|
|
DSA_NOTIFIER_HOST_VLAN_ADD,
|
|
DSA_NOTIFIER_HOST_VLAN_DEL,
|
|
DSA_NOTIFIER_MTU,
|
|
DSA_NOTIFIER_TAG_PROTO,
|
|
DSA_NOTIFIER_TAG_PROTO_CONNECT,
|
|
DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
|
|
DSA_NOTIFIER_TAG_8021Q_VLAN_ADD,
|
|
DSA_NOTIFIER_TAG_8021Q_VLAN_DEL,
|
|
DSA_NOTIFIER_MASTER_STATE_CHANGE,
|
|
};
|
|
|
|
/* DSA_NOTIFIER_AGEING_TIME */
|
|
struct dsa_notifier_ageing_time_info {
|
|
unsigned int ageing_time;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_BRIDGE_* */
|
|
struct dsa_notifier_bridge_info {
|
|
const struct dsa_port *dp;
|
|
struct dsa_bridge bridge;
|
|
bool tx_fwd_offload;
|
|
struct netlink_ext_ack *extack;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_FDB_* */
|
|
struct dsa_notifier_fdb_info {
|
|
const struct dsa_port *dp;
|
|
const unsigned char *addr;
|
|
u16 vid;
|
|
struct dsa_db db;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_LAG_FDB_* */
|
|
struct dsa_notifier_lag_fdb_info {
|
|
struct dsa_lag *lag;
|
|
const unsigned char *addr;
|
|
u16 vid;
|
|
struct dsa_db db;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_MDB_* */
|
|
struct dsa_notifier_mdb_info {
|
|
const struct dsa_port *dp;
|
|
const struct switchdev_obj_port_mdb *mdb;
|
|
struct dsa_db db;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_LAG_* */
|
|
struct dsa_notifier_lag_info {
|
|
const struct dsa_port *dp;
|
|
struct dsa_lag lag;
|
|
struct netdev_lag_upper_info *info;
|
|
struct netlink_ext_ack *extack;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_VLAN_* */
|
|
struct dsa_notifier_vlan_info {
|
|
const struct dsa_port *dp;
|
|
const struct switchdev_obj_port_vlan *vlan;
|
|
struct netlink_ext_ack *extack;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_MTU */
|
|
struct dsa_notifier_mtu_info {
|
|
const struct dsa_port *dp;
|
|
int mtu;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_TAG_PROTO_* */
|
|
struct dsa_notifier_tag_proto_info {
|
|
const struct dsa_device_ops *tag_ops;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_TAG_8021Q_VLAN_* */
|
|
struct dsa_notifier_tag_8021q_vlan_info {
|
|
const struct dsa_port *dp;
|
|
u16 vid;
|
|
};
|
|
|
|
/* DSA_NOTIFIER_MASTER_STATE_CHANGE */
|
|
struct dsa_notifier_master_state_info {
|
|
const struct net_device *master;
|
|
bool operational;
|
|
};
|
|
|
|
struct dsa_vlan *dsa_vlan_find(struct list_head *vlan_list,
|
|
const struct switchdev_obj_port_vlan *vlan);
|
|
|
|
int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v);
|
|
int dsa_broadcast(unsigned long e, void *v);
|
|
|
|
int dsa_switch_register_notifier(struct dsa_switch *ds);
|
|
void dsa_switch_unregister_notifier(struct dsa_switch *ds);
|
|
|
|
#endif
|