Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers
@ 2021-08-03 14:36 Vladimir Oltean
  2021-08-03 14:36 ` [PATCH net-next 1/2] net: make switchdev_bridge_port_{,unoffload} loosely coupled with the bridge Vladimir Oltean
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Vladimir Oltean @ 2021-08-03 14:36 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Jiri Pirko,
	Ido Schimmel, Roopa Prabhu, Nikolay Aleksandrov,
	Stephen Hemminger, bridge, Grygorii Strashko, Arnd Bergmann

The introduction of the explicit switchdev bridge port offloading API
has introduced dependency regressions between switchdev drivers and the
bridge, with some drivers where switchdev support was optional before
being now compiled as a module when the bridge is a module, or worse.

This patch makes the switchdev bridge port offload/unoffload events
visible on the blocking notifier call chain, so that the bridge can
indirectly do something when those events happen, without the driver
explicitly calling a symbol exported by the bridge driver.

Vladimir Oltean (2):
  net: make switchdev_bridge_port_{,unoffload} loosely coupled with the
    bridge
  Revert "net: build all switchdev drivers as modules when the bridge is
    a module"

 drivers/net/ethernet/microchip/sparx5/Kconfig |  1 -
 drivers/net/ethernet/ti/Kconfig               |  2 -
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  2 +-
 drivers/net/ethernet/ti/cpsw_new.c            |  2 +-
 include/linux/if_bridge.h                     | 35 -------------
 include/net/switchdev.h                       | 46 +++++++++++++++++
 net/bridge/br.c                               | 51 ++++++++++++++++++-
 net/bridge/br_private.h                       | 30 +++++++++++
 net/bridge/br_switchdev.c                     | 36 ++++---------
 net/switchdev/switchdev.c                     | 48 +++++++++++++++++
 10 files changed, 185 insertions(+), 68 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 1/2] net: make switchdev_bridge_port_{,unoffload} loosely coupled with the bridge
  2021-08-03 14:36 [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Vladimir Oltean
@ 2021-08-03 14:36 ` Vladimir Oltean
  2021-08-03 19:03   ` kernel test robot
  2021-08-03 14:36 ` [PATCH net-next 2/2] Revert "net: build all switchdev drivers as modules when the bridge is a module" Vladimir Oltean
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2021-08-03 14:36 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Jiri Pirko,
	Ido Schimmel, Roopa Prabhu, Nikolay Aleksandrov,
	Stephen Hemminger, bridge, Grygorii Strashko, Arnd Bergmann

