diff mbox series

[net-next,v2,7/7] net: dsa: vsc73xx: fix MTU configuration

Message ID 20230625115343.1603330-7-paweldembicki@gmail.com
State New
Headers
Series [net-next,v2,1/7] net: dsa: vsc73xx: use read_poll_timeout instead delay loop |

Commit Message

Pawel Dembicki June 25, 2023, 11:53 a.m. UTC 
  Switch in MAXLEN register store maximum size of data frame.
MTU size is 18 bytes smaller than frame size.

Current settings causes problems with packet forwarding.
This patch fix MTU settings to proper values.

Fixes: fb77ffc6ec86 ("net: dsa: vsc73xx: make the MTU configurable")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
v2:
  - fix commit message style issue

 drivers/net/dsa/vitesse-vsc73xx-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Vladimir Oltean June 25, 2023, 2:54 p.m. UTC | #1 
On Sun, Jun 25, 2023 at 01:53:42PM +0200, Pawel Dembicki wrote:
> Switch in MAXLEN register store maximum size of data frame.
> MTU size is 18 bytes smaller than frame size.
> 
> Current settings causes problems with packet forwarding.
> This patch fix MTU settings to proper values.
> 
> Fixes: fb77ffc6ec86 ("net: dsa: vsc73xx: make the MTU configurable")
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---

Please split this patch from the rest of the series and re-target it
towards net.git.

> v2:
>   - fix commit message style issue
> 
>  drivers/net/dsa/vitesse-vsc73xx-core.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
> index c946464489ab..59bb3dbe780a 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx-core.c
> +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
> @@ -979,17 +979,18 @@ static int vsc73xx_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
>  	struct vsc73xx *vsc = ds->priv;
>  
>  	return vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port,
> -			     VSC73XX_MAXLEN, new_mtu);
> +			     VSC73XX_MAXLEN, new_mtu + ETH_HLEN + ETH_FCS_LEN);
>  }
>  
>  /* According to application not "VSC7398 Jumbo Frames" setting
> - * up the MTU to 9.6 KB does not affect the performance on standard
> + * up the frame size to 9.6 KB does not affect the performance on standard
>   * frames. It is clear from the application note that
>   * "9.6 kilobytes" == 9600 bytes.
>   */
>  static int vsc73xx_get_max_mtu(struct dsa_switch *ds, int port)
>  {
> -	return 9600;
> +	/* max mtu = 9600 - ETH_HLEN - ETH_FCS_LEN */
> +	return 9582;

This can also be:

	return 9600 - ETH_HLEN - ETH_FCS_LEN;

since the arithmetic is on constants, it can be evaluated at compile
time and it results in the same generated code, but the comment is no
longer necessary.

>  }
>  
>  static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port,
> -- 
> 2.34.1
>
Pawel Dembicki June 28, 2023, 8:04 p.m. UTC | #2 
niedz., 25 cze 2023 o 16:54 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Sun, Jun 25, 2023 at 01:53:42PM +0200, Pawel Dembicki wrote:
> > Switch in MAXLEN register store maximum size of data frame.
> > MTU size is 18 bytes smaller than frame size.
> >
> > Current settings causes problems with packet forwarding.
> > This patch fix MTU settings to proper values.
> >
> > Fixes: fb77ffc6ec86 ("net: dsa: vsc73xx: make the MTU configurable")
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> > ---
>
> Please split this patch from the rest of the series and re-target it
> towards net.git.
>

I resend it.
https://lore.kernel.org/netdev/20230628194327.1765644-1-paweldembicki@gmail.com/

--
Pawel Dembicki

> > v2:
> >   - fix commit message style issue
> >
> >  drivers/net/dsa/vitesse-vsc73xx-core.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
> > index c946464489ab..59bb3dbe780a 100644
> > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c
> > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
> > @@ -979,17 +979,18 @@ static int vsc73xx_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> >       struct vsc73xx *vsc = ds->priv;
> >
> >       return vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port,
> > -                          VSC73XX_MAXLEN, new_mtu);
> > +                          VSC73XX_MAXLEN, new_mtu + ETH_HLEN + ETH_FCS_LEN);
> >  }
> >
> >  /* According to application not "VSC7398 Jumbo Frames" setting
> > - * up the MTU to 9.6 KB does not affect the performance on standard
> > + * up the frame size to 9.6 KB does not affect the performance on standard
> >   * frames. It is clear from the application note that
> >   * "9.6 kilobytes" == 9600 bytes.
> >   */
> >  static int vsc73xx_get_max_mtu(struct dsa_switch *ds, int port)
> >  {
> > -     return 9600;
> > +     /* max mtu = 9600 - ETH_HLEN - ETH_FCS_LEN */
> > +     return 9582;
>
> This can also be:
>
>         return 9600 - ETH_HLEN - ETH_FCS_LEN;
>
> since the arithmetic is on constants, it can be evaluated at compile
> time and it results in the same generated code, but the comment is no
> longer necessary.
>
> >  }
> >
> >  static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port,
> > --
> > 2.34.1
> >
>
diff mbox series

Patch

diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index c946464489ab..59bb3dbe780a 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -979,17 +979,18 @@  static int vsc73xx_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 	struct vsc73xx *vsc = ds->priv;
 
 	return vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port,
-			     VSC73XX_MAXLEN, new_mtu);
+			     VSC73XX_MAXLEN, new_mtu + ETH_HLEN + ETH_FCS_LEN);
 }
 
 /* According to application not "VSC7398 Jumbo Frames" setting
- * up the MTU to 9.6 KB does not affect the performance on standard
+ * up the frame size to 9.6 KB does not affect the performance on standard
  * frames. It is clear from the application note that
  * "9.6 kilobytes" == 9600 bytes.
  */
 static int vsc73xx_get_max_mtu(struct dsa_switch *ds, int port)
 {
-	return 9600;
+	/* max mtu = 9600 - ETH_HLEN - ETH_FCS_LEN */
+	return 9582;
 }
 
 static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port,