Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module
@ 2021-07-26 14:25 Vladimir Oltean
  2021-07-27 10:15 ` Anders Roxell
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Vladimir Oltean @ 2021-07-26 14:25 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Naresh Kamboju, Grygorii Strashko, Lars Povlsen, Steen Hegelund,
	UNGLinuxDriver, Florian Fainelli, Andrew Lunn, Vivien Didelot,
	Ido Schimmel, Jiri Pirko, Roopa Prabhu, Nikolay Aleksandrov,
	Linux Kernel Functional Testing

Currently, all drivers depend on the bool CONFIG_NET_SWITCHDEV, but only
the drivers that call some sort of function exported by the bridge, like
br_vlan_enabled() or whatever, have an extra dependency on CONFIG_BRIDGE.

Since the blamed commit, all switchdev drivers have a functional
dependency upon switchdev_bridge_port_{,un}offload(), which is a pair of
functions exported by the bridge module and not by the bridge-independent
part of CONFIG_NET_SWITCHDEV.

Problems appear when we have:

CONFIG_BRIDGE=m
CONFIG_NET_SWITCHDEV=y
CONFIG_TI_CPSW_SWITCHDEV=y

because cpsw, am65_cpsw and sparx5 will then be built-in but they will
call a symbol exported by a loadable module. This is not possible and
will result in the following build error:

drivers/net/ethernet/ti/cpsw_new.o: in function `cpsw_netdevice_event':
drivers/net/ethernet/ti/cpsw_new.c:1520: undefined reference to
					`switchdev_bridge_port_offload'
drivers/net/ethernet/ti/cpsw_new.c:1537: undefined reference to
					`switchdev_bridge_port_unoffload'

As mentioned, the other switchdev drivers don't suffer from this because
switchdev_bridge_port_offload() is not the first symbol exported by the
bridge that they are calling, so they already needed to deal with this
in the same way.

Fixes: 2f5dc00f7a3e ("net: bridge: switchdev: let drivers inform which bridge ports are offloaded")
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
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 insertions(+)

diff --git a/drivers/net/ethernet/microchip/sparx5/Kconfig b/drivers/net/ethernet/microchip/sparx5/Kconfig
index 7bdbb2d09a14..d39ae2a6fb49 100644
--- a/drivers/net/ethernet/microchip/sparx5/Kconfig
+++ b/drivers/net/ethernet/microchip/sparx5/Kconfig
@@ -1,5 +1,6 @@
 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 affcf92cd3aa..7ac8e5ecbe97 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -64,6 +64,7 @@ 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