With the introduction of explicit offloading API in switchdev in commit
2f5dc00f7a3e ("net: bridge: switchdev: let drivers inform which bridge
ports are offloaded"), we started having Ethernet switch drivers calling
directly into a function exported by net/bridge/br_switchdev.c, which is
a function exported by the bridge driver.

This means that drivers that did not have an explicit dependency on the
bridge before, like cpsw and am65-cpsw, now do - otherwise it is not
possible to call a symbol exported by a driver that can be built as
module unless you are a module too.

There was an attempt to solve the dependency issue in the form of commit
b0e81817629a ("net: build all switchdev drivers as modules when the
bridge is a module"). Grygorii Strashko, however, says about it:

| In my opinion, the problem is a bit bigger here than just fixing the
| build :(
|
| In case, of ^cpsw the switchdev mode is kinda optional and in many
| cases (especially for testing purposes, NFS) the multi-mac mode is
| still preferable mode.
|
| There were no such tight dependency between switchdev drivers and
| bridge core before and switchdev serviced as independent, notification
| based layer between them, so ^cpsw still can be "Y" and bridge can be
| "M". Now for mostly every kernel build configuration the CONFIG_BRIDGE
| will need to be set as "Y", or we will have to update drivers to
| support build with BRIDGE=n and maintain separate builds for
| networking vs non-networking testing.  But is this enough?  Wouldn't
| it cause 'chain reaction' required to add more and more "Y" options
| (like CONFIG_VLAN_8021Q)?
|
| PS. Just to be sure we on the same page - ARM builds will be forced
| (with this patch) to have CONFIG_TI_CPSW_SWITCHDEV=m and so all our
| automation testing will just fail with omap2plus_defconfig.

In the light of this, it would be desirable for some configurations to
avoid dependencies between switchdev drivers and the bridge, and have
the switchdev mode as completely optional within the driver.

Arnd Bergmann also tried to write a patch which better expressed the
build time dependency for Ethernet switch drivers where the switchdev
support is optional, like cpsw/am65-cpsw, and this made the drivers
follow the bridge (compile as module if the bridge is a module) only if
the optional switchdev support in the driver was enabled in the first
place:
https://patchwork.kernel.org/project/netdevbpf/patch/20210802144813.1152762-1-arnd@kernel.org/

but this still did not solve the fact that cpsw and am65-cpsw now must
be built as modules when the bridge is a module - it just expressed
correctly that optional dependency. But the new behavior is an apparent
regression from Grygorii's perspective.

So to support the use case where the Ethernet driver is built-in,
NET_SWITCHDEV (a bool option) is enabled, and the bridge is a module, we
need a framework that can handle the possible absence of the bridge from
the running system, i.e. runtime bloatware as opposed to build-time
bloatware.

Luckily we already have this framework, since switchdev has been using
it extensively. Events from the bridge side are transmitted to the
driver side using notifier chains - this was originally done so that
unrelated drivers could snoop for events emitted by the bridge towards
ports that are implemented by other drivers (think of a switch driver
with LAG offload that listens for switchdev events on a bonding/team
interface that it offloads).

There are also events which are transmitted from the driver side to the
bridge side, which again are modeled using notifiers.
SWITCHDEV_FDB_ADD_TO_BRIDGE is an example of this, and deals with
notifying the bridge that a MAC address has been dynamically learned.
So there is a precedent we can use for modeling the new framework.

The difference compared to SWITCHDEV_FDB_ADD_TO_BRIDGE is that the work
that the bridge needs to do when a port becomes offloaded is blocking in
its nature: replay VLANs, MDBs etc. The calling context is indeed
blocking (we are under rtnl_mutex), but the existing switchdev
notification chain that the bridge is subscribed to is only the atomic
one. So we need to subscribe the bridge to the blocking switchdev
notification chain too.

This patch:
- keeps the driver-side perception of the switchdev_bridge_port_{,un}offload
  unchanged
- moves the implementation of switchdev_bridge_port_{,un}offload from
  the bridge module into the switchdev module.
- makes everybody that is subscribed to the switchdev blocking notifier
  chain "hear" offload & unoffload events
- makes the bridge driver subscribe and handle those events
- moves the bridge driver's handling of those events into 2 new
  functions called br_switchdev_port_{,un}offload. These functions
  contain in fact the core of the logic that was previously in
  switchdev_bridge_port_{,un}offload, just that now we go through an
  extra indirection layer to reach them.

Unlike all the other switchdev notification structures, the structure
used to carry the bridge port information, struct
switchdev_notifier_brport_info, does not contain a "bool handled".
This is because in the current usage pattern, we always know that a
switchdev bridge port offloading event will be handled by the bridge,
because the switchdev_bridge_port_offload() call was initiated by a
NETDEV_CHANGEUPPER event in the first place, where info->upper_dev is a
bridge. So if the bridge wasn't loaded, then the CHANGEUPPER event
couldn't have happened.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c |  2 +-
 drivers/net/ethernet/ti/cpsw_new.c       |  2 +-
 include/linux/if_bridge.h                | 35 ----------------
 include/net/switchdev.h                  | 46 +++++++++++++++++++++
 net/bridge/br.c                          | 51 +++++++++++++++++++++++-
 net/bridge/br_private.h                  | 30 ++++++++++++++
 net/bridge/br_switchdev.c                | 36 +++++------------
 net/switchdev/switchdev.c                | 48 ++++++++++++++++++++++
 8 files changed, 185 insertions(+), 65 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 4f67d1a98c0d..fb5d2ac3f0d2 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -7,7 +7,6 @@
 
 #include <linux/clk.h>
 #include <linux/etherdevice.h>
-#include <linux/if_bridge.h>
 #include <linux/if_vlan.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -28,6 +27,7 @@
 #include <linux/sys_soc.h>
 #include <linux/dma/ti-cppi5.h>
 #include <linux/dma/k3-udma-glue.h>
+#include <net/switchdev.h>
 
 #include "cpsw_ale.h"
 #include "cpsw_sl.h"
diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index b4f55ff4e84f..ae167223e87f 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -11,7 +11,6 @@
 #include <linux/module.h>
 #include <linux/irqreturn.h>
 #include <linux/interrupt.h>
-#include <linux/if_bridge.h>
 #include <linux/if_ether.h>
 #include <linux/etherdevice.h>
 #include <linux/net_tstamp.h>
@@ -29,6 +28,7 @@
 #include <linux/kmemleak.h>
 #include <linux/sys_soc.h>
 
+#include <net/switchdev.h>
 #include <net/page_pool.h>
 #include <net/pkt_cls.h>
 #include <net/devlink.h>
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 21daed10322e..509e18c7e740 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -190,39 +190,4 @@ static inline clock_t br_get_ageing_time(const struct net_device *br_dev)
 }
 #endif
 
-#if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_NET_SWITCHDEV)
-
-int switchdev_bridge_port_offload(struct net_device *brport_dev,
-				  struct net_device *dev, const void *ctx,
-				  struct notifier_block *atomic_nb,
-				  struct notifier_block *blocking_nb,
-				  bool tx_fwd_offload,
-				  struct netlink_ext_ack *extack);
-void switchdev_bridge_port_unoffload(struct net_device *brport_dev,
-				     const void *ctx,
-				     struct notifier_block *atomic_nb,
-				     struct notifier_block *blocking_nb);
-
-#else
-
-static inline int
-switchdev_bridge_port_offload(struct net_device *brport_dev,
-			      struct net_device *dev, const void *ctx,
-			      struct notifier_block *atomic_nb,
-			      struct notifier_block *blocking_nb,
-			      bool tx_fwd_offload,
-			      struct netlink_ext_ack *extack)
-{
-	return -EINVAL;
-}
-
-static inline void
-switchdev_bridge_port_unoffload(struct net_device *brport_dev,
-				const void *ctx,
-				struct notifier_block *atomic_nb,
-				struct notifier_block *blocking_nb)
-{
-}
-#endif
-
 #endif
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 66468ff8cc0a..60d806b6a5ae 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -180,6 +180,14 @@ struct switchdev_obj_in_state_mrp {
 
 typedef int switchdev_obj_dump_cb_t(struct switchdev_obj *obj);
 
+struct switchdev_brport {
+	struct net_device *dev;
+	const void *ctx;
+	struct notifier_block *atomic_nb;
+	struct notifier_block *blocking_nb;
+	bool tx_fwd_offload;
+};
+
 enum switchdev_notifier_type {
 	SWITCHDEV_FDB_ADD_TO_BRIDGE = 1,
 	SWITCHDEV_FDB_DEL_TO_BRIDGE,
@@ -197,6 +205,9 @@ enum switchdev_notifier_type {
 	SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE,
 	SWITCHDEV_VXLAN_FDB_DEL_TO_DEVICE,
 	SWITCHDEV_VXLAN_FDB_OFFLOADED,
+
+	SWITCHDEV_BRPORT_OFFLOADED,
+	SWITCHDEV_BRPORT_UNOFFLOADED,
 };
 
 struct switchdev_notifier_info {
@@ -226,6 +237,11 @@ struct switchdev_notifier_port_attr_info {
 	bool handled;
 };
 
+struct switchdev_notifier_brport_info {
+	struct switchdev_notifier_info info; /* must be first */
+	const struct switchdev_brport brport;
+};
+
 static inline struct net_device *
 switchdev_notifier_info_to_dev(const struct switchdev_notifier_info *info)
 {
@@ -246,6 +262,17 @@ switchdev_fdb_is_dynamically_learned(const struct switchdev_notifier_fdb_info *f
 
 #ifdef CONFIG_NET_SWITCHDEV
 
+int switchdev_bridge_port_offload(struct net_device *brport_dev,
+				  struct net_device *dev, const void *ctx,
+				  struct notifier_block *atomic_nb,
+				  struct notifier_block *blocking_nb,
+				  bool tx_fwd_offload,
+				  struct netlink_ext_ack *extack);
+void switchdev_bridge_port_unoffload(struct net_device *brport_dev,
+				     const void *ctx,
+				     struct notifier_block *atomic_nb,
+				     struct notifier_block *blocking_nb);
+
 void switchdev_deferred_process(void);
 int switchdev_port_attr_set(struct net_device *dev,
 			    const struct switchdev_attr *attr,
@@ -316,6 +343,25 @@ int switchdev_handle_port_attr_set(struct net_device *dev,
 				      struct netlink_ext_ack *extack));
 #else
 
+static inline int
+switchdev_bridge_port_offload(struct net_device *brport_dev,
+			      struct net_device *dev, const void *ctx,
+			      struct notifier_block *atomic_nb,
+			      struct notifier_block *blocking_nb,
+			      bool tx_fwd_offload,
+			      struct netlink_ext_ack *extack)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline void
+switchdev_bridge_port_unoffload(struct net_device *brport_dev,
+				const void *ctx,
+				struct notifier_block *atomic_nb,
+				struct notifier_block *blocking_nb)
+{
+}
+
 static inline void switchdev_deferred_process(void)
 {
 }
diff --git a/net/bridge/br.c b/net/bridge/br.c
index 8fb5dca5f8e0..d3a32c6813e0 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -201,6 +201,48 @@ static struct notifier_block br_switchdev_notifier = {
 	.notifier_call = br_switchdev_event,
 };
 
+/* called under rtnl_mutex */
+static int br_switchdev_blocking_event(struct notifier_block *nb,
+				       unsigned long event, void *ptr)
+{
+	struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr);
+	struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+	struct switchdev_notifier_brport_info *brport_info;
+	const struct switchdev_brport *b;
+	struct net_bridge_port *p;
+	int err = NOTIFY_DONE;
+
+	p = br_port_get_rtnl(dev);
+	if (!p)
+		goto out;
+
+	switch (event) {
+	case SWITCHDEV_BRPORT_OFFLOADED:
+		brport_info = ptr;
+		b = &brport_info->brport;
+
+		err = br_switchdev_port_offload(p, b->dev, b->ctx,
+						b->atomic_nb, b->blocking_nb,
+						b->tx_fwd_offload, extack);
+		err = notifier_from_errno(err);
+		break;
+	case SWITCHDEV_BRPORT_UNOFFLOADED:
+		brport_info = ptr;
+		b = &brport_info->brport;
+
+		br_switchdev_port_unoffload(p, b->ctx, b->atomic_nb,
+					    b->blocking_nb);
+		break;
+	}
+
+out:
+	return err;
+}
+
+static struct notifier_block br_switchdev_blocking_notifier = {
+	.notifier_call = br_switchdev_blocking_event,
+};
+
 /* br_boolopt_toggle - change user-controlled boolean option
  *
  * @br: bridge device
@@ -355,10 +397,14 @@ static int __init br_init(void)
 	if (err)
 		goto err_out4;
 
-	err = br_netlink_init();
+	err = register_switchdev_blocking_notifier(&br_switchdev_blocking_notifier);
 	if (err)
 		goto err_out5;
 
+	err = br_netlink_init();
+	if (err)
+		goto err_out6;
+
 	brioctl_set(br_ioctl_stub);
 
 #if IS_ENABLED(CONFIG_ATM_LANE)
@@ -373,6 +419,8 @@ static int __init br_init(void)
 
 	return 0;
 
+err_out6:
+	unregister_switchdev_blocking_notifier(&br_switchdev_blocking_notifier);
 err_out5:
 	unregister_switchdev_notifier(&br_switchdev_notifier);
 err_out4:
@@ -392,6 +440,7 @@ static void __exit br_deinit(void)
 {
 	stp_proto_unregister(&br_stp_proto);
 	br_netlink_fini();
+	unregister_switchdev_blocking_notifier(&br_switchdev_blocking_notifier);
 	unregister_switchdev_notifier(&br_switchdev_notifier);
 	unregister_netdevice_notifier(&br_device_notifier);
 	brioctl_set(NULL);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index c939631428b9..c9bb710cf493 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1880,6 +1880,17 @@ static inline void br_sysfs_delbr(struct net_device *dev) { return; }
 
 /* br_switchdev.c */
 #ifdef CONFIG_NET_SWITCHDEV
+int br_switchdev_port_offload(struct net_bridge_port *p,
+			      struct net_device *dev, const void *ctx,
+			      struct notifier_block *atomic_nb,
+			      struct notifier_block *blocking_nb,
+			      bool tx_fwd_offload,
+			      struct netlink_ext_ack *extack);
+
+void br_switchdev_port_unoffload(struct net_bridge_port *p, const void *ctx,
+				 struct notifier_block *atomic_nb,
+				 struct notifier_block *blocking_nb);
+
 bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb);
 
 void br_switchdev_frame_set_offload_fwd_mark(struct sk_buff *skb);
@@ -1908,6 +1919,25 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb)
 	skb->offload_fwd_mark = 0;
 }
 #else
+static inline int
+br_switchdev_port_offload(struct net_bridge_port *p,
+			  struct net_device *dev, const void *ctx,
+			  struct notifier_block *atomic_nb,
+			  struct notifier_block *blocking_nb,
+			  bool tx_fwd_offload,
+			  struct netlink_ext_ack *extack)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline void
+br_switchdev_port_unoffload(struct net_bridge_port *p, const void *ctx,
+			    struct notifier_block *atomic_nb,
+			    struct notifier_block *blocking_nb)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb)
 {
 	return false;
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index a3cd79e3e81c..97129f09a5bf 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -315,23 +315,16 @@ static void nbp_switchdev_unsync_objs(struct net_bridge_port *p,
 /* Let the bridge know that this port is offloaded, so that it can assign a
  * switchdev hardware domain to it.
  */
-int switchdev_bridge_port_offload(struct net_device *brport_dev,
-				  struct net_device *dev, const void *ctx,
-				  struct notifier_block *atomic_nb,
-				  struct notifier_block *blocking_nb,
-				  bool tx_fwd_offload,
-				  struct netlink_ext_ack *extack)
+int br_switchdev_port_offload(struct net_bridge_port *p,
+			      struct net_device *dev, const void *ctx,
+			      struct notifier_block *atomic_nb,
+			      struct notifier_block *blocking_nb,
+			      bool tx_fwd_offload,
+			      struct netlink_ext_ack *extack)
 {
 	struct netdev_phys_item_id ppid;
-	struct net_bridge_port *p;
 	int err;
 
-	ASSERT_RTNL();
-
-	p = br_port_get_rtnl(brport_dev);
-	if (!p)
-		return -ENODEV;
-
 	err = dev_get_port_parent_id(dev, &ppid, false);
 	if (err)
 		return err;
@@ -351,23 +344,12 @@ int switchdev_bridge_port_offload(struct net_device *brport_dev,
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(switchdev_bridge_port_offload);
 
-void switchdev_bridge_port_unoffload(struct net_device *brport_dev,
-				     const void *ctx,
-				     struct notifier_block *atomic_nb,
-				     struct notifier_block *blocking_nb)
+void br_switchdev_port_unoffload(struct net_bridge_port *p, const void *ctx,
+				 struct notifier_block *atomic_nb,
+				 struct notifier_block *blocking_nb)
 {
-	struct net_bridge_port *p;
-
-	ASSERT_RTNL();
-
-	p = br_port_get_rtnl(brport_dev);
-	if (!p)
-		return;
-
 	nbp_switchdev_unsync_objs(p, ctx, atomic_nb, blocking_nb);
 
 	nbp_switchdev_del(p);
 }
-EXPORT_SYMBOL_GPL(switchdev_bridge_port_unoffload);
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 0ae3478561f4..0b2c18efc079 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -809,3 +809,51 @@ int switchdev_handle_port_attr_set(struct net_device *dev,
 	return err;
 }
 EXPORT_SYMBOL_GPL(switchdev_handle_port_attr_set);
+
+int switchdev_bridge_port_offload(struct net_device *brport_dev,
+				  struct net_device *dev, const void *ctx,
+				  struct notifier_block *atomic_nb,
+				  struct notifier_block *blocking_nb,
+				  bool tx_fwd_offload,
+				  struct netlink_ext_ack *extack)
+{
+	struct switchdev_notifier_brport_info brport_info = {
+		.brport = {
+			.dev = dev,
+			.ctx = ctx,
+			.atomic_nb = atomic_nb,
+			.blocking_nb = blocking_nb,
+			.tx_fwd_offload = tx_fwd_offload,
+		},
+	};
+	int err;
+
+	ASSERT_RTNL();
+
+	err = call_switchdev_blocking_notifiers(SWITCHDEV_BRPORT_OFFLOADED,
+						brport_dev, &brport_info.info,
+						extack);
+	return notifier_to_errno(err);
+}
+EXPORT_SYMBOL_GPL(switchdev_bridge_port_offload);
+
+void switchdev_bridge_port_unoffload(struct net_device *brport_dev,
+				     const void *ctx,
+				     struct notifier_block *atomic_nb,
+				     struct notifier_block *blocking_nb)
+{
+	struct switchdev_notifier_brport_info brport_info = {
+		.brport = {
+			.ctx = ctx,
+			.atomic_nb = atomic_nb,
+			.blocking_nb = blocking_nb,
+		},
+	};
+
+	ASSERT_RTNL();
+
+	call_switchdev_blocking_notifiers(SWITCHDEV_BRPORT_UNOFFLOADED,
+					  brport_dev, &brport_info.info,
+					  NULL);
+}
+EXPORT_SYMBOL_GPL(switchdev_bridge_port_unoffload);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 2/2] Revert "net: build all switchdev drivers as modules when the bridge is a module"
  2021-08-03 14:36 [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Vladimir Oltean
  2021-08-03 14:36 ` [PATCH net-next 1/2] net: make switchdev_bridge_port_{,unoffload} loosely coupled with the bridge Vladimir Oltean
@ 2021-08-03 14:36 ` Vladimir Oltean
  2021-08-03 17:11 ` [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Grygorii Strashko
  2021-08-03 20:44 ` Vladimir Oltean
  3 siblings, 0 replies; 8+ messages in thread
From: Vladimir Oltean @ 2021-08-03 14:36 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Jiri Pirko,
	Ido Schimmel, Roopa Prabhu, Nikolay Aleksandrov,
	Stephen Hemminger, bridge, Grygorii Strashko, Arnd Bergmann

This reverts commit b0e81817629a496854ff1799f6cbd89597db65fd. Explicit
driver dependency on the bridge is no longer needed since
switchdev_bridge_port_{,un}offload() is no longer implemented by the
bridge driver but by switchdev.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/microchip/sparx5/Kconfig | 1 -
 drivers/net/ethernet/ti/Kconfig               | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/Kconfig b/drivers/net/ethernet/microchip/sparx5/Kconfig
index d39ae2a6fb49..7bdbb2d09a14 100644
--- a/drivers/net/ethernet/microchip/sparx5/Kconfig
+++ b/drivers/net/ethernet/microchip/sparx5/Kconfig
@@ -1,6 +1,5 @@
 config SPARX5_SWITCH
 	tristate "Sparx5 switch driver"
-	depends on BRIDGE || BRIDGE=n
 	depends on NET_SWITCHDEV
 	depends on HAS_IOMEM
 	depends on OF
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 7ac8e5ecbe97..affcf92cd3aa 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -64,7 +64,6 @@ config TI_CPSW
 config TI_CPSW_SWITCHDEV
 	tristate "TI CPSW Switch Support with switchdev"
 	depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
-	depends on BRIDGE || BRIDGE=n
 	depends on NET_SWITCHDEV
 	depends on TI_CPTS || !TI_CPTS
 	select PAGE_POOL
@@ -110,7 +109,6 @@ config TI_K3_AM65_CPSW_NUSS
 config TI_K3_AM65_CPSW_SWITCHDEV
 	bool "TI K3 AM654x/J721E CPSW Switch mode support"
 	depends on TI_K3_AM65_CPSW_NUSS
-	depends on BRIDGE || BRIDGE=n
 	depends on NET_SWITCHDEV
 	help
 	 This enables switchdev support for TI K3 CPSWxG Ethernet
-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers
  2021-08-03 14:36 [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Vladimir Oltean
  2021-08-03 14:36 ` [PATCH net-next 1/2] net: make switchdev_bridge_port_{,unoffload} loosely coupled with the bridge Vladimir Oltean
  2021-08-03 14:36 ` [PATCH net-next 2/2] Revert "net: build all switchdev drivers as modules when the bridge is a module" Vladimir Oltean
@ 2021-08-03 17:11 ` Grygorii Strashko
  2021-08-03 18:15   ` Vladimir Oltean
  2021-08-03 20:44 ` Vladimir Oltean
  3 siblings, 1 reply; 8+ messages in thread
From: Grygorii Strashko @ 2021-08-03 17:11 UTC (permalink / raw)
  To: Vladimir Oltean, netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Jiri Pirko,
	Ido Schimmel, Roopa Prabhu, Nikolay Aleksandrov,
	Stephen Hemminger, bridge, Arnd Bergmann



On 03/08/2021 17:36, Vladimir Oltean wrote:
> The introduction of the explicit switchdev bridge port offloading API
> has introduced dependency regressions between switchdev drivers and the
> bridge, with some drivers where switchdev support was optional before
> being now compiled as a module when the bridge is a module, or worse.
> 
> This patch makes the switchdev bridge port offload/unoffload events
> visible on the blocking notifier call chain, so that the bridge can
> indirectly do something when those events happen, without the driver
> explicitly calling a symbol exported by the bridge driver.
> 
> Vladimir Oltean (2):
>    net: make switchdev_bridge_port_{,unoffload} loosely coupled with the
>      bridge
>    Revert "net: build all switchdev drivers as modules when the bridge is
>      a module"
> 
>   drivers/net/ethernet/microchip/sparx5/Kconfig |  1 -
>   drivers/net/ethernet/ti/Kconfig               |  2 -
>   drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  2 +-
>   drivers/net/ethernet/ti/cpsw_new.c            |  2 +-
>   include/linux/if_bridge.h                     | 35 -------------
>   include/net/switchdev.h                       | 46 +++++++++++++++++
>   net/bridge/br.c                               | 51 ++++++++++++++++++-
>   net/bridge/br_private.h                       | 30 +++++++++++
>   net/bridge/br_switchdev.c                     | 36 ++++---------
>   net/switchdev/switchdev.c                     | 48 +++++++++++++++++
>   10 files changed, 185 insertions(+), 68 deletions(-)
> 

I've tested builds, but was not able to test bridge itself on TI am57x platform.

1) See warning
[    3.958496] ------------[ cut here ]------------
[    3.963165] WARNING: CPU: 0 PID: 1 at lib/refcount.c:25 fib_create_info+0xae8/0xbd4
[    3.970855] refcount_t: addition on 0; use-after-free.
[    3.976043] Modules linked in: autofs4
[    3.979827] CPU: 0 PID: 1 Comm: systemd Not tainted 5.14.0-rc4-next-20210802-00002-g5003e4ac441d-dirty #5
[    3.989440] Hardware name: Generic DRA72X (Flattened Device Tree)
[    3.995574] [<c0111098>] (unwind_backtrace) from [<c010b834>] (show_stack+0x10/0x14)
[    4.003356] [<c010b834>] (show_stack) from [<c09da808>] (dump_stack_lvl+0x40/0x4c)
[    4.010986] [<c09da808>] (dump_stack_lvl) from [<c0137b44>] (__warn+0xd8/0x100)
[    4.018341] [<c0137b44>] (__warn) from [<c09d6368>] (warn_slowpath_fmt+0x94/0xbc)
[    4.025848] [<c09d6368>] (warn_slowpath_fmt) from [<c08f68d4>] (fib_create_info+0xae8/0xbd4)
[    4.034332] [<c08f68d4>] (fib_create_info) from [<c08f99c4>] (fib_table_insert+0x5c/0x604¢·†AËÕìÍ_¡

2) see warnings and "ip link add name br0 type bridge" just stuck
[  158.032135] unregister_netdevice: waiting for lo to become free. Usage count = 3

It might not be related to this series.


-- 
Best regards,
grygorii

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers
  2021-08-03 17:11 ` [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Grygorii Strashko
@ 2021-08-03 18:15   ` Vladimir Oltean
  2021-08-03 19:18     ` Grygorii Strashko
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2021-08-03 18:15 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: netdev, Jakub Kicinski, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot, Jiri Pirko, Ido Schimmel,
	Roopa Prabhu, Nikolay Aleksandrov, Stephen Hemminger, bridge,
	Arnd Bergmann

On Tue, Aug 03, 2021 at 08:11:56PM +0300, Grygorii Strashko wrote:
> I've tested builds, but was not able to test bridge itself on TI am57x platform.
> 
> 1) See warning
> [    3.958496] ------------[ cut here ]------------
> [    3.963165] WARNING: CPU: 0 PID: 1 at lib/refcount.c:25 fib_create_info+0xae8/0xbd4
> [    3.970855] refcount_t: addition on 0; use-after-free.
> [    3.976043] Modules linked in: autofs4
> [    3.979827] CPU: 0 PID: 1 Comm: systemd Not tainted 5.14.0-rc4-next-20210802-00002-g5003e4ac441d-dirty #5
> [    3.989440] Hardware name: Generic DRA72X (Flattened Device Tree)
> [    3.995574] [<c0111098>] (unwind_backtrace) from [<c010b834>] (show_stack+0x10/0x14)
> [    4.003356] [<c010b834>] (show_stack) from [<c09da808>] (dump_stack_lvl+0x40/0x4c)
> [    4.010986] [<c09da808>] (dump_stack_lvl) from [<c0137b44>] (__warn+0xd8/0x100)
> [    4.018341] [<c0137b44>] (__warn) from [<c09d6368>] (warn_slowpath_fmt+0x94/0xbc)
> [    4.025848] [<c09d6368>] (warn_slowpath_fmt) from [<c08f68d4>] (fib_create_info+0xae8/0xbd4)
> [    4.034332] [<c08f68d4>] (fib_create_info) from [<c08f99c4>] (fib_table_insert+0x5c/0x604¢·†AËÕìÍ_¡
> 
> 2) see warnings and "ip link add name br0 type bridge" just stuck
> [  158.032135] unregister_netdevice: waiting for lo to become free. Usage count = 3
> 
> It might not be related to this series.

100% not me.

See if you have David Ahern's bug fix, and if you do, try to see what
other refcount conversion patches Yajun Deng did, revert them one by one
and see if any one fixes the issue:
https://patchwork.kernel.org/project/netdevbpf/patch/20210802160221.27263-1-dsahern@kernel.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 1/2] net: make switchdev_bridge_port_{,unoffload} loosely coupled with the bridge
  2021-08-03 14:36 ` [PATCH net-next 1/2] net: make switchdev_bridge_port_{,unoffload} loosely coupled with the bridge Vladimir Oltean
@ 2021-08-03 19:03   ` kernel test robot
  0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-08-03 19:03 UTC (permalink / raw)
  To: Vladimir Oltean, netdev, Jakub Kicinski, David S. Miller
  Cc: clang-built-linux, kbuild-all, Florian Fainelli, Andrew Lunn,
	Vivien Didelot, Jiri Pirko, Ido Schimmel, Roopa Prabhu,
	Nikolay Aleksandrov

[-- Attachment #1: Type: text/plain, Size: 18297 bytes --]

Hi Vladimir,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Vladimir-Oltean/Convert-switchdev_bridge_port_-un-offload-to-notifiers/20210803-223837
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c8f6c77d06fe6147d07cb0e4952db008f72767cb
config: s390-randconfig-r014-20210803 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4f71f59bf3d9914188a11d0c41bedbb339d36ff5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/9be67f3f310145283841b1483fd5323e5d88132f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vladimir-Oltean/Convert-switchdev_bridge_port_-un-offload-to-notifiers/20210803-223837
        git checkout 9be67f3f310145283841b1483fd5323e5d88132f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from net/bridge/br.c:12:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from net/bridge/br.c:12:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from net/bridge/br.c:12:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   In file included from net/bridge/br.c:20:
>> net/bridge/br_private.h:1938:2: error: void function 'br_switchdev_port_unoffload' should not return a value [-Wreturn-type]
           return -EOPNOTSUPP;
           ^      ~~~~~~~~~~~
   12 warnings and 1 error generated.
--
   In file included from net/bridge/br_fdb.c:15:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from net/bridge/br_fdb.c:15:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from net/bridge/br_fdb.c:15:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   In file included from net/bridge/br_fdb.c:24:
   In file included from include/trace/events/bridge.h:10:
>> include/trace/events/../../../net/bridge/br_private.h:1938:2: error: void function 'br_switchdev_port_unoffload' should not return a value [-Wreturn-type]
           return -EOPNOTSUPP;
           ^      ~~~~~~~~~~~
   12 warnings and 1 error generated.
--
   In file included from net/core/net-traces.c:8:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from net/core/net-traces.c:8:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from net/core/net-traces.c:8:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   In file included from net/core/net-traces.c:39:
   In file included from include/trace/events/bridge.h:10:
>> include/trace/events/../../../net/bridge/br_private.h:1938:2: error: void function 'br_switchdev_port_unoffload' should not return a value [-Wreturn-type]
           return -EOPNOTSUPP;
           ^      ~~~~~~~~~~~
   In file included from net/core/net-traces.c:50:
   In file included from include/trace/events/neigh.h:255:
   In file included from include/trace/define_trace.h:102:
   In file included from include/trace/trace_events.h:738:
   include/trace/events/neigh.h:42:20: warning: variable 'pin6' set but not used [-Wunused-but-set-variable]
                   struct in6_addr *pin6;
                                    ^
   13 warnings and 1 error generated.


vim +/br_switchdev_port_unoffload +1938 net/bridge/br_private.h

  1932	
  1933	static inline void
  1934	br_switchdev_port_unoffload(struct net_bridge_port *p, const void *ctx,
  1935				    struct notifier_block *atomic_nb,
  1936				    struct notifier_block *blocking_nb)
  1937	{
> 1938		return -EOPNOTSUPP;
  1939	}
  1940	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15752 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers
  2021-08-03 18:15   ` Vladimir Oltean
@ 2021-08-03 19:18     ` Grygorii Strashko
  0 siblings, 0 replies; 8+ messages in thread
From: Grygorii Strashko @ 2021-08-03 19:18 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Jakub Kicinski, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot, Jiri Pirko, Ido Schimmel,
	Roopa Prabhu, Nikolay Aleksandrov, Stephen Hemminger, bridge,
	Arnd Bergmann



On 03/08/2021 21:15, Vladimir Oltean wrote:
> On Tue, Aug 03, 2021 at 08:11:56PM +0300, Grygorii Strashko wrote:
>> I've tested builds, but was not able to test bridge itself on TI am57x platform.
>>
>> 1) See warning
>> [    3.958496] ------------[ cut here ]------------
>> [    3.963165] WARNING: CPU: 0 PID: 1 at lib/refcount.c:25 fib_create_info+0xae8/0xbd4
>> [    3.970855] refcount_t: addition on 0; use-after-free.
>> [    3.976043] Modules linked in: autofs4
>> [    3.979827] CPU: 0 PID: 1 Comm: systemd Not tainted 5.14.0-rc4-next-20210802-00002-g5003e4ac441d-dirty #5
>> [    3.989440] Hardware name: Generic DRA72X (Flattened Device Tree)
>> [    3.995574] [<c0111098>] (unwind_backtrace) from [<c010b834>] (show_stack+0x10/0x14)
>> [    4.003356] [<c010b834>] (show_stack) from [<c09da808>] (dump_stack_lvl+0x40/0x4c)
>> [    4.010986] [<c09da808>] (dump_stack_lvl) from [<c0137b44>] (__warn+0xd8/0x100)
>> [    4.018341] [<c0137b44>] (__warn) from [<c09d6368>] (warn_slowpath_fmt+0x94/0xbc)
>> [    4.025848] [<c09d6368>] (warn_slowpath_fmt) from [<c08f68d4>] (fib_create_info+0xae8/0xbd4)
>> [    4.034332] [<c08f68d4>] (fib_create_info) from [<c08f99c4>] (fib_table_insert+0x5c/0x604¢·†AËÕìÍ_¡
>>
>> 2) see warnings and "ip link add name br0 type bridge" just stuck
>> [  158.032135] unregister_netdevice: waiting for lo to become free. Usage count = 3
>>
>> It might not be related to this series.
> 
> 100% not me.
> 
> See if you have David Ahern's bug fix, and if you do, try to see what
> other refcount conversion patches Yajun Deng did, revert them one by one
> and see if any one fixes the issue:
> https://patchwork.kernel.org/project/netdevbpf/patch/20210802160221.27263-1-dsahern@kernel.org/
> 

Yep. That's it.
build ok,
   CONFIG_TI_CPSW_SWITCHDEV=y
   CONFIG_BRIDGE=m

basic bridge with ping - ok.

Tested-by: Grygorii Strashko <grygorii.strashko@ti.com>

-- 
Best regards,
grygorii

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers
  2021-08-03 14:36 [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Vladimir Oltean
                   ` (2 preceding siblings ...)
  2021-08-03 17:11 ` [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Grygorii Strashko
@ 2021-08-03 20:44 ` Vladimir Oltean
  3 siblings, 0 replies; 8+ messages in thread
From: Vladimir Oltean @ 2021-08-03 20:44 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Jiri Pirko,
	Ido Schimmel, Roopa Prabhu, Nikolay Aleksandrov,
	Stephen Hemminger, bridge, Grygorii Strashko, Arnd Bergmann

Obsoleted by v2:
https://patchwork.kernel.org/project/netdevbpf/cover/20210803203409.1274807-1-vladimir.oltean@nxp.com/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-08-03 20:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03 14:36 [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Vladimir Oltean
2021-08-03 14:36 ` [PATCH net-next 1/2] net: make switchdev_bridge_port_{,unoffload} loosely coupled with the bridge Vladimir Oltean
2021-08-03 19:03   ` kernel test robot
2021-08-03 14:36 ` [PATCH net-next 2/2] Revert "net: build all switchdev drivers as modules when the bridge is a module" Vladimir Oltean
2021-08-03 17:11 ` [PATCH net-next 0/2] Convert switchdev_bridge_port_{,un}offload to notifiers Grygorii Strashko
2021-08-03 18:15   ` Vladimir Oltean
2021-08-03 19:18     ` Grygorii Strashko
2021-08-03 20:44 ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).