netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Regression in mvneta with XDP patches
@ 2019-11-11 13:46 Andrew Lunn
  2019-11-11 14:33 ` Ilias Apalodimas
  2019-11-11 14:37 ` Lorenzo Bianconi
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-11-11 13:46 UTC (permalink / raw)
  To: ilias.apalodimas, brouer, lorenzo; +Cc: netdev

Hi Lorenzo, Jesper, Ilias

I just found that the XDP patches to mvneta have caused a regression.

This one breaks networking:

commit 8dc9a0888f4c8e27b25e48ff1b4bc2b3a845cc2d (HEAD)
Author: Lorenzo Bianconi <lorenzo@kernel.org>
Date:   Sat Oct 19 10:13:23 2019 +0200

    net: mvneta: rely on build_skb in mvneta_rx_swbm poll routine
    
    Refactor mvneta_rx_swbm code introducing mvneta_swbm_rx_frame and
    mvneta_swbm_add_rx_fragment routines. Rely on build_skb in oreder to
    allocate skb since the previous patch introduced buffer recycling using
    the page_pool API.
    This patch fixes even an issue in the original driver where dma buffers
    are accessed before dma sync.
    mvneta driver can run on not cache coherent devices so it is
    necessary to sync DMA buffers before sending them to the device
    in order to avoid memory corruptions. Running perf analysis we can
    see a performance cost associated with this DMA-sync (anyway it is
    already there in the original driver code). In follow up patches we
    will add more logic to reduce DMA-sync as much as possible.

I'm using an Linksys WRT1900ac, which has an Armada XP SoC. Device
tree is arch/arm/boot/dts/armada-xp-linksys-mamba.dts.

With this patch applied, transmit appears to work O.K. My dhcp server
is seeing good looking BOOTP requests and replying. However what is
being received by the WRT1900ac is bad.

11:36:20.038558 d8:f7:00:00:00:00 (oui Unknown) > 00:00:00:00:5a:45 (oui Ethernet) Null Informati4
        0x0000:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0010:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0030:  0000 0000 0000                           ......
11:36:26.924914 d8:f7:00:00:00:00 (oui Unknown) > 00:00:00:00:5a:45 (oui Ethernet) Null Informati4
        0x0000:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0010:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0030:  0000 0000 0000                           ......
11:36:27.636597 4c:69:6e:75:78:20 (oui Unknown) > 6e:20:47:4e:55:2f (oui Unknown), ethertype Unkn 
        0x0000:  2873 7472 6574 6368 2920 4c69 6e75 7820  (stretch).Linux.
        0x0010:  352e 342e 302d 7263 362d 3031 3438 312d  5.4.0-rc6-01481-
        0x0020:  6739 3264 3965 3038 3439 3662 382d 6469  g92d9e08496b8-di
        0x0030:  7274 7920 2333 2053 756e 204e 6f76 2031  rty.#3.Sun.Nov.1
        0x0040:  3020 3136 3a31 373a 3531 2043 5354 2032  0.16:17:51.CST.2
        0x0050:  3031 3920 6172 6d76 376c 0e04 009c 0080  019.armv7l......
        0x0060:  100c 0501 0a00 000e 0200 0000 0200 1018  ................
        0x0070:  1102 fe80 0000 0000 0000 eefa aaff fe01  ................
        0x0080:  12fe 0200 0000 0200 0804 6574 6830 fe09  ..........eth0..
        0x0090:  0012 0f03 0100 0000 00fe 0900 120f 0103  ................
        0x00a0:  ec00 0010 0000 e3ed 5509 0000 0000 0000  ........U.......
        0x00b0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x00c0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x00d0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x00e0:  0000 0000 0000

This actually looks like random kernel memory.

     Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 13:46 Regression in mvneta with XDP patches Andrew Lunn
