Re: [PATCH net 2/3] dsa: mv88e6xxx: Fix MTU definition

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Mon, May 24, 2021 at 11:33:12PM +0200, Andrew Lunn wrote:
> The MTU passed to the DSA driver is the payload size, typically 1500.
> However, the switch uses the frame size when applying restrictions.
> Adjust the MTU with the size of the Ethernet header and the frame
> checksum.
> 
> Fixes: 1baf0fac10fb ("net: dsa: mv88e6xxx: Use chip-wide max frame size for MTU")
> Reported by: 曹煜 <cao88yu@gmail.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 12 ++++++------
>  drivers/net/dsa/mv88e6xxx/port.c |  2 ++
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index cbedaee3cefd..593dc734582e 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2775,8 +2775,8 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
>  	if (err)
>  		return err;
>  
> -	/* Port Control 2: don't force a good FCS, set the maximum frame size to
> -	 * 10240 bytes, disable 802.1q tags checking, don't discard tagged or
> +	/* Port Control 2: don't force a good FCS, set the MTU size to
> +	 * 10222 bytes, disable 802.1q tags checking, don't discard tagged or
>  	 * untagged frames on this port, do a destination address lookup on all
>  	 * received packets as usual, disable ARP mirroring and don't send a
>  	 * copy of all transmitted/received frames on this port to the CPU.
> @@ -2795,7 +2795,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
>  		return err;
>  
>  	if (chip->info->ops->port_set_jumbo_size) {
> -		err = chip->info->ops->port_set_jumbo_size(chip, port, 10240);
> +		err = chip->info->ops->port_set_jumbo_size(chip, port, 10222);
>  		if (err)
>  			return err;
>  	}
> @@ -2885,10 +2885,10 @@ static int mv88e6xxx_get_max_mtu(struct dsa_switch *ds, int port)
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  
>  	if (chip->info->ops->port_set_jumbo_size)
> -		return 10240;
> +		return 10240 - ETH_HLEN - ETH_FCS_LEN;
>  	else if (chip->info->ops->set_max_frame_size)
> -		return 1632;
> -	return 1522;
> +		return 1632 - ETH_HLEN - ETH_FCS_LEN;
> +	return 1522 - ETH_HLEN - ETH_FCS_LEN;
>  }
>  
>  static int mv88e6xxx_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index f77e2ee64a60..28664ea91244 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -1277,6 +1277,8 @@ int mv88e6165_port_set_jumbo_size(struct mv88e6xxx_chip *chip, int port,
>  	u16 reg;
>  	int err;
>  
> +	size += ETH_HLEN + ETH_FCS_LEN;
> +
>  	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL2, &reg);
>  	if (err)
>  		return err;
> -- 
> 2.31.1
> 

Does the hardware account for VLAN-tagged frames automatically? If not,
it is not uncommon to use VLAN_ETH_HLEN instead of ETH_HLEN (the VLAN
header is not part of the L2 SDU).