From 3f9bb0301d50ce27421eff4b710c2bbe58111a83 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 6 Dec 2021 18:57:47 +0200 Subject: net: dsa: make dp->bridge_num one-based MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I have seen too many bugs already due to the fact that we must encode an invalid dp->bridge_num as a negative value, because the natural tendency is to check that invalid value using (!dp->bridge_num). Latest example can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not getting cleared after ports leaving the bridge"). Convert the existing users to assume that dp->bridge_num == 0 is the encoding for invalid, and valid bridge numbers start from 1. Signed-off-by: Vladimir Oltean Reviewed-by: Alvin Šipraga Signed-off-by: Jakub Kicinski --- net/dsa/dsa2.c | 24 ++++++++++++------------ net/dsa/dsa_priv.h | 5 +++-- net/dsa/port.c | 11 ++++++----- net/dsa/tag_8021q.c | 12 +++++++----- net/dsa/tag_dsa.c | 2 +- 5 files changed, 29 insertions(+), 25 deletions(-) (limited to 'net/dsa') diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index 826957b6442b..9606e56710a5 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -141,23 +141,23 @@ static int dsa_bridge_num_find(const struct net_device *bridge_dev) */ list_for_each_entry(dst, &dsa_tree_list, list) list_for_each_entry(dp, &dst->ports, list) - if (dp->bridge_dev == bridge_dev && - dp->bridge_num != -1) + if (dp->bridge_dev == bridge_dev && dp->bridge_num) return dp->bridge_num; - return -1; + return 0; } -int dsa_bridge_num_get(const struct net_device *bridge_dev, int max) +unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max) { - int bridge_num = dsa_bridge_num_find(bridge_dev); + unsigned int bridge_num = dsa_bridge_num_find(bridge_dev); - if (bridge_num < 0) { + if (!bridge_num) { /* First port that offloads TX forwarding for this bridge */ - bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges, - DSA_MAX_NUM_OFFLOADING_BRIDGES); + bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges, + DSA_MAX_NUM_OFFLOADING_BRIDGES, + 1); if (bridge_num >= max) - return -1; + return 0; set_bit(bridge_num, &dsa_fwd_offloading_bridges); } @@ -165,12 +165,13 @@ int dsa_bridge_num_get(const struct net_device *bridge_dev, int max) return bridge_num; } -void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num) +void dsa_bridge_num_put(const struct net_device *bridge_dev, + unsigned int bridge_num) { /* Check if the bridge is still in use, otherwise it is time * to clean it up so we can reuse this bridge_num later. */ - if (dsa_bridge_num_find(bridge_dev) < 0) + if (!dsa_bridge_num_find(bridge_dev)) clear_bit(bridge_num, &dsa_fwd_offloading_bridges); } @@ -1184,7 +1185,6 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index) dp->ds = ds; dp->index = index; - dp->bridge_num = -1; INIT_LIST_HEAD(&dp->list); list_add_tail(&dp->list, &dst->ports); diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 3fb2c37c9b88..70c4a5b36a8b 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -546,8 +546,9 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst, struct net_device *master, const struct dsa_device_ops *tag_ops, const struct dsa_device_ops *old_tag_ops); -int dsa_bridge_num_get(const struct net_device *bridge_dev, int max); -void dsa_bridge_num_put(const struct net_device *bridge_dev, int bridge_num); +unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max); +void dsa_bridge_num_put(const struct net_device *bridge_dev, + unsigned int bridge_num); /* tag_8021q.c */ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds, diff --git a/net/dsa/port.c b/net/dsa/port.c index 6d5ebe61280b..9a77bd1373e2 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -273,14 +273,14 @@ 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) { - int bridge_num = dp->bridge_num; + unsigned int bridge_num = dp->bridge_num; struct dsa_switch *ds = dp->ds; /* No bridge TX forwarding offload => do nothing */ - if (!ds->ops->port_bridge_tx_fwd_unoffload || dp->bridge_num == -1) + if (!ds->ops->port_bridge_tx_fwd_unoffload || !dp->bridge_num) return; - dp->bridge_num = -1; + dp->bridge_num = 0; dsa_bridge_num_put(bridge_dev, bridge_num); @@ -295,14 +295,15 @@ static bool dsa_port_bridge_tx_fwd_offload(struct dsa_port *dp, struct net_device *bridge_dev) { struct dsa_switch *ds = dp->ds; - int bridge_num, err; + unsigned int bridge_num; + int err; if (!ds->ops->port_bridge_tx_fwd_offload) return false; bridge_num = dsa_bridge_num_get(bridge_dev, ds->num_fwd_offloading_bridges); - if (bridge_num < 0) + if (!bridge_num) return false; dp->bridge_num = bridge_num; diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c index 72cac2c0af7b..df59f16436a5 100644 --- a/net/dsa/tag_8021q.c +++ b/net/dsa/tag_8021q.c @@ -67,10 +67,12 @@ #define DSA_8021Q_PORT(x) (((x) << DSA_8021Q_PORT_SHIFT) & \ DSA_8021Q_PORT_MASK) -u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num) +u16 dsa_8021q_bridge_tx_fwd_offload_vid(unsigned int bridge_num) { - /* The VBID value of 0 is reserved for precise TX */ - return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num + 1); + /* The VBID value of 0 is reserved for precise TX, but it is also + * reserved/invalid for the bridge_num, so all is well. + */ + return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num); } EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid); @@ -409,7 +411,7 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds, int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port, struct net_device *br, - int bridge_num) + unsigned int bridge_num) { u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num); @@ -420,7 +422,7 @@ EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload); void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port, struct net_device *br, - int bridge_num) + unsigned int bridge_num) { u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num); diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c index b3da4b2ea11c..a7d70ae7cc97 100644 --- a/net/dsa/tag_dsa.c +++ b/net/dsa/tag_dsa.c @@ -140,7 +140,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev, * packets on behalf of a virtual switch device with an index * past the physical switches. */ - tag_dev = dst->last_switch + 1 + dp->bridge_num; + tag_dev = dst->last_switch + dp->bridge_num; tag_port = 0; } else { cmd = DSA_CMD_FROM_CPU; -- cgit v1.2.3