Re: [PATCH net] net: mscc: fix the frame extraction into the skb

[Alexandre Belloni]

On 20/09/2018 12:08:54+0200, Antoine Ténart wrote:
> When extracting frames from the Ocelot switch, the frame check sequence
> (FCS) is present at the end of the data extracted. The FCS was put into
> the sk buffer which introduced some issues (as length related ones), as
> the FCS shouldn't be part of an Rx sk buffer.
> 
> This patch fixes the Ocelot switch extraction behaviour by discarding
> the FCS.
> 
> Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
> Signed-off-by: Antoine Tenart <antoine.tenart@xxxxxxxxxxx>
Reviewed-by: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>

> ---
> 
> Hi all,
> 
> Once this patch will be accepted and net merged into net-next, I'll
> probably send another patch to fill skb->csum with the FCS based on
> NETIF_F_RXFCS.
> 
> Thanks!
> Antoine
> 
>  drivers/net/ethernet/mscc/ocelot_board.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> index 26bb3b18f3be..3cdf63e35b53 100644
> --- a/drivers/net/ethernet/mscc/ocelot_board.c
> +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> @@ -91,7 +91,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
>  		struct sk_buff *skb;
>  		struct net_device *dev;
>  		u32 *buf;
> -		int sz, len;
> +		int sz, len, buf_len;
>  		u32 ifh[4];
>  		u32 val;
>  		struct frame_info info;
> @@ -116,14 +116,20 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
>  			err = -ENOMEM;
>  			break;
>  		}
> -		buf = (u32 *)skb_put(skb, info.len);
> +		buf_len = info.len - ETH_FCS_LEN;
> +		buf = (u32 *)skb_put(skb, buf_len);
>  
>  		len = 0;
>  		do {
>  			sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
>  			*buf++ = val;
>  			len += sz;
> -		} while ((sz == 4) && (len < info.len));
> +		} while (len < buf_len);
> +
> +		/* Read the FCS and discard it */
> +		sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
> +		/* Update the statistics if part of the FCS was read before */
> +		len -= ETH_FCS_LEN - sz;
>  
>  		if (sz < 0) {
>  			err = sz;
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com