@ 2019-11-11 14:33 ` Ilias Apalodimas
  2019-11-11 15:05   ` Andrew Lunn
  2019-11-11 14:37 ` Lorenzo Bianconi
  1 sibling, 1 reply; 9+ messages in thread
From: Ilias Apalodimas @ 2019-11-11 14:33 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: brouer, lorenzo, netdev

Hi Andrew,

On Mon, Nov 11, 2019 at 02:46:15PM +0100, Andrew Lunn wrote:
> Hi Lorenzo, Jesper, Ilias
> 
> I just found that the XDP patches to mvneta have caused a regression.
> 
> This one breaks networking:

Thaks for the report.
Looking at the DTS i can see 'buffer-manager' in it. The changes we made were
for the driver path software buffer manager. 
Can you confirm which one your hardware uses?

I tested the patches on an espressobin, but the DTS i was using back then did
not register the dsa infrastructure and i only had an 'eth0'.

> 
> commit 8dc9a0888f4c8e27b25e48ff1b4bc2b3a845cc2d (HEAD)
> Author: Lorenzo Bianconi <lorenzo@kernel.org>
> Date:   Sat Oct 19 10:13:23 2019 +0200
> 
>     net: mvneta: rely on build_skb in mvneta_rx_swbm poll routine
>     
>     Refactor mvneta_rx_swbm code introducing mvneta_swbm_rx_frame and
>     mvneta_swbm_add_rx_fragment routines. Rely on build_skb in oreder to
>     allocate skb since the previous patch introduced buffer recycling using
>     the page_pool API.
>     This patch fixes even an issue in the original driver where dma buffers
>     are accessed before dma sync.
>     mvneta driver can run on not cache coherent devices so it is
>     necessary to sync DMA buffers before sending them to the device
>     in order to avoid memory corruptions. Running perf analysis we can
>     see a performance cost associated with this DMA-sync (anyway it is
>     already there in the original driver code). In follow up patches we
>     will add more logic to reduce DMA-sync as much as possible.
> 
> I'm using an Linksys WRT1900ac, which has an Armada XP SoC. Device
> tree is arch/arm/boot/dts/armada-xp-linksys-mamba.dts.
> 
> With this patch applied, transmit appears to work O.K. My dhcp server
> is seeing good looking BOOTP requests and replying. However what is
> being received by the WRT1900ac is bad.

What if you assign a static ip address? Same effect?

> 
> 11:36:20.038558 d8:f7:00:00:00:00 (oui Unknown) > 00:00:00:00:5a:45 (oui Ethernet) Null Informati4
>         0x0000:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0010:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0030:  0000 0000 0000                           ......
> 11:36:26.924914 d8:f7:00:00:00:00 (oui Unknown) > 00:00:00:00:5a:45 (oui Ethernet) Null Informati4
>         0x0000:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0010:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0030:  0000 0000 0000                           ......
> 11:36:27.636597 4c:69:6e:75:78:20 (oui Unknown) > 6e:20:47:4e:55:2f (oui Unknown), ethertype Unkn 
>         0x0000:  2873 7472 6574 6368 2920 4c69 6e75 7820  (stretch).Linux.
>         0x0010:  352e 342e 302d 7263 362d 3031 3438 312d  5.4.0-rc6-01481-
>         0x0020:  6739 3264 3965 3038 3439 3662 382d 6469  g92d9e08496b8-di
>         0x0030:  7274 7920 2333 2053 756e 204e 6f76 2031  rty.#3.Sun.Nov.1
>         0x0040:  3020 3136 3a31 373a 3531 2043 5354 2032  0.16:17:51.CST.2
>         0x0050:  3031 3920 6172 6d76 376c 0e04 009c 0080  019.armv7l......
>         0x0060:  100c 0501 0a00 000e 0200 0000 0200 1018  ................
>         0x0070:  1102 fe80 0000 0000 0000 eefa aaff fe01  ................
>         0x0080:  12fe 0200 0000 0200 0804 6574 6830 fe09  ..........eth0..
>         0x0090:  0012 0f03 0100 0000 00fe 0900 120f 0103  ................
>         0x00a0:  ec00 0010 0000 e3ed 5509 0000 0000 0000  ........U.......
>         0x00b0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x00c0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x00d0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x00e0:  0000 0000 0000
> 
> This actually looks like random kernel memory.
> 
>      Andrew

Thanks
/Ilias

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 13:46 Regression in mvneta with XDP patches Andrew Lunn
  2019-11-11 14:33 ` Ilias Apalodimas
@ 2019-11-11 14:37 ` Lorenzo Bianconi
  2019-11-11 15:09   ` Andrew Lunn
  1 sibling, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2019-11-11 14:37 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ilias.apalodimas, brouer, netdev

[-- Attachment #1: Type: text/plain, Size: 3790 bytes --]

> Hi Lorenzo, Jesper, Ilias
> 

Hi Andrew,

> I just found that the XDP patches to mvneta have caused a regression.
> 
> This one breaks networking:
> 
> commit 8dc9a0888f4c8e27b25e48ff1b4bc2b3a845cc2d (HEAD)
> Author: Lorenzo Bianconi <lorenzo@kernel.org>
> Date:   Sat Oct 19 10:13:23 2019 +0200
> 
>     net: mvneta: rely on build_skb in mvneta_rx_swbm poll routine
>     
>     Refactor mvneta_rx_swbm code introducing mvneta_swbm_rx_frame and
>     mvneta_swbm_add_rx_fragment routines. Rely on build_skb in oreder to
>     allocate skb since the previous patch introduced buffer recycling using
>     the page_pool API.
>     This patch fixes even an issue in the original driver where dma buffers
>     are accessed before dma sync.
>     mvneta driver can run on not cache coherent devices so it is
>     necessary to sync DMA buffers before sending them to the device
>     in order to avoid memory corruptions. Running perf analysis we can
>     see a performance cost associated with this DMA-sync (anyway it is
>     already there in the original driver code). In follow up patches we
>     will add more logic to reduce DMA-sync as much as possible.
> 
> I'm using an Linksys WRT1900ac, which has an Armada XP SoC. Device
> tree is arch/arm/boot/dts/armada-xp-linksys-mamba.dts.

looking at the dts, could you please confirm mvneta is using hw or sw buffer manager
on this board? Moreover are you using DSA as well?

Regards,
Lorenzo

> 
> With this patch applied, transmit appears to work O.K. My dhcp server
> is seeing good looking BOOTP requests and replying. However what is
> being received by the WRT1900ac is bad.
> 
> 11:36:20.038558 d8:f7:00:00:00:00 (oui Unknown) > 00:00:00:00:5a:45 (oui Ethernet) Null Informati4
>         0x0000:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0010:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0030:  0000 0000 0000                           ......
> 11:36:26.924914 d8:f7:00:00:00:00 (oui Unknown) > 00:00:00:00:5a:45 (oui Ethernet) Null Informati4
>         0x0000:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0010:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0020:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x0030:  0000 0000 0000                           ......
> 11:36:27.636597 4c:69:6e:75:78:20 (oui Unknown) > 6e:20:47:4e:55:2f (oui Unknown), ethertype Unkn 
>         0x0000:  2873 7472 6574 6368 2920 4c69 6e75 7820  (stretch).Linux.
>         0x0010:  352e 342e 302d 7263 362d 3031 3438 312d  5.4.0-rc6-01481-
>         0x0020:  6739 3264 3965 3038 3439 3662 382d 6469  g92d9e08496b8-di
>         0x0030:  7274 7920 2333 2053 756e 204e 6f76 2031  rty.#3.Sun.Nov.1
>         0x0040:  3020 3136 3a31 373a 3531 2043 5354 2032  0.16:17:51.CST.2
>         0x0050:  3031 3920 6172 6d76 376c 0e04 009c 0080  019.armv7l......
>         0x0060:  100c 0501 0a00 000e 0200 0000 0200 1018  ................
>         0x0070:  1102 fe80 0000 0000 0000 eefa aaff fe01  ................
>         0x0080:  12fe 0200 0000 0200 0804 6574 6830 fe09  ..........eth0..
>         0x0090:  0012 0f03 0100 0000 00fe 0900 120f 0103  ................
>         0x00a0:  ec00 0010 0000 e3ed 5509 0000 0000 0000  ........U.......
>         0x00b0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x00c0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x00d0:  0000 0000 0000 0000 0000 0000 0000 0000  ................
>         0x00e0:  0000 0000 0000
> 
> This actually looks like random kernel memory.
> 
>      Andrew

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 14:33 ` Ilias Apalodimas
@ 2019-11-11 15:05   ` Andrew Lunn
  2019-11-11 15:14     ` Lorenzo Bianconi
  2019-11-11 15:15     ` Ilias Apalodimas
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-11-11 15:05 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: brouer, lorenzo, netdev

On Mon, Nov 11, 2019 at 04:33:52PM +0200, Ilias Apalodimas wrote:
> Hi Andrew,
> 
> On Mon, Nov 11, 2019 at 02:46:15PM +0100, Andrew Lunn wrote:
> > Hi Lorenzo, Jesper, Ilias
> > 
> > I just found that the XDP patches to mvneta have caused a regression.
> > 
> > This one breaks networking:
> 
> Thaks for the report.
> Looking at the DTS i can see 'buffer-manager' in it. The changes we made were
> for the driver path software buffer manager. 
> Can you confirm which one your hardware uses?

Hi Ilias

Ah, interesting.

# CONFIG_MVNETA_BM_ENABLE is not set

So in fact it is not being compiled in, so should be falling back to
software buffer manager.

If i do enable it, then it works. So we are in a corner cases you
probably never tested. Requested by DT, but not actually available.

	 Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 14:37 ` Lorenzo Bianconi
@ 2019-11-11 15:09   ` Andrew Lunn
  2019-11-11 15:12     ` Ilias Apalodimas
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-11-11 15:09 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: ilias.apalodimas, brouer, netdev

> looking at the dts, could you please confirm mvneta is using hw or sw buffer manager
> on this board? Moreover are you using DSA as well?

So my reply to Ilias answered the first question. And yes, i'm using
DSA. But that should not matter, mvneta is just receiving frames which
happen to have an extra header after the two MAC addresses.

       Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 15:09   ` Andrew Lunn
@ 2019-11-11 15:12     ` Ilias Apalodimas
  0 siblings, 0 replies; 9+ messages in thread
From: Ilias Apalodimas @ 2019-11-11 15:12 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Lorenzo Bianconi, brouer, netdev

On Mon, Nov 11, 2019 at 04:09:07PM +0100, Andrew Lunn wrote:
> > looking at the dts, could you please confirm mvneta is using hw or sw buffer manager
> > on this board? Moreover are you using DSA as well?
> 
> So my reply to Ilias answered the first question. And yes, i'm using
> DSA. But that should not matter, mvneta is just receiving frames which
> happen to have an extra header after the two MAC addresses.
> 
In theory (famous last words) DSA or not shouldn't be affected by this. 
The driver was already allocating a page per packet before our changes. 
We just allocate that page via page_pool.

Thanks
/Ilias

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 15:05   ` Andrew Lunn
@ 2019-11-11 15:14     ` Lorenzo Bianconi
  2019-11-11 15:45       ` Andrew Lunn
  2019-11-11 15:15     ` Ilias Apalodimas
  1 sibling, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2019-11-11 15:14 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Ilias Apalodimas, brouer, netdev

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

On Nov 11, Andrew Lunn wrote:
> On Mon, Nov 11, 2019 at 04:33:52PM +0200, Ilias Apalodimas wrote:
> > Hi Andrew,
> > 
> > On Mon, Nov 11, 2019 at 02:46:15PM +0100, Andrew Lunn wrote:
> > > Hi Lorenzo, Jesper, Ilias
> > > 
> > > I just found that the XDP patches to mvneta have caused a regression.
> > > 
> > > This one breaks networking:
> > 
> > Thaks for the report.
> > Looking at the DTS i can see 'buffer-manager' in it. The changes we made were
> > for the driver path software buffer manager. 
> > Can you confirm which one your hardware uses?
> 
> Hi Ilias
> 
> Ah, interesting.
> 
> # CONFIG_MVNETA_BM_ENABLE is not set
> 
> So in fact it is not being compiled in, so should be falling back to
> software buffer manager.
> 
> If i do enable it, then it works. So we are in a corner cases you
> probably never tested. Requested by DT, but not actually available.
> 
> 	 Andrew

Maybe I got the issue. If mvneta_bm_port_init fails (e.g. if
CONFIG_MVNETA_BM_ENABLE is not set) rx_offset_correction will be set to a wrong
value. To bisect the issue, could you please test the following patch?

Regards,
Lorenzo

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 591d580c68b4..e476fb043379 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4944,7 +4944,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);
@@ -4969,6 +4968,9 @@ static int mvneta_probe(struct platform_device *pdev)
 	}
 	of_node_put(bm_node);
 
+	if (!pp->bm_priv)
+		pp->rx_offset_correction = MVNETA_SKB_HEADROOM;
+
 	err = mvneta_init(&pdev->dev, pp);
 	if (err < 0)
 		goto err_netdev;


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 15:05   ` Andrew Lunn
  2019-11-11 15:14     ` Lorenzo Bianconi
@ 2019-11-11 15:15     ` Ilias Apalodimas
  1 sibling, 0 replies; 9+ messages in thread
From: Ilias Apalodimas @ 2019-11-11 15:15 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: brouer, lorenzo, netdev

Hi Andrew,

On Mon, Nov 11, 2019 at 04:05:53PM +0100, Andrew Lunn wrote:
> On Mon, Nov 11, 2019 at 04:33:52PM +0200, Ilias Apalodimas wrote:
> > Hi Andrew,
> > 
> > On Mon, Nov 11, 2019 at 02:46:15PM +0100, Andrew Lunn wrote:
> > > Hi Lorenzo, Jesper, Ilias
> > > 
> > > I just found that the XDP patches to mvneta have caused a regression.
> > > 
> > > This one breaks networking:
> > 
> > Thaks for the report.
> > Looking at the DTS i can see 'buffer-manager' in it. The changes we made were
> > for the driver path software buffer manager. 
> > Can you confirm which one your hardware uses?
> 
> Hi Ilias
> 
> Ah, interesting.
> 
> # CONFIG_MVNETA_BM_ENABLE is not set
> 
> So in fact it is not being compiled in, so should be falling back to
> software buffer manager.
> 
> If i do enable it, then it works. So we are in a corner cases you
> probably never tested. Requested by DT, but not actually available.
> 

Correct, i don't think any of us had access to a hardware with BM
We did change some offsets of memory allocation in the swbm to make room for
skb_shared_info and the xdp headlen we need. 
Maybe we missed that on the 'fallback' scenario. I'll try to have a look on the
driver

> 	 Andrew

Thanks
/Ilias

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Regression in mvneta with XDP patches
  2019-11-11 15:14     ` Lorenzo Bianconi
@ 2019-11-11 15:45       ` Andrew Lunn
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-11-11 15:45 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Ilias Apalodimas, brouer, netdev

> Maybe I got the issue. If mvneta_bm_port_init fails (e.g. if
> CONFIG_MVNETA_BM_ENABLE is not set) rx_offset_correction will be set to a wrong
> value. To bisect the issue, could you please test the following patch?

Hi Lorenzo

Did not help. Received packets are still corrupt.

    Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-11-11 15:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-11 13:46 Regression in mvneta with XDP patches Andrew Lunn
2019-11-11 14:33 ` Ilias Apalodimas
2019-11-11 15:05   ` Andrew Lunn
2019-11-11 15:14     ` Lorenzo Bianconi
2019-11-11 15:45       ` Andrew Lunn
2019-11-11 15:15     ` Ilias Apalodimas
2019-11-11 14:37 ` Lorenzo Bianconi
2019-11-11 15:09   ` Andrew Lunn
2019-11-11 15:12     ` Ilias Apalodimas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).