Date:   Mon, 27 Jan 2020 09:04:55 +0000
From:   Sven Auhagen <sven.auhagen@...eatech.de>
To:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "thomas.petazzoni@...tlin.com" <thomas.petazzoni@...tlin.com>,
        "brouer@...hat.com" <brouer@...hat.com>,
        "ilias.apalodimas@...aro.org" <ilias.apalodimas@...aro.org>,
        "matteo.croce@...hat.com" <matteo.croce@...hat.com>,
        "mw@...ihalf.com" <mw@...ihalf.com>,
        "jakub.kicinski@...ronome.com" <jakub.kicinski@...ronome.com>
Subject: Re: [PATCH] mvneta driver XDP fixes armhf

On Sat, Jan 25, 2020 at 05:30:16PM +0100, Lorenzo Bianconi wrote:
> > On Fri, Jan 24, 2020 at 12:03:46PM +0000, Sven Auhagen wrote:
> > > On Wed, Jan 22, 2020 at 11:57:40PM +0100, lorenzo.bianconi@...hat.com wrote:
> > > > Hi Sven,
> > > >
> 
> [...]
> 
> > 
> > I think I found the problem. I found the documentation finally and it states:
> > 
> > The physical buffer pointer must be 64-bit aligned; therefore, bits[2:0] of the pointers are considered as zeros.
> > 
> > rx_offset is defined as MVNETA_SKB_HEADROOM which in turn is:
> > 
> > #define MVNETA_SKB_HEADROOM(max(XDP_PACKET_HEADROOM, NET_SKB_PAD) + \
> >  NET_IP_ALIGN)
> > 
> > this leads to an offset on armhf of 258 which is not 64 bit aligned and therefore
> > shortened to 256 hence the MH header is actually at 256.
> > 
> > This would explain my problem.
> > 
> > Any thoughts?
> 
> Hi Sven,
> 
> IIUC how the hw works, I guess we can reduce MVNETA_SKB_HEADROOM and let the hw put the MH
> header to align the IP header. Could you please try the following patch?
> 
> Regards,
> Lorenzo
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 67ad8b8b127d..c032cffa6ae8 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -324,8 +324,7 @@
>  	      ETH_HLEN + ETH_FCS_LEN,			     \
>  	      cache_line_size())
>  
> -#define MVNETA_SKB_HEADROOM	(max(XDP_PACKET_HEADROOM, NET_SKB_PAD) + \
> -				 NET_IP_ALIGN)
> +#define MVNETA_SKB_HEADROOM	max(XDP_PACKET_HEADROOM, NET_SKB_PAD)
>  #define MVNETA_SKB_PAD	(SKB_DATA_ALIGN(sizeof(struct skb_shared_info) + \
>  			 MVNETA_SKB_HEADROOM))
>  #define MVNETA_SKB_SIZE(len)	(SKB_DATA_ALIGN(len) + MVNETA_SKB_PAD)
> @@ -1167,6 +1166,7 @@ static void mvneta_bm_update_mtu(struct mvneta_port *pp, int mtu)
>  	mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short, 1 << pp->id);
>  
>  	pp->bm_priv = NULL;
> +	pp->rx_offset_correction = MVNETA_SKB_HEADROOM;
>  	mvreg_write(pp, MVNETA_ACC_MODE, MVNETA_ACC_MODE_EXT1);
>  	netdev_info(pp->dev, "fail to update MTU, fall back to software BM\n");
>  }
> @@ -4942,7 +4942,6 @@ static int mvneta_probe(struct platform_device *pdev)
>  	SET_NETDEV_DEV(dev, &pdev->dev);
>  
>  	pp->id = global_port_id++;
> -	pp->rx_offset_correction = MVNETA_SKB_HEADROOM;
>  
>  	/* Obtain access to BM resources if enabled and already initialized */
>  	bm_node = of_parse_phandle(dn, "buffer-manager", 0);
> @@ -4967,6 +4966,10 @@ static int mvneta_probe(struct platform_device *pdev)
>  	}
>  	of_node_put(bm_node);
>  
> +	/* sw buffer management */
> +	if (!pp->bm_priv)
> +		pp->rx_offset_correction = MVNETA_SKB_HEADROOM;
> +
>  	err = mvneta_init(&pdev->dev, pp);
>  	if (err < 0)
>  		goto err_netdev;
> @@ -5124,6 +5127,7 @@ static int mvneta_resume(struct device *device)
>  		err = mvneta_bm_port_init(pdev, pp);
>  		if (err < 0) {
>  			dev_info(&pdev->dev, "use SW buffer management\n");
> +			pp->rx_offset_correction = MVNETA_SKB_HEADROOM;
>  			pp->bm_priv = NULL;
>  		}
>  	}

This patch works on my armada 388 board, thanks.
Are you going to send in the patch?

Best
Sven

