linux-kernel - Re: [PATCH net-next 2/2] net: dsa: mt7530: add support for bridge port isolation

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: <4eaf2bcb-4fad-4211-a48e-079a5c2a6767@arinc9.com>
Date: Sun, 16 Jun 2024 09:52:00 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Matthias Schiffer <mschiffer@...verse-factory.net>,
 Daniel Golle <daniel@...rotopia.org>, DENG Qingfang <dqfext@...il.com>,
 Sean Wang <sean.wang@...iatek.com>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
 Vladimir Oltean <olteanv@...il.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Matthias Brugger <matthias.bgg@...il.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH net-next 2/2] net: dsa: mt7530: add support for bridge
 port isolation

On 15/06/2024 01:21, Matthias Schiffer wrote:
> Remove a pair of ports from the port matrix when both ports have the
> isolated flag set.
> 
> Signed-off-by: Matthias Schiffer <mschiffer@...verse-factory.net>
> ---
>   drivers/net/dsa/mt7530.c | 21 ++++++++++++++++++---
>   drivers/net/dsa/mt7530.h |  1 +
>   2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ecacaefdd694..44939379aba8 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1303,7 +1303,8 @@ mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state)
>   }
>   
>   static void mt7530_update_port_member(struct mt7530_priv *priv, int port,
> -				      const struct net_device *bridge_dev, bool join)
> +				      const struct net_device *bridge_dev,
> +				      bool join)

Run git clang-format on this patch as well please.

>   	__must_hold(&priv->reg_mutex)
>   {
>   	struct dsa_port *dp = dsa_to_port(priv->ds, port), *other_dp;
> @@ -1311,6 +1312,7 @@ static void mt7530_update_port_member(struct mt7530_priv *priv, int port,
>   	struct dsa_port *cpu_dp = dp->cpu_dp;
>   	u32 port_bitmap = BIT(cpu_dp->index);
>   	int other_port;
> +	bool isolated;
>   
>   	dsa_switch_for_each_user_port(other_dp, priv->ds) {
>   		other_port = other_dp->index;
> @@ -1327,7 +1329,9 @@ static void mt7530_update_port_member(struct mt7530_priv *priv, int port,
>   		if (!dsa_port_offloads_bridge_dev(other_dp, bridge_dev))
>   			continue;
>   
> -		if (join) {
> +		isolated = p->isolated && other_p->isolated;
> +
> +		if (join && !isolated) {
>   			other_p->pm |= PCR_MATRIX(BIT(port));
>   			port_bitmap |= BIT(other_port);
>   		} else {

Why must other_p->isolated be true as well? If I understand correctly, when
a user port is isolated, non isolated ports can't communicate with it
whilst the CPU port can. If I were to isolate a port which is the only
isolated one at the moment, the isolated flag would not be true. Therefore,
the isolated port would not be removed from the port matrix of other user
ports. Why not only check for p->isolated?

Arınç