netdev - Re: [PATCH net-next] net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <875z395zd2.fsf@waldekranz.com>
Date:   Wed, 03 Feb 2021 09:28:41 +0100
From:   Tobias Waldekranz <tobias@...dekranz.com>
To:     Vladimir Oltean <olteanv@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH net-next] net: dsa: fix SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING getting ignored

On Wed, Feb 03, 2021 at 01:31, Vladimir Oltean <olteanv@...il.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@....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@....com>
> ---

Reviewed-by: Tobias Waldekranz <tobias@...dekranz.com>
Tested-by: Tobias Waldekranz <tobias@...dekranz.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