From d0004a020bb50263de0e3e775c7b7c7a003e0e0c Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 20 Oct 2021 20:49:50 +0300 Subject: net: dsa: remove the "dsa_to_port in a loop" antipattern from the core Ever since Vivien's conversion of the ds->ports array into a dst->ports list, and the introduction of dsa_to_port, iterations through the ports of a switch became quadratic whenever dsa_to_port was needed. dsa_to_port can either be called directly, or indirectly through the dsa_is_{user,cpu,dsa,unused}_port helpers. Use the newly introduced dsa_switch_for_each_port() iteration macro that works with the iterator variable being a struct dsa_port *dp directly, and not an int i. It is an expensive variable to go from i to dp, but cheap to go from dp to i. This macro iterates through the entire ds->dst->ports list and filters by the ports belonging just to the switch provided as argument. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- net/dsa/switch.c | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) (limited to 'net/dsa/switch.c') diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 6466d0539af9..19651674c8c7 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -17,14 +17,11 @@ static unsigned int dsa_switch_fastest_ageing_time(struct dsa_switch *ds, unsigned int ageing_time) { - int i; - - for (i = 0; i < ds->num_ports; ++i) { - struct dsa_port *dp = dsa_to_port(ds, i); + struct dsa_port *dp; + dsa_switch_for_each_port(dp, ds) if (dp->ageing_time && dp->ageing_time < ageing_time) ageing_time = dp->ageing_time; - } return ageing_time; } @@ -120,7 +117,8 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds, struct netlink_ext_ack extack = {0}; bool change_vlan_filtering = false; bool vlan_filtering; - int err, port; + struct dsa_port *dp; + int err; if (dst->index == info->tree_index && ds->index == info->sw_index && ds->ops->port_bridge_leave) @@ -150,10 +148,10 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds, * VLAN-aware bridge. */ if (change_vlan_filtering && ds->vlan_filtering_is_global) { - for (port = 0; port < ds->num_ports; port++) { + dsa_switch_for_each_port(dp, ds) { struct net_device *bridge_dev; - bridge_dev = dsa_to_port(ds, port)->bridge_dev; + bridge_dev = dp->bridge_dev; if (bridge_dev && br_vlan_enabled(bridge_dev)) { change_vlan_filtering = false; @@ -579,38 +577,34 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds, struct dsa_notifier_tag_proto_info *info) { const struct dsa_device_ops *tag_ops = info->tag_ops; - int port, err; + struct dsa_port *dp, *cpu_dp; + int err; if (!ds->ops->change_tag_protocol) return -EOPNOTSUPP; ASSERT_RTNL(); - for (port = 0; port < ds->num_ports; port++) { - if (!dsa_is_cpu_port(ds, port)) - continue; - - err = ds->ops->change_tag_protocol(ds, port, tag_ops->proto); + dsa_switch_for_each_cpu_port(cpu_dp, ds) { + err = ds->ops->change_tag_protocol(ds, cpu_dp->index, + tag_ops->proto); if (err) return err; - dsa_port_set_tag_protocol(dsa_to_port(ds, port), tag_ops); + dsa_port_set_tag_protocol(cpu_dp, tag_ops); } /* Now that changing the tag protocol can no longer fail, let's update * the remaining bits which are "duplicated for faster access", and the * bits that depend on the tagger, such as the MTU. */ - for (port = 0; port < ds->num_ports; port++) { - if (dsa_is_user_port(ds, port)) { - struct net_device *slave; + dsa_switch_for_each_user_port(dp, ds) { + struct net_device *slave = dp->slave; - slave = dsa_to_port(ds, port)->slave; - dsa_slave_setup_tagger(slave); + dsa_slave_setup_tagger(slave); - /* rtnl_mutex is held in dsa_tree_change_tag_proto */ - dsa_slave_change_mtu(slave, slave->mtu); - } + /* rtnl_mutex is held in dsa_tree_change_tag_proto */ + dsa_slave_change_mtu(slave, slave->mtu); } return 0; -- cgit v1.2.3-58-ga151 From fac6abd5f13297ddeae7ff6eb903e933cffc071b Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 20 Oct 2021 20:49:53 +0300 Subject: net: dsa: convert cross-chip notifiers to iterate using dp The majority of cross-chip switch notifiers need to filter in some way over the type of ports: some install VLANs etc on all cascade ports. The difference is that the matching function, which filters by port type, is separate from the function where the iteration happens. So this patch needs to refactor the matching functions' prototypes as well, to take the dp as argument. In a future patch/series, I might convert dsa_towards_port to return a struct dsa_port *dp too, but at the moment it is a bit entangled with dsa_routing_port which is also used by mv88e6xxx and they both return an int port. So keep dsa_towards_port the way it is and convert it into a dp using dsa_to_port. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- net/dsa/switch.c | 129 ++++++++++++++++++++++++++++------------------------ net/dsa/tag_8021q.c | 85 +++++++++++++++++----------------- 2 files changed, 112 insertions(+), 102 deletions(-) (limited to 'net/dsa/switch.c') diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 19651674c8c7..2b1b21bde830 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -46,10 +46,10 @@ static int dsa_switch_ageing_time(struct dsa_switch *ds, return 0; } -static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port, - struct dsa_notifier_mtu_info *info) +static bool dsa_port_mtu_match(struct dsa_port *dp, + struct dsa_notifier_mtu_info *info) { - if (ds->index == info->sw_index && port == info->port) + if (dp->ds->index == info->sw_index && dp->index == info->port) return true; /* Do not propagate to other switches in the tree if the notifier was @@ -58,7 +58,7 @@ static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port, if (info->targeted_match) return false; - if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port)) + if (dsa_port_is_dsa(dp) || dsa_port_is_cpu(dp)) return true; return false; @@ -67,14 +67,16 @@ static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port, static int dsa_switch_mtu(struct dsa_switch *ds, struct dsa_notifier_mtu_info *info) { - int port, ret; + struct dsa_port *dp; + int ret; if (!ds->ops->port_change_mtu) return -EOPNOTSUPP; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_mtu_match(ds, port, info)) { - ret = ds->ops->port_change_mtu(ds, port, info->mtu); + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_mtu_match(dp, info)) { + ret = ds->ops->port_change_mtu(ds, dp->index, + info->mtu); if (ret) return ret; } @@ -177,19 +179,19 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds, * DSA links) that sit between the targeted port on which the notifier was * emitted and its dedicated CPU port. */ -static bool dsa_switch_host_address_match(struct dsa_switch *ds, int port, - int info_sw_index, int info_port) +static bool dsa_port_host_address_match(struct dsa_port *dp, + int info_sw_index, int info_port) { struct dsa_port *targeted_dp, *cpu_dp; struct dsa_switch *targeted_ds; - targeted_ds = dsa_switch_find(ds->dst->index, info_sw_index); + targeted_ds = dsa_switch_find(dp->ds->dst->index, info_sw_index); targeted_dp = dsa_to_port(targeted_ds, info_port); cpu_dp = targeted_dp->cpu_dp; - if (dsa_switch_is_upstream_of(ds, targeted_ds)) - return port == dsa_towards_port(ds, cpu_dp->ds->index, - cpu_dp->index); + if (dsa_switch_is_upstream_of(dp->ds, targeted_ds)) + return dp->index == dsa_towards_port(dp->ds, cpu_dp->ds->index, + cpu_dp->index); return false; } @@ -207,11 +209,12 @@ static struct dsa_mac_addr *dsa_mac_addr_find(struct list_head *addr_list, return NULL; } -static int dsa_switch_do_mdb_add(struct dsa_switch *ds, int port, - const struct switchdev_obj_port_mdb *mdb) +static int dsa_port_do_mdb_add(struct dsa_port *dp, + const struct switchdev_obj_port_mdb *mdb) { - struct dsa_port *dp = dsa_to_port(ds, port); + struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; + int port = dp->index; int err; /* No need to bother with refcounting for user ports */ @@ -242,11 +245,12 @@ static int dsa_switch_do_mdb_add(struct dsa_switch *ds, int port, return 0; } -static int dsa_switch_do_mdb_del(struct dsa_switch *ds, int port, - const struct switchdev_obj_port_mdb *mdb) +static int dsa_port_do_mdb_del(struct dsa_port *dp, + const struct switchdev_obj_port_mdb *mdb) { - struct dsa_port *dp = dsa_to_port(ds, port); + struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; + int port = dp->index; int err; /* No need to bother with refcounting for user ports */ @@ -272,11 +276,12 @@ static int dsa_switch_do_mdb_del(struct dsa_switch *ds, int port, return 0; } -static int dsa_switch_do_fdb_add(struct dsa_switch *ds, int port, - const unsigned char *addr, u16 vid) +static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr, + u16 vid) { - struct dsa_port *dp = dsa_to_port(ds, port); + struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; + int port = dp->index; int err; /* No need to bother with refcounting for user ports */ @@ -307,11 +312,12 @@ static int dsa_switch_do_fdb_add(struct dsa_switch *ds, int port, return 0; } -static int dsa_switch_do_fdb_del(struct dsa_switch *ds, int port, - const unsigned char *addr, u16 vid) +static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr, + u16 vid) { - struct dsa_port *dp = dsa_to_port(ds, port); + struct dsa_switch *ds = dp->ds; struct dsa_mac_addr *a; + int port = dp->index; int err; /* No need to bother with refcounting for user ports */ @@ -340,17 +346,16 @@ static int dsa_switch_do_fdb_del(struct dsa_switch *ds, int port, static int dsa_switch_host_fdb_add(struct dsa_switch *ds, struct dsa_notifier_fdb_info *info) { + struct dsa_port *dp; int err = 0; - int port; if (!ds->ops->port_fdb_add) return -EOPNOTSUPP; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_host_address_match(ds, port, info->sw_index, - info->port)) { - err = dsa_switch_do_fdb_add(ds, port, info->addr, - info->vid); + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_host_address_match(dp, info->sw_index, + info->port)) { + err = dsa_port_do_fdb_add(dp, info->addr, info->vid); if (err) break; } @@ -362,17 +367,16 @@ static int dsa_switch_host_fdb_add(struct dsa_switch *ds, static int dsa_switch_host_fdb_del(struct dsa_switch *ds, struct dsa_notifier_fdb_info *info) { + struct dsa_port *dp; int err = 0; - int port; if (!ds->ops->port_fdb_del) return -EOPNOTSUPP; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_host_address_match(ds, port, info->sw_index, - info->port)) { - err = dsa_switch_do_fdb_del(ds, port, info->addr, - info->vid); + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_host_address_match(dp, info->sw_index, + info->port)) { + err = dsa_port_do_fdb_del(dp, info->addr, info->vid); if (err) break; } @@ -385,22 +389,24 @@ static int dsa_switch_fdb_add(struct dsa_switch *ds, struct dsa_notifier_fdb_info *info) { int port = dsa_towards_port(ds, info->sw_index, info->port); + struct dsa_port *dp = dsa_to_port(ds, port); if (!ds->ops->port_fdb_add) return -EOPNOTSUPP; - return dsa_switch_do_fdb_add(ds, port, info->addr, info->vid); + return dsa_port_do_fdb_add(dp, info->addr, info->vid); } static int dsa_switch_fdb_del(struct dsa_switch *ds, struct dsa_notifier_fdb_info *info) { int port = dsa_towards_port(ds, info->sw_index, info->port); + struct dsa_port *dp = dsa_to_port(ds, port); if (!ds->ops->port_fdb_del) return -EOPNOTSUPP; - return dsa_switch_do_fdb_del(ds, port, info->addr, info->vid); + return dsa_port_do_fdb_del(dp, info->addr, info->vid); } static int dsa_switch_hsr_join(struct dsa_switch *ds, @@ -466,37 +472,39 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds, struct dsa_notifier_mdb_info *info) { int port = dsa_towards_port(ds, info->sw_index, info->port); + struct dsa_port *dp = dsa_to_port(ds, port); if (!ds->ops->port_mdb_add) return -EOPNOTSUPP; - return dsa_switch_do_mdb_add(ds, port, info->mdb); + return dsa_port_do_mdb_add(dp, info->mdb); } static int dsa_switch_mdb_del(struct dsa_switch *ds, struct dsa_notifier_mdb_info *info) { int port = dsa_towards_port(ds, info->sw_index, info->port); + struct dsa_port *dp = dsa_to_port(ds, port); if (!ds->ops->port_mdb_del) return -EOPNOTSUPP; - return dsa_switch_do_mdb_del(ds, port, info->mdb); + return dsa_port_do_mdb_del(dp, info->mdb); } static int dsa_switch_host_mdb_add(struct dsa_switch *ds, struct dsa_notifier_mdb_info *info) { + struct dsa_port *dp; int err = 0; - int port; if (!ds->ops->port_mdb_add) return -EOPNOTSUPP; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_host_address_match(ds, port, info->sw_index, - info->port)) { - err = dsa_switch_do_mdb_add(ds, port, info->mdb); + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_host_address_match(dp, info->sw_index, + info->port)) { + err = dsa_port_do_mdb_add(dp, info->mdb); if (err) break; } @@ -508,16 +516,16 @@ static int dsa_switch_host_mdb_add(struct dsa_switch *ds, static int dsa_switch_host_mdb_del(struct dsa_switch *ds, struct dsa_notifier_mdb_info *info) { + struct dsa_port *dp; int err = 0; - int port; if (!ds->ops->port_mdb_del) return -EOPNOTSUPP; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_host_address_match(ds, port, info->sw_index, - info->port)) { - err = dsa_switch_do_mdb_del(ds, port, info->mdb); + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_host_address_match(dp, info->sw_index, + info->port)) { + err = dsa_port_do_mdb_del(dp, info->mdb); if (err) break; } @@ -526,13 +534,13 @@ static int dsa_switch_host_mdb_del(struct dsa_switch *ds, return err; } -static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port, - struct dsa_notifier_vlan_info *info) +static bool dsa_port_vlan_match(struct dsa_port *dp, + struct dsa_notifier_vlan_info *info) { - if (ds->index == info->sw_index && port == info->port) + if (dp->ds->index == info->sw_index && dp->index == info->port) return true; - if (dsa_is_dsa_port(ds, port)) + if (dsa_port_is_dsa(dp)) return true; return false; @@ -541,14 +549,15 @@ static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port, static int dsa_switch_vlan_add(struct dsa_switch *ds, struct dsa_notifier_vlan_info *info) { - int port, err; + struct dsa_port *dp; + int err; if (!ds->ops->port_vlan_add) return -EOPNOTSUPP; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_vlan_match(ds, port, info)) { - err = ds->ops->port_vlan_add(ds, port, info->vlan, + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_vlan_match(dp, info)) { + err = ds->ops->port_vlan_add(ds, dp->index, info->vlan, info->extack); if (err) return err; diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c index 935d0264ebd8..8f4e0af2f74f 100644 --- a/net/dsa/tag_8021q.c +++ b/net/dsa/tag_8021q.c @@ -138,12 +138,13 @@ dsa_tag_8021q_vlan_find(struct dsa_8021q_context *ctx, int port, u16 vid) return NULL; } -static int dsa_switch_do_tag_8021q_vlan_add(struct dsa_switch *ds, int port, - u16 vid, u16 flags) +static int dsa_port_do_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, + u16 flags) { - struct dsa_8021q_context *ctx = ds->tag_8021q_ctx; - struct dsa_port *dp = dsa_to_port(ds, port); + struct dsa_8021q_context *ctx = dp->ds->tag_8021q_ctx; + struct dsa_switch *ds = dp->ds; struct dsa_tag_8021q_vlan *v; + int port = dp->index; int err; /* No need to bother with refcounting for user ports */ @@ -174,12 +175,12 @@ static int dsa_switch_do_tag_8021q_vlan_add(struct dsa_switch *ds, int port, return 0; } -static int dsa_switch_do_tag_8021q_vlan_del(struct dsa_switch *ds, int port, - u16 vid) +static int dsa_port_do_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid) { - struct dsa_8021q_context *ctx = ds->tag_8021q_ctx; - struct dsa_port *dp = dsa_to_port(ds, port); + struct dsa_8021q_context *ctx = dp->ds->tag_8021q_ctx; + struct dsa_switch *ds = dp->ds; struct dsa_tag_8021q_vlan *v; + int port = dp->index; int err; /* No need to bother with refcounting for user ports */ @@ -206,14 +207,16 @@ static int dsa_switch_do_tag_8021q_vlan_del(struct dsa_switch *ds, int port, } static bool -dsa_switch_tag_8021q_vlan_match(struct dsa_switch *ds, int port, - struct dsa_notifier_tag_8021q_vlan_info *info) +dsa_port_tag_8021q_vlan_match(struct dsa_port *dp, + struct dsa_notifier_tag_8021q_vlan_info *info) { - if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port)) + struct dsa_switch *ds = dp->ds; + + if (dsa_port_is_dsa(dp) || dsa_port_is_cpu(dp)) return true; if (ds->dst->index == info->tree_index && ds->index == info->sw_index) - return port == info->port; + return dp->index == info->port; return false; } @@ -221,7 +224,8 @@ dsa_switch_tag_8021q_vlan_match(struct dsa_switch *ds, int port, int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds, struct dsa_notifier_tag_8021q_vlan_info *info) { - int port, err; + struct dsa_port *dp; + int err; /* Since we use dsa_broadcast(), there might be other switches in other * trees which don't support tag_8021q, so don't return an error. @@ -231,21 +235,20 @@ int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds, if (!ds->ops->tag_8021q_vlan_add || !ds->tag_8021q_ctx) return 0; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_tag_8021q_vlan_match(ds, port, info)) { + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_tag_8021q_vlan_match(dp, info)) { u16 flags = 0; - if (dsa_is_user_port(ds, port)) + if (dsa_port_is_user(dp)) flags |= BRIDGE_VLAN_INFO_UNTAGGED; if (vid_is_dsa_8021q_rxvlan(info->vid) && dsa_8021q_rx_switch_id(info->vid) == ds->index && - dsa_8021q_rx_source_port(info->vid) == port) + dsa_8021q_rx_source_port(info->vid) == dp->index) flags |= BRIDGE_VLAN_INFO_PVID; - err = dsa_switch_do_tag_8021q_vlan_add(ds, port, - info->vid, - flags); + err = dsa_port_do_tag_8021q_vlan_add(dp, info->vid, + flags); if (err) return err; } @@ -257,15 +260,15 @@ int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds, int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds, struct dsa_notifier_tag_8021q_vlan_info *info) { - int port, err; + struct dsa_port *dp; + int err; if (!ds->ops->tag_8021q_vlan_del || !ds->tag_8021q_ctx) return 0; - for (port = 0; port < ds->num_ports; port++) { - if (dsa_switch_tag_8021q_vlan_match(ds, port, info)) { - err = dsa_switch_do_tag_8021q_vlan_del(ds, port, - info->vid); + dsa_switch_for_each_port(dp, ds) { + if (dsa_port_tag_8021q_vlan_match(dp, info)) { + err = dsa_port_do_tag_8021q_vlan_del(dp, info->vid); if (err) return err; } @@ -321,15 +324,14 @@ int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds, * +-+-----+-+-----+-+-----+-+-----+-+ +-+-----+-+-----+-+-----+-+-----+-+ * swp0 swp1 swp2 swp3 swp0 swp1 swp2 swp3 */ -static bool dsa_tag_8021q_bridge_match(struct dsa_switch *ds, int port, - struct dsa_notifier_bridge_info *info) +static bool +dsa_port_tag_8021q_bridge_match(struct dsa_port *dp, + struct dsa_notifier_bridge_info *info) { - struct dsa_port *dp = dsa_to_port(ds, port); - /* Don't match on self */ - if (ds->dst->index == info->tree_index && - ds->index == info->sw_index && - port == info->port) + if (dp->ds->dst->index == info->tree_index && + dp->ds->index == info->sw_index && + dp->index == info->port) return false; if (dsa_port_is_user(dp)) @@ -343,8 +345,9 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, { struct dsa_switch *targeted_ds; struct dsa_port *targeted_dp; + struct dsa_port *dp; u16 targeted_rx_vid; - int err, port; + int err; if (!ds->tag_8021q_ctx) return 0; @@ -353,11 +356,10 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, targeted_dp = dsa_to_port(targeted_ds, info->port); targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port); - for (port = 0; port < ds->num_ports; port++) { - struct dsa_port *dp = dsa_to_port(ds, port); - u16 rx_vid = dsa_8021q_rx_vid(ds, port); + dsa_switch_for_each_port(dp, ds) { + u16 rx_vid = dsa_8021q_rx_vid(ds, dp->index); - if (!dsa_tag_8021q_bridge_match(ds, port, info)) + if (!dsa_port_tag_8021q_bridge_match(dp, info)) continue; /* Install the RX VID of the targeted port in our VLAN table */ @@ -379,8 +381,8 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, { struct dsa_switch *targeted_ds; struct dsa_port *targeted_dp; + struct dsa_port *dp; u16 targeted_rx_vid; - int port; if (!ds->tag_8021q_ctx) return 0; @@ -389,11 +391,10 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, targeted_dp = dsa_to_port(targeted_ds, info->port); targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port); - for (port = 0; port < ds->num_ports; port++) { - struct dsa_port *dp = dsa_to_port(ds, port); - u16 rx_vid = dsa_8021q_rx_vid(ds, port); + dsa_switch_for_each_port(dp, ds) { + u16 rx_vid = dsa_8021q_rx_vid(ds, dp->index); - if (!dsa_tag_8021q_bridge_match(ds, port, info)) + if (!dsa_port_tag_8021q_bridge_match(dp, info)) continue; /* Remove the RX VID of the targeted port from our VLAN table */ -- cgit v1.2.3-58-ga151 From d3bd89243768615ac4a5da5ea08b6f471c5b806b Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 22 Oct 2021 21:43:09 +0300 Subject: net: dsa: introduce locking for the address lists on CPU and DSA ports Now that the rtnl_mutex is going away for dsa_port_{host_,}fdb_{add,del}, no one is serializing access to the address lists that DSA keeps for the purpose of reference counting on shared ports (CPU and cascade ports). It can happen for one dsa_switch_do_fdb_del to do list_del on a dp->fdbs element while another dsa_switch_do_fdb_{add,del} is traversing dp->fdbs. We need to avoid that. Currently dp->mdbs is not at risk, because dsa_switch_do_mdb_{add,del} still runs under the rtnl_mutex. But it would be nice if it would not depend on that being the case. So let's introduce a mutex per port (the address lists are per port too) and share it between dp->mdbs and dp->fdbs. The place where we put the locking is interesting. It could be tempting to put a DSA-level lock which still serializes calls to .port_fdb_{add,del}, but it would still not avoid concurrency with other driver code paths that are currently under rtnl_mutex (.port_fdb_dump, .port_fast_age). So it would add a very false sense of security (and adding a global switch-wide lock in DSA to resynchronize with the rtnl_lock is also counterproductive and hard). So the locking is intentionally done only where the dp->fdbs and dp->mdbs lists are traversed. That means, from a driver perspective, that .port_fdb_add will be called with the dp->addr_lists_lock mutex held on the CPU port, but not held on user ports. This is done so that driver writers are not encouraged to rely on any guarantee offered by dp->addr_lists_lock. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- include/net/dsa.h | 1 + net/dsa/dsa2.c | 1 + net/dsa/switch.c | 76 +++++++++++++++++++++++++++++++++++++------------------ 3 files changed, 54 insertions(+), 24 deletions(-) (limited to 'net/dsa/switch.c') diff --git a/include/net/dsa.h b/include/net/dsa.h index 1cd9c2461f0d..badd214f7470 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -287,6 +287,7 @@ struct dsa_port { /* List of MAC addresses that must be forwarded on this port. * These are only valid on CPU ports and DSA links. */ + struct mutex addr_lists_lock; struct list_head fdbs; struct list_head mdbs; diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index f5270114dcb8..826957b6442b 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -433,6 +433,7 @@ static int dsa_port_setup(struct dsa_port *dp) if (dp->setup) return 0; + mutex_init(&dp->addr_lists_lock); INIT_LIST_HEAD(&dp->fdbs); INIT_LIST_HEAD(&dp->mdbs); 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, -- cgit v1.2.3-58-ga151 From 2d7e73f09fc2f5d968ca375f047718cf25ae2b92 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Mon, 25 Oct 2021 12:59:25 +0100 Subject: Revert "Merge branch 'dsa-rtnl'" This reverts commit 965e6b262f48257dbdb51b565ecfd84877a0ab5f, reversing changes made to 4d98bb0d7ec2d0b417df6207b0bafe1868bad9f8. --- MAINTAINERS | 1 - drivers/net/dsa/b53/b53_common.c | 40 ++-------- drivers/net/dsa/b53/b53_priv.h | 1 - drivers/net/dsa/lantiq_gswip.c | 28 ++----- drivers/net/dsa/sja1105/sja1105.h | 2 - drivers/net/dsa/sja1105/sja1105_dynamic_config.c | 91 ++++++---------------- drivers/net/dsa/sja1105/sja1105_main.c | 1 - drivers/net/ethernet/mscc/ocelot.c | 53 +++---------- include/net/dsa.h | 1 - include/soc/mscc/ocelot.h | 3 - net/dsa/dsa2.c | 1 - net/dsa/slave.c | 2 + net/dsa/switch.c | 76 ++++++------------ .../drivers/net/dsa/test_bridge_fdb_stress.sh | 47 ----------- tools/testing/selftests/net/forwarding/lib.sh | 10 +-- 15 files changed, 74 insertions(+), 283 deletions(-) delete mode 100755 tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh (limited to 'net/dsa/switch.c') diff --git a/MAINTAINERS b/MAINTAINERS index 975086c5345d..c5aa142d4b3a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13056,7 +13056,6 @@ F: include/linux/dsa/ F: include/linux/platform_data/dsa.h F: include/net/dsa.h F: net/dsa/ -F: tools/testing/selftests/drivers/net/dsa/ NETWORKING [GENERAL] M: "David S. Miller" diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index b0262e69a170..51dfaf817a40 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -1544,7 +1544,6 @@ EXPORT_SYMBOL(b53_vlan_del); /* Address Resolution Logic routines */ static int b53_arl_op_wait(struct b53_device *dev) - __must_hold(&dev->arl_mutex) { unsigned int timeout = 10; u8 reg; @@ -1563,7 +1562,6 @@ static int b53_arl_op_wait(struct b53_device *dev) } static int b53_arl_rw_op(struct b53_device *dev, unsigned int op) - __must_hold(&dev->arl_mutex) { u8 reg; @@ -1587,7 +1585,6 @@ static int b53_arl_rw_op(struct b53_device *dev, unsigned int op) static int b53_arl_read(struct b53_device *dev, u64 mac, u16 vid, struct b53_arl_entry *ent, u8 *idx) - __must_hold(&dev->arl_mutex) { DECLARE_BITMAP(free_bins, B53_ARLTBL_MAX_BIN_ENTRIES); unsigned int i; @@ -1633,7 +1630,6 @@ static int b53_arl_read(struct b53_device *dev, u64 mac, static int b53_arl_op(struct b53_device *dev, int op, int port, const unsigned char *addr, u16 vid, bool is_valid) - __must_hold(&dev->arl_mutex) { struct b53_arl_entry ent; u32 fwd_entry; @@ -1711,7 +1707,6 @@ int b53_fdb_add(struct dsa_switch *ds, int port, const unsigned char *addr, u16 vid) { struct b53_device *priv = ds->priv; - int ret; /* 5325 and 5365 require some more massaging, but could * be supported eventually @@ -1719,11 +1714,7 @@ int b53_fdb_add(struct dsa_switch *ds, int port, if (is5325(priv) || is5365(priv)) return -EOPNOTSUPP; - mutex_lock(&priv->arl_mutex); - ret = b53_arl_op(priv, 0, port, addr, vid, true); - mutex_unlock(&priv->arl_mutex); - - return ret; + return b53_arl_op(priv, 0, port, addr, vid, true); } EXPORT_SYMBOL(b53_fdb_add); @@ -1731,18 +1722,12 @@ int b53_fdb_del(struct dsa_switch *ds, int port, const unsigned char *addr, u16 vid) { struct b53_device *priv = ds->priv; - int ret; - mutex_lock(&priv->arl_mutex); - ret = b53_arl_op(priv, 0, port, addr, vid, false); - mutex_unlock(&priv->arl_mutex); - - return ret; + return b53_arl_op(priv, 0, port, addr, vid, false); } EXPORT_SYMBOL(b53_fdb_del); static int b53_arl_search_wait(struct b53_device *dev) - __must_hold(&dev->arl_mutex) { unsigned int timeout = 1000; u8 reg; @@ -1763,7 +1748,6 @@ static int b53_arl_search_wait(struct b53_device *dev) static void b53_arl_search_rd(struct b53_device *dev, u8 idx, struct b53_arl_entry *ent) - __must_hold(&dev->arl_mutex) { u64 mac_vid; u32 fwd_entry; @@ -1796,8 +1780,6 @@ int b53_fdb_dump(struct dsa_switch *ds, int port, int ret; u8 reg; - mutex_lock(&priv->arl_mutex); - /* Start search operation */ reg = ARL_SRCH_STDN; b53_write8(priv, B53_ARLIO_PAGE, B53_ARL_SRCH_CTL, reg); @@ -1805,18 +1787,18 @@ int b53_fdb_dump(struct dsa_switch *ds, int port, do { ret = b53_arl_search_wait(priv); if (ret) - break; + return ret; b53_arl_search_rd(priv, 0, &results[0]); ret = b53_fdb_copy(port, &results[0], cb, data); if (ret) - break; + return ret; if (priv->num_arl_bins > 2) { b53_arl_search_rd(priv, 1, &results[1]); ret = b53_fdb_copy(port, &results[1], cb, data); if (ret) - break; + return ret; if (!results[0].is_valid && !results[1].is_valid) break; @@ -1824,8 +1806,6 @@ int b53_fdb_dump(struct dsa_switch *ds, int port, } while (count++ < b53_max_arl_entries(priv) / 2); - mutex_unlock(&priv->arl_mutex); - return 0; } EXPORT_SYMBOL(b53_fdb_dump); @@ -1834,7 +1814,6 @@ int b53_mdb_add(struct dsa_switch *ds, int port, const struct switchdev_obj_port_mdb *mdb) { struct b53_device *priv = ds->priv; - int ret; /* 5325 and 5365 require some more massaging, but could * be supported eventually @@ -1842,11 +1821,7 @@ int b53_mdb_add(struct dsa_switch *ds, int port, if (is5325(priv) || is5365(priv)) return -EOPNOTSUPP; - mutex_lock(&priv->arl_mutex); - ret = b53_arl_op(priv, 0, port, mdb->addr, mdb->vid, true); - mutex_unlock(&priv->arl_mutex); - - return ret; + return b53_arl_op(priv, 0, port, mdb->addr, mdb->vid, true); } EXPORT_SYMBOL(b53_mdb_add); @@ -1856,9 +1831,7 @@ int b53_mdb_del(struct dsa_switch *ds, int port, struct b53_device *priv = ds->priv; int ret; - mutex_lock(&priv->arl_mutex); ret = b53_arl_op(priv, 0, port, mdb->addr, mdb->vid, false); - mutex_unlock(&priv->arl_mutex); if (ret) dev_err(ds->dev, "failed to delete MDB entry\n"); @@ -2695,7 +2668,6 @@ struct b53_device *b53_switch_alloc(struct device *base, mutex_init(&dev->reg_mutex); mutex_init(&dev->stats_mutex); - mutex_init(&dev->arl_mutex); return dev; } diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h index 579da74ada64..544101e74bca 100644 --- a/drivers/net/dsa/b53/b53_priv.h +++ b/drivers/net/dsa/b53/b53_priv.h @@ -107,7 +107,6 @@ struct b53_device { struct mutex reg_mutex; struct mutex stats_mutex; - struct mutex arl_mutex; const struct b53_io_ops *ops; /* chip specific data */ diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c index 7056d98d8177..a3e937a371ef 100644 --- a/drivers/net/dsa/lantiq_gswip.c +++ b/drivers/net/dsa/lantiq_gswip.c @@ -276,7 +276,6 @@ struct gswip_priv { int num_gphy_fw; struct gswip_gphy_fw *gphy_fw; u32 port_vlan_filter; - struct mutex pce_table_lock; }; struct gswip_pce_table_entry { @@ -524,14 +523,10 @@ static int gswip_pce_table_entry_read(struct gswip_priv *priv, u16 addr_mode = tbl->key_mode ? GSWIP_PCE_TBL_CTRL_OPMOD_KSRD : GSWIP_PCE_TBL_CTRL_OPMOD_ADRD; - mutex_lock(&priv->pce_table_lock); - err = gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL, GSWIP_PCE_TBL_CTRL_BAS); - if (err) { - mutex_unlock(&priv->pce_table_lock); + if (err) return err; - } gswip_switch_w(priv, tbl->index, GSWIP_PCE_TBL_ADDR); gswip_switch_mask(priv, GSWIP_PCE_TBL_CTRL_ADDR_MASK | @@ -541,10 +536,8 @@ static int gswip_pce_table_entry_read(struct gswip_priv *priv, err = gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL, GSWIP_PCE_TBL_CTRL_BAS); - if (err) { - mutex_unlock(&priv->pce_table_lock); + if (err) return err; - } for (i = 0; i < ARRAY_SIZE(tbl->key); i++) tbl->key[i] = gswip_switch_r(priv, GSWIP_PCE_TBL_KEY(i)); @@ -560,8 +553,6 @@ static int gswip_pce_table_entry_read(struct gswip_priv *priv, tbl->valid = !!(crtl & GSWIP_PCE_TBL_CTRL_VLD); tbl->gmap = (crtl & GSWIP_PCE_TBL_CTRL_GMAP_MASK) >> 7; - mutex_unlock(&priv->pce_table_lock); - return 0; } @@ -574,14 +565,10 @@ static int gswip_pce_table_entry_write(struct gswip_priv *priv, u16 addr_mode = tbl->key_mode ? GSWIP_PCE_TBL_CTRL_OPMOD_KSWR : GSWIP_PCE_TBL_CTRL_OPMOD_ADWR; - mutex_lock(&priv->pce_table_lock); - err = gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL, GSWIP_PCE_TBL_CTRL_BAS); - if (err) { - mutex_unlock(&priv->pce_table_lock); + if (err) return err; - } gswip_switch_w(priv, tbl->index, GSWIP_PCE_TBL_ADDR); gswip_switch_mask(priv, GSWIP_PCE_TBL_CTRL_ADDR_MASK | @@ -613,12 +600,8 @@ static int gswip_pce_table_entry_write(struct gswip_priv *priv, crtl |= GSWIP_PCE_TBL_CTRL_BAS; gswip_switch_w(priv, crtl, GSWIP_PCE_TBL_CTRL); - err = gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL, - GSWIP_PCE_TBL_CTRL_BAS); - - mutex_unlock(&priv->pce_table_lock); - - return err; + return gswip_switch_r_timeout(priv, GSWIP_PCE_TBL_CTRL, + GSWIP_PCE_TBL_CTRL_BAS); } /* Add the LAN port into a bridge with the CPU port by @@ -2121,7 +2104,6 @@ static int gswip_probe(struct platform_device *pdev) priv->ds->priv = priv; priv->ds->ops = priv->hw_info->ops; priv->dev = dev; - mutex_init(&priv->pce_table_lock); version = gswip_switch_r(priv, GSWIP_VERSION); np = dev->of_node; diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h index 21dba16af097..808419f3b808 100644 --- a/drivers/net/dsa/sja1105/sja1105.h +++ b/drivers/net/dsa/sja1105/sja1105.h @@ -261,8 +261,6 @@ struct sja1105_private { * the switch doesn't confuse them with one another. */ struct mutex mgmt_lock; - /* Serializes access to the dynamic config interface */ - struct mutex dynamic_config_lock; struct devlink_region **regions; struct sja1105_cbs_entry *cbs; struct mii_bus *mdio_base_t1; diff --git a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c index 7729d3f8b7f5..f2049f52833c 100644 --- a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c +++ b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c @@ -1170,56 +1170,6 @@ const struct sja1105_dynamic_table_ops sja1110_dyn_ops[BLK_IDX_MAX_DYN] = { }, }; -#define SJA1105_DYNAMIC_CONFIG_SLEEP_US 10 -#define SJA1105_DYNAMIC_CONFIG_TIMEOUT_US 100000 - -static int -sja1105_dynamic_config_poll_valid(struct sja1105_private *priv, - struct sja1105_dyn_cmd *cmd, - const struct sja1105_dynamic_table_ops *ops) -{ - u8 packed_buf[SJA1105_MAX_DYN_CMD_SIZE] = {}; - int rc; - - /* We don't _need_ to read the full entry, just the command area which - * is a fixed SJA1105_SIZE_DYN_CMD. But our cmd_packing() API expects a - * buffer that contains the full entry too. Additionally, our API - * doesn't really know how many bytes into the buffer does the command - * area really begin. So just read back the whole entry. - */ - rc = sja1105_xfer_buf(priv, SPI_READ, ops->addr, packed_buf, - ops->packed_size); - if (rc) - return rc; - - /* Unpack the command structure, and return it to the caller in case it - * needs to perform further checks on it (VALIDENT). - */ - memset(cmd, 0, sizeof(*cmd)); - ops->cmd_packing(packed_buf, cmd, UNPACK); - - /* Hardware hasn't cleared VALID => still working on it */ - return cmd->valid ? -EAGAIN : 0; -} - -/* Poll the dynamic config entry's control area until the hardware has - * cleared the VALID bit, which means we have confirmation that it has - * finished processing the command. - */ -static int -sja1105_dynamic_config_wait_complete(struct sja1105_private *priv, - struct sja1105_dyn_cmd *cmd, - const struct sja1105_dynamic_table_ops *ops) -{ - int rc; - - return read_poll_timeout(sja1105_dynamic_config_poll_valid, - rc, rc != -EAGAIN, - SJA1105_DYNAMIC_CONFIG_SLEEP_US, - SJA1105_DYNAMIC_CONFIG_TIMEOUT_US, - false, priv, cmd, ops); -} - /* Provides read access to the settings through the dynamic interface * of the switch. * @blk_idx is used as key to select from the sja1105_dynamic_table_ops. @@ -1246,6 +1196,7 @@ int sja1105_dynamic_config_read(struct sja1105_private *priv, struct sja1105_dyn_cmd cmd = {0}; /* SPI payload buffer */ u8 packed_buf[SJA1105_MAX_DYN_CMD_SIZE] = {0}; + int retries = 3; int rc; if (blk_idx >= BLK_IDX_MAX_DYN) @@ -1283,21 +1234,33 @@ int sja1105_dynamic_config_read(struct sja1105_private *priv, ops->entry_packing(packed_buf, entry, PACK); /* Send SPI write operation: read config table entry */ - mutex_lock(&priv->dynamic_config_lock); rc = sja1105_xfer_buf(priv, SPI_WRITE, ops->addr, packed_buf, ops->packed_size); - if (rc < 0) { - mutex_unlock(&priv->dynamic_config_lock); - return rc; - } - - rc = sja1105_dynamic_config_wait_complete(priv, &cmd, ops); - mutex_unlock(&priv->dynamic_config_lock); if (rc < 0) return rc; - if (!cmd.valident && !(ops->access & OP_VALID_ANYWAY)) - return -ENOENT; + /* Loop until we have confirmation that hardware has finished + * processing the command and has cleared the VALID field + */ + do { + memset(packed_buf, 0, ops->packed_size); + + /* Retrieve the read operation's result */ + rc = sja1105_xfer_buf(priv, SPI_READ, ops->addr, packed_buf, + ops->packed_size); + if (rc < 0) + return rc; + + cmd = (struct sja1105_dyn_cmd) {0}; + ops->cmd_packing(packed_buf, &cmd, UNPACK); + + if (!cmd.valident && !(ops->access & OP_VALID_ANYWAY)) + return -ENOENT; + cpu_relax(); + } while (cmd.valid && --retries); + + if (cmd.valid) + return -ETIMEDOUT; /* Don't dereference possibly NULL pointer - maybe caller * only wanted to see whether the entry existed or not. @@ -1353,16 +1316,8 @@ int sja1105_dynamic_config_write(struct sja1105_private *priv, ops->entry_packing(packed_buf, entry, PACK); /* Send SPI write operation: read config table entry */ - mutex_lock(&priv->dynamic_config_lock); rc = sja1105_xfer_buf(priv, SPI_WRITE, ops->addr, packed_buf, ops->packed_size); - if (rc < 0) { - mutex_unlock(&priv->dynamic_config_lock); - return rc; - } - - rc = sja1105_dynamic_config_wait_complete(priv, &cmd, ops); - mutex_unlock(&priv->dynamic_config_lock); if (rc < 0) return rc; diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index c343effe2e96..ef46ae53ab56 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -3365,7 +3365,6 @@ static int sja1105_probe(struct spi_device *spi) priv->ds = ds; mutex_init(&priv->ptp_data.lock); - mutex_init(&priv->dynamic_config_lock); mutex_init(&priv->mgmt_lock); rc = sja1105_parse_dt(priv); diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index 33a4a9a17436..4e5ae687d2e2 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -21,13 +21,11 @@ struct ocelot_mact_entry { }; static inline u32 ocelot_mact_read_macaccess(struct ocelot *ocelot) - __must_hold(&ocelot->mact_lock) { return ocelot_read(ocelot, ANA_TABLES_MACACCESS); } static inline int ocelot_mact_wait_for_completion(struct ocelot *ocelot) - __must_hold(&ocelot->mact_lock) { u32 val; @@ -41,7 +39,6 @@ static inline int ocelot_mact_wait_for_completion(struct ocelot *ocelot) static void ocelot_mact_select(struct ocelot *ocelot, const unsigned char mac[ETH_ALEN], unsigned int vid) - __must_hold(&ocelot->mact_lock) { u32 macl = 0, mach = 0; @@ -70,7 +67,6 @@ int ocelot_mact_learn(struct ocelot *ocelot, int port, ANA_TABLES_MACACCESS_ENTRYTYPE(type) | ANA_TABLES_MACACCESS_MAC_TABLE_CMD(MACACCESS_CMD_LEARN); unsigned int mc_ports; - int err; /* Set MAC_CPU_COPY if the CPU port is used by a multicast entry */ if (type == ENTRYTYPE_MACv4) @@ -83,28 +79,18 @@ int ocelot_mact_learn(struct ocelot *ocelot, int port, if (mc_ports & BIT(ocelot->num_phys_ports)) cmd |= ANA_TABLES_MACACCESS_MAC_CPU_COPY; - mutex_lock(&ocelot->mact_lock); - ocelot_mact_select(ocelot, mac, vid); /* Issue a write command */ ocelot_write(ocelot, cmd, ANA_TABLES_MACACCESS); - err = ocelot_mact_wait_for_completion(ocelot); - - mutex_unlock(&ocelot->mact_lock); - - return err; + return ocelot_mact_wait_for_completion(ocelot); } EXPORT_SYMBOL(ocelot_mact_learn); int ocelot_mact_forget(struct ocelot *ocelot, const unsigned char mac[ETH_ALEN], unsigned int vid) { - int err; - - mutex_lock(&ocelot->mact_lock); - ocelot_mact_select(ocelot, mac, vid); /* Issue a forget command */ @@ -112,11 +98,7 @@ int ocelot_mact_forget(struct ocelot *ocelot, ANA_TABLES_MACACCESS_MAC_TABLE_CMD(MACACCESS_CMD_FORGET), ANA_TABLES_MACACCESS); - err = ocelot_mact_wait_for_completion(ocelot); - - mutex_unlock(&ocelot->mact_lock); - - return err; + return ocelot_mact_wait_for_completion(ocelot); } EXPORT_SYMBOL(ocelot_mact_forget); @@ -132,9 +114,7 @@ static void ocelot_mact_init(struct ocelot *ocelot) | ANA_AGENCTRL_LEARN_IGNORE_VLAN, ANA_AGENCTRL); - /* Clear the MAC table. We are not concurrent with anyone, so - * holding &ocelot->mact_lock is pointless. - */ + /* Clear the MAC table */ ocelot_write(ocelot, MACACCESS_CMD_INIT, ANA_TABLES_MACACCESS); } @@ -1192,7 +1172,6 @@ EXPORT_SYMBOL(ocelot_port_fdb_do_dump); static int ocelot_mact_read(struct ocelot *ocelot, int port, int row, int col, struct ocelot_mact_entry *entry) - __must_hold(&ocelot->mact_lock) { u32 val, dst, macl, mach; char mac[ETH_ALEN]; @@ -1241,40 +1220,33 @@ static int ocelot_mact_read(struct ocelot *ocelot, int port, int row, int col, int ocelot_fdb_dump(struct ocelot *ocelot, int port, dsa_fdb_dump_cb_t *cb, void *data) { - int err = 0; int i, j; - /* We could take the lock just around ocelot_mact_read, but doing so - * thousands of times in a row seems rather pointless and inefficient. - */ - mutex_lock(&ocelot->mact_lock); - /* Loop through all the mac tables entries. */ for (i = 0; i < ocelot->num_mact_rows; i++) { for (j = 0; j < 4; j++) { struct ocelot_mact_entry entry; bool is_static; + int ret; - err = ocelot_mact_read(ocelot, port, i, j, &entry); + ret = ocelot_mact_read(ocelot, port, i, j, &entry); /* If the entry is invalid (wrong port, invalid...), * skip it. */ - if (err == -EINVAL) + if (ret == -EINVAL) continue; - else if (err) - break; + else if (ret) + return ret; is_static = (entry.type == ENTRYTYPE_LOCKED); - err = cb(entry.mac, entry.vid, is_static, data); - if (err) - break; + ret = cb(entry.mac, entry.vid, is_static, data); + if (ret) + return ret; } } - mutex_unlock(&ocelot->mact_lock); - - return err; + return 0; } EXPORT_SYMBOL(ocelot_fdb_dump); @@ -2259,7 +2231,6 @@ int ocelot_init(struct ocelot *ocelot) mutex_init(&ocelot->stats_lock); mutex_init(&ocelot->ptp_lock); - mutex_init(&ocelot->mact_lock); spin_lock_init(&ocelot->ptp_clock_lock); spin_lock_init(&ocelot->ts_id_lock); snprintf(queue_name, sizeof(queue_name), "%s-stats", diff --git a/include/net/dsa.h b/include/net/dsa.h index badd214f7470..1cd9c2461f0d 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -287,7 +287,6 @@ struct dsa_port { /* List of MAC addresses that must be forwarded on this port. * These are only valid on CPU ports and DSA links. */ - struct mutex addr_lists_lock; struct list_head fdbs; struct list_head mdbs; diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h index fef3a36b0210..9b872da0c246 100644 --- a/include/soc/mscc/ocelot.h +++ b/include/soc/mscc/ocelot.h @@ -675,9 +675,6 @@ struct ocelot { struct delayed_work stats_work; struct workqueue_struct *stats_queue; - /* Lock for serializing access to the MAC table */ - struct mutex mact_lock; - struct workqueue_struct *owq; u8 ptp:1; diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 826957b6442b..f5270114dcb8 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -433,7 +433,6 @@ static int dsa_port_setup(struct dsa_port *dp) if (dp->setup) return 0; - mutex_init(&dp->addr_lists_lock); INIT_LIST_HEAD(&dp->fdbs); INIT_LIST_HEAD(&dp->mdbs); diff --git a/net/dsa/slave.c b/net/dsa/slave.c index adcfb2cb4e61..9d9fef668eba 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2413,6 +2413,7 @@ 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) @@ -2447,6 +2448,7 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work) break; } + rtnl_unlock(); dev_put(switchdev_work->dev); kfree(switchdev_work); diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 6871e5f9b597..2b1b21bde830 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -215,30 +215,26 @@ 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 = 0; + int err; /* 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); - goto out; + return 0; } a = kzalloc(sizeof(*a), GFP_KERNEL); - if (!a) { - err = -ENOMEM; - goto out; - } + if (!a) + return -ENOMEM; err = ds->ops->port_mdb_add(ds, port, mdb); if (err) { kfree(a); - goto out; + return err; } ether_addr_copy(a->addr, mdb->addr); @@ -246,10 +242,7 @@ static int dsa_port_do_mdb_add(struct dsa_port *dp, refcount_set(&a->refcount, 1); list_add_tail(&a->list, &dp->mdbs); -out: - mutex_unlock(&dp->addr_lists_lock); - - return err; + return 0; } static int dsa_port_do_mdb_del(struct dsa_port *dp, @@ -258,36 +251,29 @@ 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 = 0; + int err; /* 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) { - err = -ENOENT; - goto out; - } + if (!a) + return -ENOENT; if (!refcount_dec_and_test(&a->refcount)) - goto out; + return 0; err = ds->ops->port_mdb_del(ds, port, mdb); if (err) { refcount_inc(&a->refcount); - goto out; + return err; } list_del(&a->list); kfree(a); -out: - mutex_unlock(&dp->addr_lists_lock); - - return err; + return 0; } static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr, @@ -296,30 +282,26 @@ 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 = 0; + int err; /* 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); - goto out; + return 0; } a = kzalloc(sizeof(*a), GFP_KERNEL); - if (!a) { - err = -ENOMEM; - goto out; - } + if (!a) + return -ENOMEM; err = ds->ops->port_fdb_add(ds, port, addr, vid); if (err) { kfree(a); - goto out; + return err; } ether_addr_copy(a->addr, addr); @@ -327,10 +309,7 @@ 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); -out: - mutex_unlock(&dp->addr_lists_lock); - - return err; + return 0; } static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr, @@ -339,36 +318,29 @@ 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 = 0; + int err; /* 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) { - err = -ENOENT; - goto out; - } + if (!a) + return -ENOENT; if (!refcount_dec_and_test(&a->refcount)) - goto out; + return 0; err = ds->ops->port_fdb_del(ds, port, addr, vid); if (err) { refcount_inc(&a->refcount); - goto out; + return err; } list_del(&a->list); kfree(a); -out: - mutex_unlock(&dp->addr_lists_lock); - - return err; + return 0; } static int dsa_switch_host_fdb_add(struct dsa_switch *ds, diff --git a/tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh b/tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh deleted file mode 100755 index dca8be6092b9..000000000000 --- a/tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh +++ /dev/null @@ -1,47 +0,0 @@ -#!/bin/bash -# SPDX-License-Identifier: GPL-2.0 - -# Bridge FDB entries can be offloaded to DSA switches without holding the -# rtnl_mutex. Traditionally this mutex has conferred drivers implicit -# serialization, which means their code paths are not well tested in the -# presence of concurrency. -# This test creates a background task that stresses the FDB by adding and -# deleting an entry many times in a row without the rtnl_mutex held. -# It then tests the driver resistance to concurrency by calling .ndo_fdb_dump -# (with rtnl_mutex held) from a foreground task. -# Since either the FDB dump or the additions/removals can fail, but the -# additions and removals are performed in deferred as opposed to process -# context, we cannot simply check for user space error codes. - -WAIT_TIME=1 -NUM_NETIFS=1 -REQUIRE_JQ="no" -REQUIRE_MZ="no" -NETIF_CREATE="no" -lib_dir=$(dirname $0)/../../../net/forwarding -source $lib_dir/lib.sh - -cleanup() { - echo "Cleaning up" - kill $pid && wait $pid &> /dev/null - ip link del br0 - echo "Please check kernel log for errors" -} -trap 'cleanup' EXIT - -eth=${NETIFS[p1]} - -ip link del br0 2&>1 >/dev/null || : -ip link add br0 type bridge && ip link set $eth master br0 - -(while :; do - bridge fdb add 00:01:02:03:04:05 dev $eth master static - bridge fdb del 00:01:02:03:04:05 dev $eth master static -done) & -pid=$! - -for i in $(seq 1 50); do - bridge fdb show > /dev/null - sleep 3 - echo "$((${i} * 2))% complete..." -done diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 520d8b53464b..92087d423bcf 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -23,8 +23,6 @@ MC_CLI=${MC_CLI:=smcroutectl} PING_TIMEOUT=${PING_TIMEOUT:=5} WAIT_TIMEOUT=${WAIT_TIMEOUT:=20} INTERFACE_TIMEOUT=${INTERFACE_TIMEOUT:=600} -REQUIRE_JQ=${REQUIRE_JQ:=yes} -REQUIRE_MZ=${REQUIRE_MZ:=yes} relative_path="${BASH_SOURCE%/*}" if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then @@ -143,12 +141,8 @@ require_command() fi } -if [[ "$REQUIRE_JQ" = "yes" ]]; then - require_command jq -fi -if [[ "$REQUIRE_MZ" = "yes" ]]; then - require_command $MZ -fi +require_command jq +require_command $MZ if [[ ! -v NUM_NETIFS ]]; then echo "SKIP: importer does not define \"NUM_NETIFS\"" -- cgit v1.2.3-58-ga151 From 232deb3f9567ce37d99b8616a6c07c1fc0436abf Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 24 Oct 2021 20:17:48 +0300 Subject: net: dsa: avoid refcount warnings when ->port_{fdb,mdb}_del returns error At present, when either of ds->ops->port_fdb_del() or ds->ops->port_mdb_del() return a non-zero error code, we attempt to save the day and keep the data structure associated with that switchdev object, as the deletion procedure did not complete. However, the way in which we do this is suspicious to the checker in lib/refcount.c, who thinks it is buggy to increment a refcount that became zero, and that this is indicative of a use-after-free. Fixes: 161ca59d39e9 ("net: dsa: reference count the MDB entries at the cross-chip notifier level") Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- net/dsa/switch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/dsa/switch.c') diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 2b1b21bde830..8f8ed8248c2c 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -266,7 +266,7 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp, err = ds->ops->port_mdb_del(ds, port, mdb); if (err) { - refcount_inc(&a->refcount); + refcount_set(&a->refcount, 1); return err; } @@ -333,7 +333,7 @@ static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr, err = ds->ops->port_fdb_del(ds, port, addr, vid); if (err) { - refcount_inc(&a->refcount); + refcount_set(&a->refcount, 1); return err; } -- cgit v1.2.3-58-ga151 From 338a3a4745aae748835eb611765a8e36779810e1 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Sun, 24 Oct 2021 20:17:54 +0300 Subject: net: dsa: introduce locking for the address lists on CPU and DSA ports Now that the rtnl_mutex is going away for dsa_port_{host_,}fdb_{add,del}, no one is serializing access to the address lists that DSA keeps for the purpose of reference counting on shared ports (CPU and cascade ports). It can happen for one dsa_switch_do_fdb_del to do list_del on a dp->fdbs element while another dsa_switch_do_fdb_{add,del} is traversing dp->fdbs. We need to avoid that. Currently dp->mdbs is not at risk, because dsa_switch_do_mdb_{add,del} still runs under the rtnl_mutex. But it would be nice if it would not depend on that being the case. So let's introduce a mutex per port (the address lists are per port too) and share it between dp->mdbs and dp->fdbs. The place where we put the locking is interesting. It could be tempting to put a DSA-level lock which still serializes calls to .port_fdb_{add,del}, but it would still not avoid concurrency with other driver code paths that are currently under rtnl_mutex (.port_fdb_dump, .port_fast_age). So it would add a very false sense of security (and adding a global switch-wide lock in DSA to resynchronize with the rtnl_lock is also counterproductive and hard). So the locking is intentionally done only where the dp->fdbs and dp->mdbs lists are traversed. That means, from a driver perspective, that .port_fdb_add will be called with the dp->addr_lists_lock mutex held on the CPU port, but not held on user ports. This is done so that driver writers are not encouraged to rely on any guarantee offered by dp->addr_lists_lock. Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- include/net/dsa.h | 1 + net/dsa/dsa2.c | 1 + net/dsa/switch.c | 76 +++++++++++++++++++++++++++++++++++++------------------ 3 files changed, 54 insertions(+), 24 deletions(-) (limited to 'net/dsa/switch.c') diff --git a/include/net/dsa.h b/include/net/dsa.h index 1cd9c2461f0d..badd214f7470 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -287,6 +287,7 @@ struct dsa_port { /* List of MAC addresses that must be forwarded on this port. * These are only valid on CPU ports and DSA links. */ + struct mutex addr_lists_lock; struct list_head fdbs; struct list_head mdbs; diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index f5270114dcb8..826957b6442b 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -433,6 +433,7 @@ static int dsa_port_setup(struct dsa_port *dp) if (dp->setup) return 0; + mutex_init(&dp->addr_lists_lock); INIT_LIST_HEAD(&dp->fdbs); INIT_LIST_HEAD(&dp->mdbs); diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 8f8ed8248c2c..bb155a16d454 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_set(&a->refcount, 1); - 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_set(&a->refcount, 1); - 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, -- cgit v1.2.3-58-ga151