> 
> > 
> > Best
> > Sven
> > 
> > >
> > > >
> > > > >
> > > > > Attached is a patch that fixes the problem on my armhf platform, as said I am not sure if this is a universal fix or armhf only.
> > > > >
> > > > > Any feedback is appreciated.
> > > > >
> > > > > Signed-off-by: Sven Auhagen <sven.auhagen@...eatech.de>
> > > > >
> > > > > --- a/drivers/net/ethernet/marvell/mvneta.c2020-01-22 08:44:05.611395960 +0000
> > > > > +++ b/drivers/net/ethernet/marvell/mvneta.c2020-01-22 08:59:27.053739433 +0000
> > > > > @@ -2158,7 +2158,7 @@ mvneta_swbm_rx_frame(struct mvneta_port
> > > > >  prefetch(data);
> > > > >
> > > > >  xdp->data_hard_start = data;
> > > > > -xdp->data = data + pp->rx_offset_correction + MVNETA_MH_SIZE;
> > > > > +xdp->data = data + pp->rx_offset_correction;
> > > >
> > > > This will break XDP support for 'real' sw buffer devices like Espressobin.
> > >
> > > The current code seems to break real hw buffer devices using sw buffer on armhf though.
> > >
> > > Best
> > > Sven
> > >
> > > >
> > > > Regards,
> > > > Lorenzo
> > > >
> > > > >  xdp->data_end = xdp->data + data_len;
> > > > >  xdp_set_data_meta_invalid(xdp);
> > > > >
> > > > > @@ -4960,7 +4960,8 @@ static int mvneta_probe(struct platform_
> > > > >   * NET_SKB_PAD, exceeds 64B. It should be 64B for 64-bit
> > > > >   * platforms and 0B for 32-bit ones.
> > > > >   */
> > > > > -pp->rx_offset_correction = max(0,
> > > > > +if (pp->bm_priv)
> > > > > +pp->rx_offset_correction = max(0,
> > > > >         NET_SKB_PAD -
> > > > >         MVNETA_RX_PKT_OFFSET_CORRECTION);
> > > > >  }
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > +++ Voleatech auf der E-World, 11. bis 13. Februar 2020, Halle 5, Stand 521 +++
> > > > >
> > > > > Beste Grüße/Best regards
> > > > >
> > > > > Sven Auhagen
> > > > > Dipl. Math. oec., M.Sc.
> > > > > Voleatech GmbH
> > > > > HRB: B 754643
> > > > > USTID: DE303643180
> > > > > Grathwohlstr. 5
> > > > > 72762 Reutlingen
> > > > > Tel: +49 7121539550
> > > > > Fax: +49 7121539551
> > > > > E-Mail: sven.auhagen@...eatech.de
> > > > > https://eur03.safelinks.protection.outlook.com/?url=www.voleatech.de&amp;data=02%7C01%7Csven.auhagen%40voleatech.de%7C16ecc6de7670473d7de108d7a0c5cf85%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637154643755759442&amp;sdata=PlhiaiQCqIc9Pmkux%2B8xLf%2FiwP3Nn3UsMRozhbX%2FR%2B0%3D&amp;reserved=0<https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.voleatech.de&amp;data=02%7C01%7Csven.auhagen%40voleatech.de%7C16ecc6de7670473d7de108d7a0c5cf85%7Cb82a99f679814a7295344d35298f847b%7C0%7C0%7C637154643755759442&amp;sdata=sSpe8NSqXN8dJOp%2Fb%2FaaHcEPTdtT4jE59ek97VvTtlY%3D&amp;reserved=0>
> > > > > Diese Information ist ausschließlich für den Adressaten bestimmt und kann vertraulich oder gesetzlich geschützte Informationen enthalten. Wenn Sie nicht der bestimmungsgemäße Adressat sind, unterrichten Sie bitte den Absender und vernichten Sie diese Mail. Anderen als dem bestimmungsgemäßen Adressaten ist es untersagt, diese E-Mail zu lesen, zu speichern, weiterzuleiten oder ihren Inhalt auf welche Weise auch immer zu verwenden. Für den Adressaten sind die Informationen in dieser Mail nur zum persönlichen Gebrauch. Eine Weiterleitung darf nur nach Rücksprache mit dem Absender erfolgen. Wir verwenden aktuelle Virenschutzprogramme. Für Schäden, die dem Empfänger gleichwohl durch von uns zugesandte mit Viren befallene E-Mails entstehen, schließen wir jede Haftung aus.
> > >
> > >
> > >
> > 
> > +++ Voleatech auf der E-World, 11. bis 13. Februar 2020, Halle 5, Stand 521 +++
> > 
> > Beste Grüße/Best regards
> > 
> > Sven Auhagen
> > Dipl. Math. oec., M.Sc.
> > Voleatech GmbH
> > HRB: B 754643
> > USTID: DE303643180
> > Grathwohlstr. 5
> > 72762 Reutlingen
> > Tel: +49 7121539550
> > Fax: +49 7121539551
> > E-Mail: sven.auhagen@...eatech.de
> > www.voleatech.de<https://www.voleatech.de>
> > Diese Information ist ausschließlich für den Adressaten bestimmt und kann vertraulich oder gesetzlich geschützte Informationen enthalten. Wenn Sie nicht der bestimmungsgemäße Adressat sind, unterrichten Sie bitte den Absender und vernichten Sie diese Mail. Anderen als dem bestimmungsgemäßen Adressaten ist es untersagt, diese E-Mail zu lesen, zu speichern, weiterzuleiten oder ihren Inhalt auf welche Weise auch immer zu verwenden. Für den Adressaten sind die Informationen in dieser Mail nur zum persönlichen Gebrauch. Eine Weiterleitung darf nur nach Rücksprache mit dem Absender erfolgen. Wir verwenden aktuelle Virenschutzprogramme. Für Schäden, die dem Empfänger gleichwohl durch von uns zugesandte mit Viren befallene E-Mails entstehen, schließen wir jede Haftung aus.
> > 

Powered by blists - more mailing lists