linux-kernel - Re: [net-next PATCH v2 1/4] drivers: net: dsa: qca8k: drop MTU tracking from qca8k

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]

Date:   Thu, 14 Apr 2022 16:48:18 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Ansuel Smith <ansuelsmth@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH v2 1/4] drivers: net: dsa: qca8k: drop MTU
 tracking from qca8k_priv

On Tue, Apr 12, 2022 at 07:30:16PM +0200, Ansuel Smith wrote:
> DSA set the CPU port based on the largest MTU of all the slave ports.
> Based on this we can drop the MTU array from qca8k_priv and set the
> port_change_mtu logic on DSA changing MTU of the CPU port as the switch
> have a global MTU settingfor each port.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@...il.com>

>  drivers/net/dsa/qca8k.c | 25 ++++++++-----------------
>  drivers/net/dsa/qca8k.h |  1 -
>  2 files changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index d3ed0a7f8077..820eeab19873 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -2367,16 +2367,17 @@ static int
>  qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
>  {
>  	struct qca8k_priv *priv = ds->priv;
> -	int i, mtu = 0;
>  
> -	priv->port_mtu[port] = new_mtu;
> -
> -	for (i = 0; i < QCA8K_NUM_PORTS; i++)
> -		if (priv->port_mtu[i] > mtu)
> -			mtu = priv->port_mtu[i];
> +	/* We have only have a general MTU setting.
> +	 * DSA always set the CPU port's MTU to the largest MTU of the salve ports.

s/salve/slave/

Also watch for the 80 characters limit.

> +	 * Setting MTU just for the CPU port is sufficient to correctly set a
> +	 * value for every port.
> +	 */
> +	if (!dsa_is_cpu_port(ds, port))
> +		return 0;
>  
>  	/* Include L2 header / FCS length */
> -	return qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN);
> +	return qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, new_mtu + ETH_HLEN + ETH_FCS_LEN);
>  }
>  
>  static int
> @@ -3033,16 +3034,6 @@ qca8k_setup(struct dsa_switch *ds)
>  				  QCA8K_PORT_HOL_CTRL1_WRED_EN,
>  				  mask);
>  		}
> -
> -		/* Set initial MTU for every port.
> -		 * We have only have a general MTU setting. So track
> -		 * every port and set the max across all port.
> -		 * Set per port MTU to 1500 as the MTU change function
> -		 * will add the overhead and if its set to 1518 then it
> -		 * will apply the overhead again and we will end up with
> -		 * MTU of 1536 instead of 1518
> -		 */
> -		priv->port_mtu[i] = ETH_DATA_LEN;
>  	}
>  
>  	/* Special GLOBAL_FC_THRESH value are needed for ar8327 switch */
> diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
> index f375627174c8..562d75997e55 100644
> --- a/drivers/net/dsa/qca8k.h
> +++ b/drivers/net/dsa/qca8k.h
> @@ -398,7 +398,6 @@ struct qca8k_priv {
>  	struct device *dev;
>  	struct dsa_switch_ops ops;
>  	struct gpio_desc *reset_gpio;
> -	unsigned int port_mtu[QCA8K_NUM_PORTS];
>  	struct net_device *mgmt_master; /* Track if mdio/mib Ethernet is available */
>  	struct qca8k_mgmt_eth_data mgmt_eth_data;
>  	struct qca8k_mib_eth_data mib_eth_data;
> -- 
> 2.34.1
>