[PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[Frank Wunderlich]

From: René van Dorst <opensource@vdorst.com>

in recent Kernel-Versions there are warnings about incorrect MTU-Size
like these:

mt7530 mdio-bus:00: nonfatal error -95 setting MTU on port x
eth0: mtu greater than device maximum
mtk_soc_eth 1b100000.ethernet eth0: error -22 setting MTU to include DSA overhead

Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports")
Fixes: 72579e14a1d3 ("net: dsa: don't fail to probe if we couldn't set the MTU")
Fixes: 7a4c53bee332 ("net: report invalid mtu value via netlink extack")
Signed-off-by: René van Dorst <opensource@vdorst.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
changes in v2:
  Fixes: tag show 12-chars of sha1 and moved above other tags
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 85735d32ecb0..00e3d70f7d07 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2891,6 +2891,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	eth->netdev[id]->irq = eth->irq[0];
 	eth->netdev[id]->dev.of_node = np;

+	eth->netdev[id]->mtu = 1536;
+	eth->netdev[id]->min_mtu = ETH_MIN_MTU;
+	eth->netdev[id]->max_mtu = 1536;
+
 	return 0;

 free_netdev:
--
2.25.1

Re: [PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[Andrew Lunn]

On Thu, Jul 09, 2020 at 07:57:42AM +0200, Frank Wunderlich wrote:
> From: René van Dorst <opensource@vdorst.com>
> 
> in recent Kernel-Versions there are warnings about incorrect MTU-Size
> like these:
> 
> mt7530 mdio-bus:00: nonfatal error -95 setting MTU on port x
> eth0: mtu greater than device maximum
> mtk_soc_eth 1b100000.ethernet eth0: error -22 setting MTU to include DSA overhead
> 
> Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports")
> Fixes: 72579e14a1d3 ("net: dsa: don't fail to probe if we couldn't set the MTU")
> Fixes: 7a4c53bee332 ("net: report invalid mtu value via netlink extack")
> Signed-off-by: René van Dorst <opensource@vdorst.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> changes in v2:
>   Fixes: tag show 12-chars of sha1 and moved above other tags
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 85735d32ecb0..00e3d70f7d07 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2891,6 +2891,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
>  	eth->netdev[id]->irq = eth->irq[0];
>  	eth->netdev[id]->dev.of_node = np;
> 
> +	eth->netdev[id]->mtu = 1536;

Hi Frank

Don't change to MTU from the default. Anybody using this interface for
non-DSA traffic expects the default MTU. DSA will change it as needed.

> +	eth->netdev[id]->min_mtu = ETH_MIN_MTU;

No need to set the minimum. ether_setup() will initialize it.

> +	eth->netdev[id]->max_mtu = 1536;

I assume this is enough to make the DSA warning go away, but it is the
true max? I have a similar patch for the FEC driver which i should
post sometime. Reviewing the FEC code and after some testing, i found
the real max was 2K - 64.

     Andrew

Aw: Re: [PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[Frank Wunderlich]

> Gesendet: Donnerstag, 09. Juli 2020 um 15:41 Uhr
> Von: "Andrew Lunn" <andrew@lunn.ch>

> > +	eth->netdev[id]->max_mtu = 1536;
>
> I assume this is enough to make the DSA warning go away, but it is the
> true max? I have a similar patch for the FEC driver which i should
> post sometime. Reviewing the FEC code and after some testing, i found
> the real max was 2K - 64.

i tried setting only the max_mtu, but the dsa-error is still present

mt7530 mdio-bus:00: nonfatal error -95 setting MTU on port 0

but i got it too, if i revert the change...mhm, strange that these were absent last time...

the other 2 are fixed with only max_mtu.
@andrew where did you got the 2k-64 (=1984) information? sounds like orwell ;)

-95 is EOPNOTSUPP

as so far, commit 72579e14a1d3d3d561039dfe7e5f47aaf22e3fd3 introduces the warning,
but the change is making it non-fatal...so i need to adjust my fixes-tag.
i guess the real problem lies in this:

bfcb813203e6 net: dsa: configure the MTU for switch ports

it looks like dsa_slave_change_mtu failes because of missing callback in mtk_driver (mt7530 for mt7531 in my case).

net/dsa/slave.c
1405 static int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
...
1420     if (!ds->ops->port_change_mtu)
1421         return -EOPNOTSUPP;

i added an empty callback to avoid this message, but mtu should be set in hardware too...
here i will ne some assistance from mtk ethernet experts and mt7531 driver (from landen chao) to be merged first (after some needed changes)

--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2259,6 +2259,12 @@ mt753x_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
        return priv->info->phy_write(ds, port, regnum, val);
 }

+static int
+mt753x_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+       return 0;
+}
+
 static const struct dsa_switch_ops mt7530_switch_ops = {
        .get_tag_protocol       = mtk_get_tag_protocol,
        .setup                  = mt753x_setup,
@@ -2281,6 +2287,7 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
        .port_vlan_del          = mt7530_port_vlan_del,
        .port_mirror_add        = mt7530_port_mirror_add,
        .port_mirror_del        = mt7530_port_mirror_del,
+       .port_change_mtu        = mt753x_port_change_mtu,
        .phylink_validate       = mt753x_phylink_validate,
        .phylink_mac_link_state = mt7530_phylink_mac_link_state,
        .phylink_mac_config     = mt753x_phylink_mac_config,

Re: Re: [PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[Andrew Lunn]

On Thu, Jul 09, 2020 at 09:48:01PM +0200, Frank Wunderlich wrote:
> > Gesendet: Donnerstag, 09. Juli 2020 um 15:41 Uhr
> > Von: "Andrew Lunn" <andrew@lunn.ch>
> 
> > > +	eth->netdev[id]->max_mtu = 1536;
> >
> > I assume this is enough to make the DSA warning go away, but it is the
> > true max? I have a similar patch for the FEC driver which i should
> > post sometime. Reviewing the FEC code and after some testing, i found
> > the real max was 2K - 64.
> 
> i tried setting only the max_mtu, but the dsa-error is still present
> 
> mt7530 mdio-bus:00: nonfatal error -95 setting MTU on port 0
> 
> but i got it too, if i revert the change...mhm, strange that these were absent last time...
> 
> the other 2 are fixed with only max_mtu.
> @andrew where did you got the 2k-64 (=1984) information? sounds like orwell ;)

drivers/net/ethernet/freescale/fec_main.c:

/* The FEC stores dest/src/type/vlan, data, and checksum for receive packets.
 *
 * 2048 byte skbufs are allocated. However, alignment requirements
 * varies between FEC variants. Worst case is 64, so round down by 64.
 */
#define PKT_MAXBUF_SIZE         (round_down(2048 - 64, 64))

So i set the max MTU to this.

> 1405 static int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
> ...
> 1420     if (!ds->ops->port_change_mtu)
> 1421         return -EOPNOTSUPP;

Yes, i also needed to change the mv88e6xxx driver to implement this
function. These switches do support jumbo frames, so i had some real
code in there, not a dummy function.

The marketing brief for the mt7530 says it supports 1518, 1536, 1552
and 9K jumbo frames. It would be good if you can figure out how to
support that, rather than add a dummy function.

	Andrew

Re: [PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[Russell King - ARM Linux admin]

On Thu, Jul 09, 2020 at 03:41:15PM +0200, Andrew Lunn wrote:
> On Thu, Jul 09, 2020 at 07:57:42AM +0200, Frank Wunderlich wrote:
> > From: René van Dorst <opensource@vdorst.com>
> > 
> > in recent Kernel-Versions there are warnings about incorrect MTU-Size
> > like these:
> > 
> > mt7530 mdio-bus:00: nonfatal error -95 setting MTU on port x
> > eth0: mtu greater than device maximum
> > mtk_soc_eth 1b100000.ethernet eth0: error -22 setting MTU to include DSA overhead
> > 
> > Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports")
> > Fixes: 72579e14a1d3 ("net: dsa: don't fail to probe if we couldn't set the MTU")
> > Fixes: 7a4c53bee332 ("net: report invalid mtu value via netlink extack")
> > Signed-off-by: René van Dorst <opensource@vdorst.com>
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> > changes in v2:
> >   Fixes: tag show 12-chars of sha1 and moved above other tags
> > ---
> >  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index 85735d32ecb0..00e3d70f7d07 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -2891,6 +2891,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
> >  	eth->netdev[id]->irq = eth->irq[0];
> >  	eth->netdev[id]->dev.of_node = np;
> > 
> > +	eth->netdev[id]->mtu = 1536;
> 
> Hi Frank
> 
> Don't change to MTU from the default. Anybody using this interface for
> non-DSA traffic expects the default MTU. DSA will change it as needed.
> 
> > +	eth->netdev[id]->min_mtu = ETH_MIN_MTU;
> 
> No need to set the minimum. ether_setup() will initialize it.
> 
> > +	eth->netdev[id]->max_mtu = 1536;
> 
> I assume this is enough to make the DSA warning go away, but it is the
> true max? I have a similar patch for the FEC driver which i should
> post sometime. Reviewing the FEC code and after some testing, i found
> the real max was 2K - 64.

Are there any plans to solve these warnings for Marvell 88e6xxx DSA ports?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[Frank Wunderlich]

Am 9. Juli 2020 22:31:34 MESZ schrieb Russell King - ARM Linux admin <linux@armlinux.org.uk>:
>Are there any plans to solve these warnings for Marvell 88e6xxx DSA
>ports?

Maybe it's a better idea to restore previous condition?

if (ret && ret != -EOPNOTSUPP)

Or use another loglevel (dev_dbg)
regards Frank

Re: [PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

[Andrew Lunn]

> Are there any plans to solve these warnings for Marvell 88e6xxx DSA ports?

Hi Russell

I have patches for FEC + mv88e6xxx. I should post them.

  Andrew