@@ -109,6 +110,7 @@ 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] net: build all switchdev drivers as modules when the bridge is a module
  2021-07-26 14:25 [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module Vladimir Oltean
@ 2021-07-27 10:15 ` Anders Roxell
  2021-07-27 10:20 ` patchwork-bot+netdevbpf
  2021-08-02 14:47 ` Arnd Bergmann
  2 siblings, 0 replies; 8+ messages in thread
From: Anders Roxell @ 2021-07-27 10:15 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Networking, Jakub Kicinski, David S. Miller, Naresh Kamboju,
	Grygorii Strashko, Lars Povlsen, Steen Hegelund, UNGLinuxDriver,
	Florian Fainelli, Andrew Lunn, Vivien Didelot, Ido Schimmel,
	Jiri Pirko, Roopa Prabhu, Nikolay Aleksandrov,
	Linux Kernel Functional Testing

On Mon, 26 Jul 2021 at 16:26, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> Currently, all drivers depend on the bool CONFIG_NET_SWITCHDEV, but only
> the drivers that call some sort of function exported by the bridge, like
> br_vlan_enabled() or whatever, have an extra dependency on CONFIG_BRIDGE.
>
> Since the blamed commit, all switchdev drivers have a functional
> dependency upon switchdev_bridge_port_{,un}offload(), which is a pair of
> functions exported by the bridge module and not by the bridge-independent
> part of CONFIG_NET_SWITCHDEV.
>
> Problems appear when we have:
>
> CONFIG_BRIDGE=m
> CONFIG_NET_SWITCHDEV=y
> CONFIG_TI_CPSW_SWITCHDEV=y
>
> because cpsw, am65_cpsw and sparx5 will then be built-in but they will
> call a symbol exported by a loadable module. This is not possible and
> will result in the following build error:
>
> drivers/net/ethernet/ti/cpsw_new.o: in function `cpsw_netdevice_event':
> drivers/net/ethernet/ti/cpsw_new.c:1520: undefined reference to
>                                         `switchdev_bridge_port_offload'
> drivers/net/ethernet/ti/cpsw_new.c:1537: undefined reference to
>                                         `switchdev_bridge_port_unoffload'
>
> As mentioned, the other switchdev drivers don't suffer from this because
> switchdev_bridge_port_offload() is not the first symbol exported by the
> bridge that they are calling, so they already needed to deal with this
> in the same way.
>
> Fixes: 2f5dc00f7a3e ("net: bridge: switchdev: let drivers inform which bridge ports are offloaded")
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Thank you for providing this fix.

Tested building omap2plus_defconfig.

Tested-by: Anders Roxell <anders.roxell@linaro.org>

Cheers,
Anders

> ---
>  drivers/net/ethernet/microchip/sparx5/Kconfig | 1 +
>  drivers/net/ethernet/ti/Kconfig               | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/Kconfig b/drivers/net/ethernet/microchip/sparx5/Kconfig
> index 7bdbb2d09a14..d39ae2a6fb49 100644
> --- a/drivers/net/ethernet/microchip/sparx5/Kconfig
> +++ b/drivers/net/ethernet/microchip/sparx5/Kconfig
> @@ -1,5 +1,6 @@
>  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 affcf92cd3aa..7ac8e5ecbe97 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -64,6 +64,7 @@ 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
> @@ -109,6 +110,7 @@ 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] net: build all switchdev drivers as modules when the bridge is a module
  2021-07-26 14:25 [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module Vladimir Oltean
  2021-07-27 10:15 ` Anders Roxell
@ 2021-07-27 10:20 ` patchwork-bot+netdevbpf
  2021-08-02 14:47 ` Arnd Bergmann
  2 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-07-27 10:20 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, kuba, davem, naresh.kamboju, grygorii.strashko,
	lars.povlsen, Steen.Hegelund, UNGLinuxDriver, f.fainelli, andrew,
	vivien.didelot, idosch, jiri, roopa, nikolay, lkft

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Mon, 26 Jul 2021 17:25:36 +0300 you wrote:
> Currently, all drivers depend on the bool CONFIG_NET_SWITCHDEV, but only
> the drivers that call some sort of function exported by the bridge, like
> br_vlan_enabled() or whatever, have an extra dependency on CONFIG_BRIDGE.
> 
> Since the blamed commit, all switchdev drivers have a functional
> dependency upon switchdev_bridge_port_{,un}offload(), which is a pair of
> functions exported by the bridge module and not by the bridge-independent
> part of CONFIG_NET_SWITCHDEV.
> 
> [...]

Here is the summary with links:
  - [net-next] net: build all switchdev drivers as modules when the bridge is a module
    https://git.kernel.org/netdev/net-next/c/b0e81817629a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module
  2021-07-26 14:25 [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module Vladimir Oltean
  2021-07-27 10:15 ` Anders Roxell
  2021-07-27 10:20 ` patchwork-bot+netdevbpf
@ 2021-08-02 14:47 ` Arnd Bergmann
  2021-08-03 11:18   ` Grygorii Strashko
  2 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2021-08-02 14:47 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Networking, Jakub Kicinski, David S. Miller, Naresh Kamboju,
	Grygorii Strashko, Lars Povlsen, Steen Hegelund,
	Microchip Linux Driver Support, Florian Fainelli, Andrew Lunn,
	Vivien Didelot, Ido Schimmel, Jiri Pirko, Roopa Prabhu,
	Nikolay Aleksandrov, Linux Kernel Functional Testing

On Mon, Jul 26, 2021 at 4:28 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> Currently, all drivers depend on the bool CONFIG_NET_SWITCHDEV, but only
> the drivers that call some sort of function exported by the bridge, like
> br_vlan_enabled() or whatever, have an extra dependency on CONFIG_BRIDGE.
>
> Since the blamed commit, all switchdev drivers have a functional
> dependency upon switchdev_bridge_port_{,un}offload(), which is a pair of
> functions exported by the bridge module and not by the bridge-independent
> part of CONFIG_NET_SWITCHDEV.
>
> Problems appear when we have:
>
> CONFIG_BRIDGE=m
> CONFIG_NET_SWITCHDEV=y
> CONFIG_TI_CPSW_SWITCHDEV=y
>
> because cpsw, am65_cpsw and sparx5 will then be built-in but they will
> call a symbol exported by a loadable module. This is not possible and
> will result in the following build error:
>
> drivers/net/ethernet/ti/cpsw_new.o: in function `cpsw_netdevice_event':
> drivers/net/ethernet/ti/cpsw_new.c:1520: undefined reference to
>                                         `switchdev_bridge_port_offload'
> drivers/net/ethernet/ti/cpsw_new.c:1537: undefined reference to
>                                         `switchdev_bridge_port_unoffload'
>
> As mentioned, the other switchdev drivers don't suffer from this because
> switchdev_bridge_port_offload() is not the first symbol exported by the
> bridge that they are calling, so they already needed to deal with this
> in the same way.
>
> Fixes: 2f5dc00f7a3e ("net: bridge: switchdev: let drivers inform which bridge ports are offloaded")
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

I'm still seeing build failures after this patch was applied. I have a fixup
patch that seems to work, but I'm still not sure if that version is complete.

      Arnd

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

* Re: [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module
  2021-08-02 14:47 ` Arnd Bergmann
@ 2021-08-03 11:18   ` Grygorii Strashko
  2021-08-03 11:58     ` Vladimir Oltean
  0 siblings, 1 reply; 8+ messages in thread
From: Grygorii Strashko @ 2021-08-03 11:18 UTC (permalink / raw)
  To: Arnd Bergmann, Vladimir Oltean
  Cc: Networking, Jakub Kicinski, David S. Miller, Naresh Kamboju,
	Lars Povlsen, Steen Hegelund, Microchip Linux Driver Support,
	Florian Fainelli, Andrew Lunn, Vivien Didelot, Ido Schimmel,
	Jiri Pirko, Roopa Prabhu, Nikolay Aleksandrov,
	Linux Kernel Functional Testing, Vignesh Raghavendra,
	Florian Fainelli, Andrew Lunn

Hi All,

On 02/08/2021 17:47, Arnd Bergmann wrote:
> On Mon, Jul 26, 2021 at 4:28 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>>
>> Currently, all drivers depend on the bool CONFIG_NET_SWITCHDEV, but only
>> the drivers that call some sort of function exported by the bridge, like
>> br_vlan_enabled() or whatever, have an extra dependency on CONFIG_BRIDGE.
>>
>> Since the blamed commit, all switchdev drivers have a functional
>> dependency upon switchdev_bridge_port_{,un}offload(), which is a pair of
>> functions exported by the bridge module and not by the bridge-independent
>> part of CONFIG_NET_SWITCHDEV.
>>
>> Problems appear when we have:
>>
>> CONFIG_BRIDGE=m
>> CONFIG_NET_SWITCHDEV=y
>> CONFIG_TI_CPSW_SWITCHDEV=y
>>
>> because cpsw, am65_cpsw and sparx5 will then be built-in but they will
>> call a symbol exported by a loadable module. This is not possible and
>> will result in the following build error:
>>
>> drivers/net/ethernet/ti/cpsw_new.o: in function `cpsw_netdevice_event':
>> drivers/net/ethernet/ti/cpsw_new.c:1520: undefined reference to
>>                                          `switchdev_bridge_port_offload'
>> drivers/net/ethernet/ti/cpsw_new.c:1537: undefined reference to
>>                                          `switchdev_bridge_port_unoffload'
>>
>> As mentioned, the other switchdev drivers don't suffer from this because
>> switchdev_bridge_port_offload() is not the first symbol exported by the
>> bridge that they are calling, so they already needed to deal with this
>> in the same way.
>>
>> Fixes: 2f5dc00f7a3e ("net: bridge: switchdev: let drivers inform which bridge ports are offloaded")
>> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> I'm still seeing build failures after this patch was applied. I have a fixup
> patch that seems to work, but I'm still not sure if that version is complete.

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.
-- 
Best regards,
grygorii

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

* Re: [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module
  2021-08-03 11:18   ` Grygorii Strashko
@ 2021-08-03 11:58     ` Vladimir Oltean
  2021-08-03 12:33       ` Arnd Bergmann
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2021-08-03 11:58 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Arnd Bergmann, Networking, Jakub Kicinski, David S. Miller,
	Naresh Kamboju, Lars Povlsen, Steen Hegelund,
	Microchip Linux Driver Support, Florian Fainelli, Andrew Lunn,
	Vivien Didelot, Ido Schimmel, Jiri Pirko, Roopa Prabhu,
	Nikolay Aleksandrov, Linux Kernel Functional Testing,
	Vignesh Raghavendra

On Tue, Aug 03, 2021 at 02:18:38PM +0300, Grygorii Strashko wrote:
> 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.

I didn't realize it is such a big use case to have the bridge built as
module and cpsw as built-in. I will send a patch that converts the
switchdev_bridge_port_{,un}offload functions to simply emit a blocking
switchdev notifier which the bridge processes (a la SWITCHDEV_FDB_ADD_TO_BRIDGE),
therefore making switchdev and the bridge loosely coupled in order to
keep your setup the way it was before.

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

* Re: [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module
  2021-08-03 11:58     ` Vladimir Oltean
@ 2021-08-03 12:33       ` Arnd Bergmann
  2021-08-03 12:46         ` Grygorii Strashko
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2021-08-03 12:33 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Grygorii Strashko, Networking, Jakub Kicinski, David S. Miller,
	Naresh Kamboju, Lars Povlsen, Steen Hegelund,
	Microchip Linux Driver Support, Florian Fainelli, Andrew Lunn,
	Vivien Didelot, Ido Schimmel, Jiri Pirko, Roopa Prabhu,
	Nikolay Aleksandrov, Linux Kernel Functional Testing,
	Vignesh Raghavendra

On Tue, Aug 3, 2021 at 1:58 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> On Tue, Aug 03, 2021 at 02:18:38PM +0300, Grygorii Strashko wrote:
> > 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.
>
> I didn't realize it is such a big use case to have the bridge built as
> module and cpsw as built-in.

I don't think anybody realistically cares about doing, I was only interested in
correctly expressing what the dependency is.

> I will send a patch that converts the
> switchdev_bridge_port_{,un}offload functions to simply emit a blocking
> switchdev notifier which the bridge processes (a la SWITCHDEV_FDB_ADD_TO_BRIDGE),
> therefore making switchdev and the bridge loosely coupled in order to
> keep your setup the way it was before.

That does sounds like it can avoid future build regressions, and simplify the
Kconfig dependencies, so that would probably be a good solution.

       arnd

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

* Re: [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module
  2021-08-03 12:33       ` Arnd Bergmann
@ 2021-08-03 12:46         ` Grygorii Strashko
  0 siblings, 0 replies; 8+ messages in thread
From: Grygorii Strashko @ 2021-08-03 12:46 UTC (permalink / raw)
  To: Arnd Bergmann, Vladimir Oltean
  Cc: Networking, Jakub Kicinski, David S. Miller, Naresh Kamboju,
	Lars Povlsen, Steen Hegelund, Microchip Linux Driver Support,
	Florian Fainelli, Andrew Lunn, Vivien Didelot, Ido Schimmel,
	Jiri Pirko, Roopa Prabhu, Nikolay Aleksandrov,
	Linux Kernel Functional Testing, Vignesh Raghavendra



On 03/08/2021 15:33, Arnd Bergmann wrote:
> On Tue, Aug 3, 2021 at 1:58 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>>
>> On Tue, Aug 03, 2021 at 02:18:38PM +0300, Grygorii Strashko wrote:
>>> 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.
>>
>> I didn't realize it is such a big use case to have the bridge built as
>> module and cpsw as built-in.
> 
> I don't think anybody realistically cares about doing, I was only interested in
> correctly expressing what the dependency is.
> 
>> I will send a patch that converts the
>> switchdev_bridge_port_{,un}offload functions to simply emit a blocking
>> switchdev notifier which the bridge processes (a la SWITCHDEV_FDB_ADD_TO_BRIDGE),
>> therefore making switchdev and the bridge loosely coupled in order to
>> keep your setup the way it was before.
> 
> That does sounds like it can avoid future build regressions, and simplify the
> Kconfig dependencies, so that would probably be a good solution.

Yes. it sounds good, thank you.
Just a thought - might be good to follow switchdev_attr approach (extendable), but in the opposite direction as, I feel,
more notification dev->bridge might be added in the future.

-- 
Best regards,
grygorii

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

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

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26 14:25 [PATCH net-next] net: build all switchdev drivers as modules when the bridge is a module Vladimir Oltean
2021-07-27 10:15 ` Anders Roxell
2021-07-27 10:20 ` patchwork-bot+netdevbpf
2021-08-02 14:47 ` Arnd Bergmann
2021-08-03 11:18   ` Grygorii Strashko
2021-08-03 11:58     ` Vladimir Oltean
2021-08-03 12:33       ` Arnd Bergmann
2021-08-03 12:46         ` Grygorii Strashko

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).