diff mbox series

[net-next] net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored

Message ID 20210202233109.1591466-1-olteanv@gmail.com (mailing list archive)
State Accepted
Commit 99b8202b179fc3dbbca69e8af6da660224c9d676
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Vladimir Oltean Feb. 2, 2021, 11:31 p.m. UTC
From: Vladimir Oltean <vladimir.oltean@nxp.com>

The bridge emits VLAN filtering events and quite a few others via
switchdev with orig_dev = br->dev. After the blamed commit, these events
started getting ignored.

The point of the patch was to not offload switchdev objects for ports
that didn't go through dsa_port_bridge_join, because the configuration
is unsupported:
- ports that offload a bonding/team interface go through
  dsa_port_bridge_join when that bonding/team interface is later bridged
  with another switch port or LAG
- ports that don't offload LAG don't get notified of the bridge that is
  on top of that LAG.

Sadly, a check is missing, which is that the orig_dev is equal to the
bridge device. This check is compatible with the original intention,
because ports that don't offload bridging because they use a software
LAG don't have dp->bridge_dev set.

On a semi-related note, we should not offload switchdev objects or
populate dp->bridge_dev if the driver doesn't implement .port_bridge_join
either. However there is no regression associated with that, so it can
be done separately.

Fixes: 5696c8aedfcc ("net: dsa: Don't offload port attributes on standalone ports")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa_priv.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Tobias Waldekranz Feb. 3, 2021, 8:28 a.m. UTC | #1
On Wed, Feb 03, 2021 at 01:31, Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> The bridge emits VLAN filtering events and quite a few others via
> switchdev with orig_dev = br->dev. After the blamed commit, these events
> started getting ignored.
>
> The point of the patch was to not offload switchdev objects for ports
> that didn't go through dsa_port_bridge_join, because the configuration
> is unsupported:
> - ports that offload a bonding/team interface go through
>   dsa_port_bridge_join when that bonding/team interface is later bridged
>   with another switch port or LAG
> - ports that don't offload LAG don't get notified of the bridge that is
>   on top of that LAG.
>
> Sadly, a check is missing, which is that the orig_dev is equal to the
> bridge device. This check is compatible with the original intention,
> because ports that don't offload bridging because they use a software
> LAG don't have dp->bridge_dev set.
>
> On a semi-related note, we should not offload switchdev objects or
> populate dp->bridge_dev if the driver doesn't implement .port_bridge_join
> either. However there is no regression associated with that, so it can
> be done separately.
>
> Fixes: 5696c8aedfcc ("net: dsa: Don't offload port attributes on standalone ports")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Reviewed-by: Tobias Waldekranz <tobias@waldekranz.com>
Tested-by: Tobias Waldekranz <tobias@waldekranz.com>

>  net/dsa/dsa_priv.h | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 2ce46bb87703..1c4ee741b4b8 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -182,7 +182,15 @@ static inline bool dsa_port_offloads_netdev(struct dsa_port *dp,
>  	/* Switchdev offloading can be configured on: */
>  
>  	if (dev == dp->slave)
> -		/* DSA ports directly connected to a bridge. */
> +		/* DSA ports directly connected to a bridge, and event
> +		 * was emitted for the ports themselves.
> +		 */
> +		return true;
> +
> +	if (dp->bridge_dev == dev)
> +		/* DSA ports connected to a bridge, and event was emitted
> +		 * for the bridge.
> +		 */
>  		return true;
>  
>  	if (dp->lag_dev == dev)
> -- 
> 2.25.1
patchwork-bot+netdevbpf@kernel.org Feb. 4, 2021, 4 a.m. UTC | #2
Hello:

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

On Wed,  3 Feb 2021 01:31:09 +0200 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The bridge emits VLAN filtering events and quite a few others via
> switchdev with orig_dev = br->dev. After the blamed commit, these events
> started getting ignored.
> 
> The point of the patch was to not offload switchdev objects for ports
> that didn't go through dsa_port_bridge_join, because the configuration
> is unsupported:
> - ports that offload a bonding/team interface go through
>   dsa_port_bridge_join when that bonding/team interface is later bridged
>   with another switch port or LAG
> - ports that don't offload LAG don't get notified of the bridge that is
>   on top of that LAG.
> 
> [...]

Here is the summary with links:
  - [net-next] net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored
    https://git.kernel.org/netdev/net-next/c/99b8202b179f

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

Patch

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2ce46bb87703..1c4ee741b4b8 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -182,7 +182,15 @@  static inline bool dsa_port_offloads_netdev(struct dsa_port *dp,
 	/* Switchdev offloading can be configured on: */
 
 	if (dev == dp->slave)
-		/* DSA ports directly connected to a bridge. */
+		/* DSA ports directly connected to a bridge, and event
+		 * was emitted for the ports themselves.
+		 */
+		return true;
+
+	if (dp->bridge_dev == dev)
+		/* DSA ports connected to a bridge, and event was emitted
+		 * for the bridge.
+		 */
 		return true;
 
 	if (dp->lag_dev == dev)