2022-04-12 22:52:13

by Christian Marangi

[permalink] [raw]
Subject: [net-next PATCH v2 1/4] drivers: net: dsa: qca8k: drop MTU tracking from qca8k_priv

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 <[email protected]>
---
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.
+ * 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


2022-04-15 23:47:23

by Paolo Abeni

[permalink] [raw]
Subject: Re: [net-next PATCH v2 1/4] drivers: net: dsa: qca8k: drop MTU tracking from qca8k_priv

On Tue, 2022-04-12 at 19:30 +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 <[email protected]>

@Vladimir: it looks like this addresses your concern on the previous
iteration, could you please have a look?

Thanks!

Paolo

2022-04-16 01:52:45

by Vladimir Oltean

[permalink] [raw]
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 <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

> 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
>

2022-04-17 09:28:11

by Florian Fainelli

[permalink] [raw]
Subject: Re: [net-next PATCH v2 1/4] drivers: net: dsa: qca8k: drop MTU tracking from qca8k_priv



On 4/12/2022 10:30 AM, 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 <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian