netdev - Re: [PATCH] mvneta driver XDP fixes armhf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200125111158.nge3qahnocq2obot@SvensMacBookAir.sven.lan>
Date:   Sat, 25 Jan 2020 11:11:59 +0000
From:   Sven Auhagen <sven.auhagen@...eatech.de>
To:     "lorenzo.bianconi@...hat.com" <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 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,
> > 
> > > Recently XDP Support was added to the mvneta driver for software buffer management.
> > > I tested XDP with my armada 388 board. It has hardware buffer management defined in the device tree file.
> > > I disabled the mvneta_bm module to test XDP.
> > > 
> > > I found multiple problems.
> > > 
> > > 1. With hardware buffer management enabled and mvneta_bm disabled the rx_offset was set to 0 with armhf (32 bit) which leads to no headroom in XDP and therefore the XDP Redirect did not work.
> > > 2. Removing the hardware buffer management from the device tree file completely made the mvneta driver unusable as it did not work anymore.
> > 
> > Do you mean removing 'buffer-manager' property from the device tree?
> 
> Yes removing it from each nics device tree definition.
> The if statement is not used in that case and the rx_offset_correction is always MVNETA_SKB_HEADROOM.
> 
> > 
> > > 
> > > After some debugging I found out that xdp->data = data + pp->rx_offset_correction + MVNETA_MH_SIZE;  has to be xdp->data = data + pp->rx_offset_correction; if pp->rx_offset_correction > 0.
> > > I am not sure why and I am looking for help if someone is seeing the same on an arm64 board.
> > 
> > Are you sure the hw does not insert the mvneta header before the data? It seems
> > to me that it is added even for hw buffer devices (according to the code)
> 
> It is definitely possible. The 2 bytes before the data are 0. I believe
> the mvneta header is also 0 when no switch is attached.
> Is it added when a hw buffer capable device is not using and initializing
> the hw buffer?
> 
> Do you have any documentation regarding the header?
> I have access to the Marvell Extranet but could not find anything.

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?

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.
> 
> 
>