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

From: Alexandre Belloni
Date: Mon Oct 01 2018 - 05:49:15 EST


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