netdev - Re: MT7530 bug, forward broadcast and unknown frames to the correct CPU port

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: <20230429173522.tqd7izelbhr4rvqz@skbuf>
Date:   Sat, 29 Apr 2023 20:35:22 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Arınç ÜNAL <arinc.unal@...nc9.com>
Cc:     DENG Qingfang <dqfext@...il.com>, Greg Ungerer <gerg@...nel.org>,
        Daniel Golle <daniel@...rotopia.org>,
        Richard van Schagen <richard@...terhints.com>,
        Richard van Schagen <vschagen@...com>,
        Frank Wunderlich <frank-w@...lic-files.de>,
        mithat.guner@...ont.com, erkin.bozoglu@...ont.com,
        bartel.eerdekens@...stell8.be, netdev <netdev@...r.kernel.org>
Subject: Re: MT7530 bug, forward broadcast and unknown frames to the correct
 CPU port

On Sat, Apr 29, 2023 at 04:03:57PM +0300, Arınç ÜNAL wrote:
> This is the final diff I'm going to submit to net.
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 4d5c5820e461..cc5fa641b026 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1008,9 +1008,9 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
>  	mt7530_write(priv, MT7530_PVC_P(port),
>  		     PORT_SPEC_TAG);
> -	/* Disable flooding by default */
> -	mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK,
> -		   BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port)));
> +	/* Enable flooding on the CPU port */
> +	mt7530_set(priv, MT7530_MFC, BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) |
> +		   UNU_FFP(BIT(port)));
>  	/* Set CPU port number */
>  	if (priv->id == ID_MT7621)
> @@ -2225,6 +2225,10 @@ mt7530_setup(struct dsa_switch *ds)
>  		/* Disable learning by default on all ports */
>  		mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
> +		/* Disable flooding on all ports */
> +		mt7530_clear(priv, MT7530_MFC, BC_FFP(BIT(i)) | UNM_FFP(BIT(i)) |
> +			     UNU_FFP(BIT(i)));
> +
>  		if (dsa_is_cpu_port(ds, i)) {
>  			ret = mt753x_cpu_port_enable(ds, i);
>  			if (ret)
> @@ -2412,6 +2416,10 @@ mt7531_setup(struct dsa_switch *ds)
>  		mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR);
> +		/* Disable flooding on all ports */
> +		mt7530_clear(priv, MT7530_MFC, BC_FFP(BIT(i)) | UNM_FFP(BIT(i)) |
> +			     UNU_FFP(BIT(i)));
> +
>  		if (dsa_is_cpu_port(ds, i)) {
>  			ret = mt753x_cpu_port_enable(ds, i);
>  			if (ret)

Looks ok, but considering that the register is the same for all ports,
then instead of accessing the hardware one by one for each port, you
could issue a single:

	mt7530_clear(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK);

before the per-port for loop.