From d3eed0e57d5d1bcbf1bd60f83a4adfe7d7b8dd9c Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 6 Dec 2021 18:57:56 +0200 Subject: net: dsa: keep the bridge_dev and bridge_num as part of the same structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main desire behind this is to provide coherent bridge information to the fast path without locking. For example, right now we set dp->bridge_dev and dp->bridge_num from separate code paths, it is theoretically possible for a packet transmission to read these two port properties consecutively and find a bridge number which does not correspond with the bridge device. Another desire is to start passing more complex bridge information to dsa_switch_ops functions. For example, with FDB isolation, it is expected that drivers will need to be passed the bridge which requested an FDB/MDB entry to be offloaded, and along with that bridge_dev, the associated bridge_num should be passed too, in case the driver might want to implement an isolation scheme based on that number. We already pass the {bridge_dev, bridge_num} pair to the TX forwarding offload switch API, however we'd like to remove that and squash it into the basic bridge join/leave API. So that means we need to pass this pair to the bridge join/leave API. During dsa_port_bridge_leave, first we unset dp->bridge_dev, then we call the driver's .port_bridge_leave with what used to be our dp->bridge_dev, but provided as an argument. When bridge_dev and bridge_num get folded into a single structure, we need to preserve this behavior in dsa_port_bridge_leave: we need a copy of what used to be in dp->bridge. Switch drivers check bridge membership by comparing dp->bridge_dev with the provided bridge_dev, but now, if we provide the struct dsa_bridge as a pointer, they cannot keep comparing dp->bridge to the provided pointer, since this only points to an on-stack copy. To make this obvious and prevent driver writers from forgetting and doing stupid things, in this new API, the struct dsa_bridge is provided as a full structure (not very large, contains an int and a pointer) instead of a pointer. An explicit comparison function needs to be used to determine bridge membership: dsa_port_offloads_bridge(). Signed-off-by: Vladimir Oltean Reviewed-by: Alvin Šipraga Signed-off-by: Jakub Kicinski --- net/dsa/port.c | 70 +++++++++++++++++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 32 deletions(-) (limited to 'net/dsa/port.c') diff --git a/net/dsa/port.c b/net/dsa/port.c index f6ea41cbcdd5..fbf2d7fc5c91 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -130,7 +130,7 @@ int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy) return err; } - if (!dp->bridge_dev) + if (!dp->bridge) dsa_port_set_state_now(dp, BR_STATE_FORWARDING, false); if (dp->pl) @@ -158,7 +158,7 @@ void dsa_port_disable_rt(struct dsa_port *dp) if (dp->pl) phylink_stop(dp->pl); - if (!dp->bridge_dev) + if (!dp->bridge) dsa_port_set_state_now(dp, BR_STATE_DISABLED, false); if (ds->ops->port_disable) @@ -271,36 +271,32 @@ static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp) } static void dsa_port_bridge_tx_fwd_unoffload(struct dsa_port *dp, - struct net_device *bridge_dev, - unsigned int bridge_num) + struct dsa_bridge bridge) { struct dsa_switch *ds = dp->ds; /* No bridge TX forwarding offload => do nothing */ - if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge_num) + if (!ds->ops->port_bridge_tx_fwd_unoffload || !bridge.num) return; /* Notify the chips only once the offload has been deactivated, so * that they can update their configuration accordingly. */ - ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge_dev, - bridge_num); + ds->ops->port_bridge_tx_fwd_unoffload(ds, dp->index, bridge); } static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp, - struct net_device *bridge_dev, - unsigned int bridge_num) + struct dsa_bridge bridge) { struct dsa_switch *ds = dp->ds; int err; /* FDB isolation is required for TX forwarding offload */ - if (!ds->ops->port_bridge_tx_fwd_offload || !bridge_num) + if (!ds->ops->port_bridge_tx_fwd_offload || !bridge.num) return false; /* Notify the driver */ - err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge_dev, - bridge_num); + err = ds->ops->port_bridge_tx_fwd_offload(ds, dp->index, bridge); return err ? false : true; } @@ -310,21 +306,32 @@ static int dsa_port_bridge_create(struct dsa_port *dp, struct netlink_ext_ack *extack) { struct dsa_switch *ds = dp->ds; - unsigned int bridge_num; + struct dsa_bridge *bridge; - dp->bridge_dev = br; - - if (!ds->max_num_bridges) + bridge = dsa_tree_bridge_find(ds->dst, br); + if (bridge) { + refcount_inc(&bridge->refcount); + dp->bridge = bridge; return 0; + } + + bridge = kzalloc(sizeof(*bridge), GFP_KERNEL); + if (!bridge) + return -ENOMEM; + + refcount_set(&bridge->refcount, 1); + + bridge->dev = br; - bridge_num = dsa_bridge_num_get(br, ds->max_num_bridges); - if (!bridge_num) { + bridge->num = dsa_bridge_num_get(br, ds->max_num_bridges); + if (ds->max_num_bridges && !bridge->num) { NL_SET_ERR_MSG_MOD(extack, "Range of offloadable bridges exceeded"); + kfree(bridge); return -EOPNOTSUPP; } - dp->bridge_num = bridge_num; + dp->bridge = bridge; return 0; } @@ -332,16 +339,17 @@ static int dsa_port_bridge_create(struct dsa_port *dp, static void dsa_port_bridge_destroy(struct dsa_port *dp, const struct net_device *br) { - struct dsa_switch *ds = dp->ds; + struct dsa_bridge *bridge = dp->bridge; + + dp->bridge = NULL; - dp->bridge_dev = NULL; + if (!refcount_dec_and_test(&bridge->refcount)) + return; - if (ds->max_num_bridges) { - int bridge_num = dp->bridge_num; + if (bridge->num) + dsa_bridge_num_put(br, bridge->num); - dp->bridge_num = 0; - dsa_bridge_num_put(br, bridge_num); - } + kfree(bridge); } int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br, @@ -351,7 +359,6 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br, .tree_index = dp->ds->dst->index, .sw_index = dp->ds->index, .port = dp->index, - .br = br, }; struct net_device *dev = dp->slave; struct net_device *brport_dev; @@ -367,12 +374,12 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br, brport_dev = dsa_port_to_bridge_port(dp); + info.bridge = *dp->bridge; err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_JOIN, &info); if (err) goto out_rollback; - tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, br, - dsa_port_bridge_num_get(dp)); + tx_fwd_offload = dsa_port_bridge_tx_fwd_offload(dp, info.bridge); err = switchdev_bridge_port_offload(brport_dev, dev, dp, &dsa_slave_switchdev_notifier, @@ -415,12 +422,11 @@ void dsa_port_pre_bridge_leave(struct dsa_port *dp, struct net_device *br) void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br) { - unsigned int bridge_num = dsa_port_bridge_num_get(dp); struct dsa_notifier_bridge_info info = { .tree_index = dp->ds->dst->index, .sw_index = dp->ds->index, .port = dp->index, - .br = br, + .bridge = *dp->bridge, }; int err; @@ -429,7 +435,7 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br) */ dsa_port_bridge_destroy(dp, br); - dsa_port_bridge_tx_fwd_unoffload(dp, br, bridge_num); + dsa_port_bridge_tx_fwd_unoffload(dp, info.bridge); err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info); if (err) -- cgit v1.2.3