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 --- include/net/dsa.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'include/net/dsa.h') diff --git a/include/net/dsa.h b/include/net/dsa.h index 8ca9d50cbbc2..a23cfbaa09d6 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -257,7 +257,7 @@ struct dsa_port { bool learning; u8 stp_state; struct net_device *bridge_dev; - int bridge_num; + unsigned int bridge_num; struct devlink_port devlink_port; bool devlink_port_setup; struct phylink *pl; @@ -754,11 +754,11 @@ struct dsa_switch_ops { /* Called right after .port_bridge_join() */ int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port, struct net_device *bridge, - int bridge_num); + unsigned int bridge_num); /* Called right before .port_bridge_leave() */ void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port, struct net_device *bridge, - int bridge_num); + unsigned int bridge_num); void (*port_stp_state_set)(struct dsa_switch *ds, int port, u8 state); void (*port_fast_age)(struct dsa_switch *ds, int port); -- cgit v1.2.3 From 947c8746e2c31bb469ba9ee02dc739be6a98ee38 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 6 Dec 2021 18:57:48 +0200 Subject: net: dsa: assign a bridge number even without TX forwarding offload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The service where DSA assigns a unique bridge number for each forwarding domain is useful even for drivers which do not implement the TX forwarding offload feature. For example, drivers might use the dp->bridge_num for FDB isolation. So rename ds->num_fwd_offloading_bridges to ds->max_num_bridges, and calculate a unique bridge_num for all drivers that set this value. Signed-off-by: Vladimir Oltean Reviewed-by: Alvin Šipraga Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'include/net/dsa.h') diff --git a/include/net/dsa.h b/include/net/dsa.h index a23cfbaa09d6..00fbd87ae4ff 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -413,12 +413,12 @@ struct dsa_switch { */ unsigned int num_lag_ids; - /* Drivers that support bridge forwarding offload should set this to - * the maximum number of bridges spanning the same switch tree (or all - * trees, in the case of cross-tree bridging support) that can be - * offloaded. + /* Drivers that support bridge forwarding offload or FDB isolation + * should set this to the maximum number of bridges spanning the same + * switch tree (or all trees, in the case of cross-tree bridging + * support) that can be offloaded. */ - unsigned int num_fwd_offloading_bridges; + unsigned int max_num_bridges; size_t num_ports; }; -- cgit v1.2.3 From 36cbf39b56908bb2e7e8e392e5f08895c3ca4326 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 6 Dec 2021 18:57:52 +0200 Subject: net: dsa: hide dp->bridge_dev and dp->bridge_num in the core behind helpers The location of the bridge device pointer and number is going to change. It is not going to be kept individually per port, but in a common structure allocated dynamically and which will have lockdep validation. Create helpers to access these elements so that we have a migration path to the new organization. Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'include/net/dsa.h') diff --git a/include/net/dsa.h b/include/net/dsa.h index 00fbd87ae4ff..18bce0383267 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -599,6 +599,27 @@ struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp) return dp->slave; } +static inline struct net_device * +dsa_port_bridge_dev_get(const struct dsa_port *dp) +{ + return dp->bridge_dev; +} + +static inline unsigned int dsa_port_bridge_num_get(struct dsa_port *dp) +{ + return dp->bridge_num; +} + +static inline bool dsa_port_bridge_same(const struct dsa_port *a, + const struct dsa_port *b) +{ + struct net_device *br_a = dsa_port_bridge_dev_get(a); + struct net_device *br_b = dsa_port_bridge_dev_get(b); + + /* Standalone ports are not in the same bridge with one another */ + return (!br_a || !br_b) ? false : (br_a == br_b); +} + typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid, bool is_static, void *data); struct dsa_switch_ops { -- cgit v1.2.3 From 6a43cba3034015d1c029c8a81b62eb9c2660fd6e Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 6 Dec 2021 18:57:55 +0200 Subject: net: dsa: export bridging offload helpers to drivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the static inline helpers from net/dsa/dsa_priv.h to include/net/dsa.h, so that drivers can call functions such as dsa_port_offloads_bridge_dev(), which will be necessary after the transition to a more complex bridge structure. More functions than are needed right now are being moved, but this is done for uniformity. Signed-off-by: Vladimir Oltean Reviewed-by: Alvin Šipraga Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'include/net/dsa.h') diff --git a/include/net/dsa.h b/include/net/dsa.h index 18bce0383267..899e13d56fc2 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -620,6 +620,49 @@ static inline bool dsa_port_bridge_same(const struct dsa_port *a, return (!br_a || !br_b) ? false : (br_a == br_b); } +static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp, + const struct net_device *dev) +{ + return dsa_port_to_bridge_port(dp) == dev; +} + +static inline bool +dsa_port_offloads_bridge_dev(struct dsa_port *dp, + const struct net_device *bridge_dev) +{ + /* DSA ports connected to a bridge, and event was emitted + * for the bridge. + */ + return dsa_port_bridge_dev_get(dp) == bridge_dev; +} + +/* Returns true if any port of this tree offloads the given net_device */ +static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst, + const struct net_device *dev) +{ + struct dsa_port *dp; + + list_for_each_entry(dp, &dst->ports, list) + if (dsa_port_offloads_bridge_port(dp, dev)) + return true; + + return false; +} + +/* Returns true if any port of this tree offloads the given bridge */ +static inline bool +dsa_tree_offloads_bridge_dev(struct dsa_switch_tree *dst, + const struct net_device *bridge_dev) +{ + struct dsa_port *dp; + + list_for_each_entry(dp, &dst->ports, list) + if (dsa_port_offloads_bridge_dev(dp, bridge_dev)) + return true; + + return false; +} + typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid, bool is_static, void *data); struct dsa_switch_ops { -- cgit v1.2.3 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 --- include/net/dsa.h | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) (limited to 'include/net/dsa.h') diff --git a/include/net/dsa.h b/include/net/dsa.h index 899e13d56fc2..b9789c0cd5e3 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -219,6 +219,11 @@ struct dsa_mall_tc_entry { }; }; +struct dsa_bridge { + struct net_device *dev; + unsigned int num; + refcount_t refcount; +}; struct dsa_port { /* A CPU port is physically connected to a master device. @@ -256,8 +261,7 @@ struct dsa_port { /* Managed by DSA on user ports and by drivers on CPU and DSA ports */ bool learning; u8 stp_state; - struct net_device *bridge_dev; - unsigned int bridge_num; + struct dsa_bridge *bridge; struct devlink_port devlink_port; bool devlink_port_setup; struct phylink *pl; @@ -588,7 +592,7 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp) static inline struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp) { - if (!dp->bridge_dev) + if (!dp->bridge) return NULL; if (dp->lag_dev) @@ -602,12 +606,12 @@ struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp) static inline struct net_device * dsa_port_bridge_dev_get(const struct dsa_port *dp) { - return dp->bridge_dev; + return dp->bridge ? dp->bridge->dev : NULL; } static inline unsigned int dsa_port_bridge_num_get(struct dsa_port *dp) { - return dp->bridge_num; + return dp->bridge ? dp->bridge->num : 0; } static inline bool dsa_port_bridge_same(const struct dsa_port *a, @@ -636,6 +640,12 @@ dsa_port_offloads_bridge_dev(struct dsa_port *dp, return dsa_port_bridge_dev_get(dp) == bridge_dev; } +static inline bool dsa_port_offloads_bridge(struct dsa_port *dp, + const struct dsa_bridge *bridge) +{ + return dsa_port_bridge_dev_get(dp) == bridge->dev; +} + /* Returns true if any port of this tree offloads the given net_device */ static inline bool dsa_tree_offloads_bridge_port(struct dsa_switch_tree *dst, const struct net_device *dev) @@ -812,17 +822,15 @@ struct dsa_switch_ops { */ int (*set_ageing_time)(struct dsa_switch *ds, unsigned int msecs); int (*port_bridge_join)(struct dsa_switch *ds, int port, - struct net_device *bridge); + struct dsa_bridge bridge); void (*port_bridge_leave)(struct dsa_switch *ds, int port, - struct net_device *bridge); + struct dsa_bridge bridge); /* Called right after .port_bridge_join() */ int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port, - struct net_device *bridge, - unsigned int bridge_num); + struct dsa_bridge bridge); /* Called right before .port_bridge_leave() */ void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port, - struct net_device *bridge, - unsigned int bridge_num); + struct dsa_bridge bridge); void (*port_stp_state_set)(struct dsa_switch *ds, int port, u8 state); void (*port_fast_age)(struct dsa_switch *ds, int port); @@ -894,10 +902,10 @@ struct dsa_switch_ops { */ int (*crosschip_bridge_join)(struct dsa_switch *ds, int tree_index, int sw_index, int port, - struct net_device *br); + struct dsa_bridge bridge); void (*crosschip_bridge_leave)(struct dsa_switch *ds, int tree_index, int sw_index, int port, - struct net_device *br); + struct dsa_bridge bridge); int (*crosschip_lag_change)(struct dsa_switch *ds, int sw_index, int port); int (*crosschip_lag_join)(struct dsa_switch *ds, int sw_index, -- cgit v1.2.3 From b079922ba2acf072b23d82fa246a0d8de198f0a2 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 6 Dec 2021 18:57:57 +0200 Subject: net: dsa: add a "tx_fwd_offload" argument to ->port_bridge_join MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a preparation patch for the removal of the DSA switch methods ->port_bridge_tx_fwd_offload() and ->port_bridge_tx_fwd_unoffload(). The plan is for the switch to report whether it offloads TX forwarding directly as a response to the ->port_bridge_join() method. This change deals with the noisy portion of converting all existing function prototypes to take this new boolean pointer argument. The bool is placed in the cross-chip notifier structure for bridge join, and a reference to it is provided to drivers. In the next change, DSA will then actually look at this value instead of calling ->port_bridge_tx_fwd_offload(). Signed-off-by: Vladimir Oltean Reviewed-by: Alvin Šipraga Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include/net/dsa.h') diff --git a/include/net/dsa.h b/include/net/dsa.h index b9789c0cd5e3..584b3f9462a0 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -822,7 +822,8 @@ struct dsa_switch_ops { */ int (*set_ageing_time)(struct dsa_switch *ds, unsigned int msecs); int (*port_bridge_join)(struct dsa_switch *ds, int port, - struct dsa_bridge bridge); + struct dsa_bridge bridge, + bool *tx_fwd_offload); void (*port_bridge_leave)(struct dsa_switch *ds, int port, struct dsa_bridge bridge); /* Called right after .port_bridge_join() */ -- cgit v1.2.3 From 857fdd74fb38dad4d0333401fb1826cd8cf0df2c Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 6 Dec 2021 18:57:58 +0200 Subject: net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't really need new switch API for these, and with new switches which intend to add support for this feature, it will become cumbersome to maintain. The change consists in restructuring the two drivers that implement this offload (sja1105 and mv88e6xxx) such that the offload is enabled and disabled from the ->port_bridge_{join,leave} methods instead of the old ->port_bridge_tx_fwd_{,un}offload. The only non-trivial change is that mv88e6xxx_map_virtual_bridge_to_pvt() has been moved to avoid a forward declaration, and the mv88e6xxx_reg_lock() calls from inside it have been removed, since locking is now done from mv88e6xxx_port_bridge_{join,leave}. Signed-off-by: Vladimir Oltean Reviewed-by: Alvin Šipraga Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'include/net/dsa.h') diff --git a/include/net/dsa.h b/include/net/dsa.h index 584b3f9462a0..bdf308a5c55e 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -222,6 +222,7 @@ struct dsa_mall_tc_entry { struct dsa_bridge { struct net_device *dev; unsigned int num; + bool tx_fwd_offload; refcount_t refcount; }; @@ -826,12 +827,6 @@ struct dsa_switch_ops { bool *tx_fwd_offload); void (*port_bridge_leave)(struct dsa_switch *ds, int port, struct dsa_bridge bridge); - /* Called right after .port_bridge_join() */ - int (*port_bridge_tx_fwd_offload)(struct dsa_switch *ds, int port, - struct dsa_bridge bridge); - /* Called right before .port_bridge_leave() */ - void (*port_bridge_tx_fwd_unoffload)(struct dsa_switch *ds, int port, - struct dsa_bridge bridge); void (*port_stp_state_set)(struct dsa_switch *ds, int port, u8 state); void (*port_fast_age)(struct dsa_switch *ds, int port); -- cgit v1.2.3