* [PATCH net v2 1/5] qede: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
@ 2023-01-25 7:48 ` Magnus Karlsson
2023-01-25 7:48 ` [PATCH net v2 2/5] lan966x: " Magnus Karlsson
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Magnus Karlsson @ 2023-01-25 7:48 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, mst, jasowang, ioana.ciornei,
madalin.bucur
Cc: bpf
From: Magnus Karlsson <magnus.karlsson@intel.com>
Make sure that xdp_do_flush() is always executed before
napi_complete_done(). This is important for two reasons. First, a
redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
napi context X on CPU Y will be followed by a xdp_do_flush() from the
same napi context and CPU. This is not guaranteed if the
napi_complete_done() is executed before xdp_do_flush(), as it tells
the napi logic that it is fine to schedule napi context X on another
CPU. Details from a production system triggering this bug using the
veth driver can be found following the first link below.
The second reason is that the XDP_REDIRECT logic in itself relies on
being inside a single NAPI instance through to the xdp_do_flush() call
for RCU protection of all in-kernel data structures. Details can be
found in the second link below.
Fixes: d1b25b79e162b ("qede: add .ndo_xdp_xmit() and XDP_REDIRECT support")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
Link: https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
---
drivers/net/ethernet/qlogic/qede/qede_fp.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 7c2af482192d..cb1746bc0e0c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -1438,6 +1438,10 @@ int qede_poll(struct napi_struct *napi, int budget)
rx_work_done = (likely(fp->type & QEDE_FASTPATH_RX) &&
qede_has_rx_work(fp->rxq)) ?
qede_rx_int(fp, budget) : 0;
+
+ if (fp->xdp_xmit & QEDE_XDP_REDIRECT)
+ xdp_do_flush();
+
/* Handle case where we are called by netpoll with a budget of 0 */
if (rx_work_done < budget || !budget) {
if (!qede_poll_is_more_work(fp)) {
@@ -1457,9 +1461,6 @@ int qede_poll(struct napi_struct *napi, int budget)
qede_update_tx_producer(fp->xdp_tx);
}
- if (fp->xdp_xmit & QEDE_XDP_REDIRECT)
- xdp_do_flush_map();
-
return rx_work_done;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net v2 2/5] lan966x: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
2023-01-25 7:48 ` [PATCH net v2 1/5] qede: " Magnus Karlsson
@ 2023-01-25 7:48 ` Magnus Karlsson
2023-01-25 7:48 ` [PATCH net v2 3/5] virtio-net: " Magnus Karlsson
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Magnus Karlsson @ 2023-01-25 7:48 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, mst, jasowang, ioana.ciornei,
madalin.bucur
Cc: bpf, Steen Hegelund
From: Magnus Karlsson <magnus.karlsson@intel.com>
Make sure that xdp_do_flush() is always executed before
napi_complete_done(). This is important for two reasons. First, a
redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
napi context X on CPU Y will be followed by a xdp_do_flush() from the
same napi context and CPU. This is not guaranteed if the
napi_complete_done() is executed before xdp_do_flush(), as it tells
the napi logic that it is fine to schedule napi context X on another
CPU. Details from a production system triggering this bug using the
veth driver can be found following the first link below.
The second reason is that the XDP_REDIRECT logic in itself relies on
being inside a single NAPI instance through to the xdp_do_flush() call
for RCU protection of all in-kernel data structures. Details can be
found in the second link below.
Fixes: a825b611c7c1 ("net: lan966x: Add support for XDP_REDIRECT")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
Link: https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
---
drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
index 5314c064ceae..55b484b10562 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
@@ -608,12 +608,12 @@ static int lan966x_fdma_napi_poll(struct napi_struct *napi, int weight)
lan966x_fdma_rx_reload(rx);
}
- if (counter < weight && napi_complete_done(napi, counter))
- lan_wr(0xff, lan966x, FDMA_INTR_DB_ENA);
-
if (redirect)
xdp_do_flush();
+ if (counter < weight && napi_complete_done(napi, counter))
+ lan_wr(0xff, lan966x, FDMA_INTR_DB_ENA);
+
return counter;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net v2 3/5] virtio-net: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
2023-01-25 7:48 ` [PATCH net v2 1/5] qede: " Magnus Karlsson
2023-01-25 7:48 ` [PATCH net v2 2/5] lan966x: " Magnus Karlsson
@ 2023-01-25 7:48 ` Magnus Karlsson
2023-01-27 10:49 ` Michael S. Tsirkin
2023-01-25 7:49 ` [PATCH net v2 4/5] dpaa_eth: " Magnus Karlsson
` (3 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Magnus Karlsson @ 2023-01-25 7:48 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, mst, jasowang, ioana.ciornei,
madalin.bucur
Cc: bpf
From: Magnus Karlsson <magnus.karlsson@intel.com>
Make sure that xdp_do_flush() is always executed before
napi_complete_done(). This is important for two reasons. First, a
redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
napi context X on CPU Y will be followed by a xdp_do_flush() from the
same napi context and CPU. This is not guaranteed if the
napi_complete_done() is executed before xdp_do_flush(), as it tells
the napi logic that it is fine to schedule napi context X on another
CPU. Details from a production system triggering this bug using the
veth driver can be found following the first link below.
The second reason is that the XDP_REDIRECT logic in itself relies on
being inside a single NAPI instance through to the xdp_do_flush() call
for RCU protection of all in-kernel data structures. Details can be
found in the second link below.
Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
Link: https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
---
drivers/net/virtio_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 18b3de854aeb..6df14dd5bf46 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1677,13 +1677,13 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
received = virtnet_receive(rq, budget, &xdp_xmit);
+ if (xdp_xmit & VIRTIO_XDP_REDIR)
+ xdp_do_flush();
+
/* Out of packets? */
if (received < budget)
virtqueue_napi_complete(napi, rq->vq, received);
- if (xdp_xmit & VIRTIO_XDP_REDIR)
- xdp_do_flush();
-
if (xdp_xmit & VIRTIO_XDP_TX) {
sq = virtnet_xdp_get_sq(vi);
if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net v2 3/5] virtio-net: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 ` [PATCH net v2 3/5] virtio-net: " Magnus Karlsson
@ 2023-01-27 10:49 ` Michael S. Tsirkin
0 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2023-01-27 10:49 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, jasowang, ioana.ciornei,
madalin.bucur, bpf
On Wed, Jan 25, 2023 at 08:48:59AM +0100, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Make sure that xdp_do_flush() is always executed before
> napi_complete_done(). This is important for two reasons. First, a
> redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
> napi context X on CPU Y will be followed by a xdp_do_flush() from the
> same napi context and CPU. This is not guaranteed if the
> napi_complete_done() is executed before xdp_do_flush(), as it tells
> the napi logic that it is fine to schedule napi context X on another
> CPU. Details from a production system triggering this bug using the
> veth driver can be found following the first link below.
>
> The second reason is that the XDP_REDIRECT logic in itself relies on
> being inside a single NAPI instance through to the xdp_do_flush() call
> for RCU protection of all in-kernel data structures. Details can be
> found in the second link below.
>
> Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
> Link: https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 18b3de854aeb..6df14dd5bf46 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1677,13 +1677,13 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>
> received = virtnet_receive(rq, budget, &xdp_xmit);
>
> + if (xdp_xmit & VIRTIO_XDP_REDIR)
> + xdp_do_flush();
> +
> /* Out of packets? */
> if (received < budget)
> virtqueue_napi_complete(napi, rq->vq, received);
>
> - if (xdp_xmit & VIRTIO_XDP_REDIR)
> - xdp_do_flush();
> -
> if (xdp_xmit & VIRTIO_XDP_TX) {
> sq = virtnet_xdp_get_sq(vi);
> if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) {
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net v2 4/5] dpaa_eth: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
` (2 preceding siblings ...)
2023-01-25 7:48 ` [PATCH net v2 3/5] virtio-net: " Magnus Karlsson
@ 2023-01-25 7:49 ` Magnus Karlsson
2023-01-25 14:58 ` Camelia Alexandra Groza
2023-01-25 7:49 ` [PATCH net v2 5/5] dpaa2-eth: " Magnus Karlsson
` (2 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Magnus Karlsson @ 2023-01-25 7:49 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, mst, jasowang, ioana.ciornei,
madalin.bucur
Cc: bpf
From: Magnus Karlsson <magnus.karlsson@intel.com>
Make sure that xdp_do_flush() is always executed before
napi_complete_done(). This is important for two reasons. First, a
redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
napi context X on CPU Y will be followed by a xdp_do_flush() from the
same napi context and CPU. This is not guaranteed if the
napi_complete_done() is executed before xdp_do_flush(), as it tells
the napi logic that it is fine to schedule napi context X on another
CPU. Details from a production system triggering this bug using the
veth driver can be found following the first link below.
The second reason is that the XDP_REDIRECT logic in itself relies on
being inside a single NAPI instance through to the xdp_do_flush() call
for RCU protection of all in-kernel data structures. Details can be
found in the second link below.
Fixes: a1e031ffb422 ("dpaa_eth: add XDP_REDIRECT support")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
Link: https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 3f8032947d86..027fff9f7db0 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2410,6 +2410,9 @@ static int dpaa_eth_poll(struct napi_struct *napi, int budget)
cleaned = qman_p_poll_dqrr(np->p, budget);
+ if (np->xdp_act & XDP_REDIRECT)
+ xdp_do_flush();
+
if (cleaned < budget) {
napi_complete_done(napi, cleaned);
qman_p_irqsource_add(np->p, QM_PIRQ_DQRI);
@@ -2417,9 +2420,6 @@ static int dpaa_eth_poll(struct napi_struct *napi, int budget)
qman_p_irqsource_add(np->p, QM_PIRQ_DQRI);
}
- if (np->xdp_act & XDP_REDIRECT)
- xdp_do_flush();
-
return cleaned;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* RE: [PATCH net v2 4/5] dpaa_eth: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:49 ` [PATCH net v2 4/5] dpaa_eth: " Magnus Karlsson
@ 2023-01-25 14:58 ` Camelia Alexandra Groza
0 siblings, 0 replies; 10+ messages in thread
From: Camelia Alexandra Groza @ 2023-01-25 14:58 UTC (permalink / raw)
To: Magnus Karlsson, magnus.karlsson@intel.com, bjorn@kernel.org,
ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
jonathan.lemon@gmail.com, maciej.fijalkowski@intel.com,
kuba@kernel.org, toke@redhat.com, pabeni@redhat.com,
davem@davemloft.net, aelior@marvell.com, manishc@marvell.com,
horatiu.vultur@microchip.com, UNGLinuxDriver@microchip.com,
mst@redhat.com, jasowang@redhat.com, Ioana Ciornei, Madalin Bucur
Cc: bpf@vger.kernel.org
> -----Original Message-----
> From: Magnus Karlsson <magnus.karlsson@gmail.com>
> Sent: Wednesday, January 25, 2023 9:49
> To: magnus.karlsson@intel.com; bjorn@kernel.org; ast@kernel.org;
> daniel@iogearbox.net; netdev@vger.kernel.org;
> jonathan.lemon@gmail.com; maciej.fijalkowski@intel.com;
> kuba@kernel.org; toke@redhat.com; pabeni@redhat.com;
> davem@davemloft.net; aelior@marvell.com; manishc@marvell.com;
> horatiu.vultur@microchip.com; UNGLinuxDriver@microchip.com;
> mst@redhat.com; jasowang@redhat.com; Ioana Ciornei
> <ioana.ciornei@nxp.com>; Madalin Bucur <madalin.bucur@nxp.com>
> Cc: bpf@vger.kernel.org
> Subject: [PATCH net v2 4/5] dpaa_eth: execute xdp_do_flush() before
> napi_complete_done()
>
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Make sure that xdp_do_flush() is always executed before
> napi_complete_done(). This is important for two reasons. First, a
> redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
> napi context X on CPU Y will be followed by a xdp_do_flush() from the
> same napi context and CPU. This is not guaranteed if the
> napi_complete_done() is executed before xdp_do_flush(), as it tells
> the napi logic that it is fine to schedule napi context X on another
> CPU. Details from a production system triggering this bug using the
> veth driver can be found following the first link below.
>
> The second reason is that the XDP_REDIRECT logic in itself relies on
> being inside a single NAPI instance through to the xdp_do_flush() call
> for RCU protection of all in-kernel data structures. Details can be
> found in the second link below.
>
> Fixes: a1e031ffb422 ("dpaa_eth: add XDP_REDIRECT support")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
> Link: https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
> ---
Acked-by: Camelia Groza <camelia.groza@nxp.com>
Thanks!
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index 3f8032947d86..027fff9f7db0 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2410,6 +2410,9 @@ static int dpaa_eth_poll(struct napi_struct *napi, int
> budget)
>
> cleaned = qman_p_poll_dqrr(np->p, budget);
>
> + if (np->xdp_act & XDP_REDIRECT)
> + xdp_do_flush();
> +
> if (cleaned < budget) {
> napi_complete_done(napi, cleaned);
> qman_p_irqsource_add(np->p, QM_PIRQ_DQRI);
> @@ -2417,9 +2420,6 @@ static int dpaa_eth_poll(struct napi_struct *napi, int
> budget)
> qman_p_irqsource_add(np->p, QM_PIRQ_DQRI);
> }
>
> - if (np->xdp_act & XDP_REDIRECT)
> - xdp_do_flush();
> -
> return cleaned;
> }
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net v2 5/5] dpaa2-eth: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
` (3 preceding siblings ...)
2023-01-25 7:49 ` [PATCH net v2 4/5] dpaa_eth: " Magnus Karlsson
@ 2023-01-25 7:49 ` Magnus Karlsson
2023-01-27 10:50 ` [PATCH net v2 0/5] net: xdp: " Michael S. Tsirkin
2023-01-28 6:40 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 10+ messages in thread
From: Magnus Karlsson @ 2023-01-25 7:49 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, mst, jasowang, ioana.ciornei,
madalin.bucur
Cc: bpf
From: Magnus Karlsson <magnus.karlsson@intel.com>
Make sure that xdp_do_flush() is always executed before
napi_complete_done(). This is important for two reasons. First, a
redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
napi context X on CPU Y will be followed by a xdp_do_flush() from the
same napi context and CPU. This is not guaranteed if the
napi_complete_done() is executed before xdp_do_flush(), as it tells
the napi logic that it is fine to schedule napi context X on another
CPU. Details from a production system triggering this bug using the
veth driver can be found following the first link below.
The second reason is that the XDP_REDIRECT logic in itself relies on
being inside a single NAPI instance through to the xdp_do_flush() call
for RCU protection of all in-kernel data structures. Details can be
found in the second link below.
Fixes: d678be1dc1ec ("dpaa2-eth: add XDP_REDIRECT support")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
Link: https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
---
drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 0c35abb7d065..2e79d18fc3c7 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -1993,10 +1993,15 @@ static int dpaa2_eth_poll(struct napi_struct *napi, int budget)
if (rx_cleaned >= budget ||
txconf_cleaned >= DPAA2_ETH_TXCONF_PER_NAPI) {
work_done = budget;
+ if (ch->xdp.res & XDP_REDIRECT)
+ xdp_do_flush();
goto out;
}
} while (store_cleaned);
+ if (ch->xdp.res & XDP_REDIRECT)
+ xdp_do_flush();
+
/* Update NET DIM with the values for this CDAN */
dpaa2_io_update_net_dim(ch->dpio, ch->stats.frames_per_cdan,
ch->stats.bytes_per_cdan);
@@ -2032,9 +2037,7 @@ static int dpaa2_eth_poll(struct napi_struct *napi, int budget)
txc_fq->dq_bytes = 0;
}
- if (ch->xdp.res & XDP_REDIRECT)
- xdp_do_flush_map();
- else if (rx_cleaned && ch->xdp.res & XDP_TX)
+ if (rx_cleaned && ch->xdp.res & XDP_TX)
dpaa2_eth_xdp_tx_flush(priv, ch, &priv->fq[flowid]);
return work_done;
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
` (4 preceding siblings ...)
2023-01-25 7:49 ` [PATCH net v2 5/5] dpaa2-eth: " Magnus Karlsson
@ 2023-01-27 10:50 ` Michael S. Tsirkin
2023-01-28 6:40 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2023-01-27 10:50 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, jasowang, ioana.ciornei,
madalin.bucur, bpf
On Wed, Jan 25, 2023 at 08:48:56AM +0100, Magnus Karlsson wrote:
> Make sure that xdp_do_flush() is always executed before
> napi_complete_done(). This is important for two reasons. First, a
> redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
> napi context X on CPU Y will be followed by a xdp_do_flush() from the
> same napi context and CPU. This is not guaranteed if the
> napi_complete_done() is executed before xdp_do_flush(), as it tells
> the napi logic that it is fine to schedule napi context X on another
> CPU. Details from a production system triggering this bug using the
> veth driver can be found in [1].
>
> The second reason is that the XDP_REDIRECT logic in itself relies on
> being inside a single NAPI instance through to the xdp_do_flush() call
> for RCU protection of all in-kernel data structures. Details can be
> found in [2].
>
> The drivers have only been compile-tested since I do not own any of
> the HW below. So if you are a maintainer, it would be great if you
> could take a quick look to make sure I did not mess something up.
>
> Note that these were the drivers I found that violated the ordering by
> running a simple script and manually checking the ones that came up as
> potential offenders. But the script was not perfect in any way. There
> might still be offenders out there, since the script can generate
> false negatives.
BTW all this series is stable material, right?
> v1 -> v2:
> * Added acks [Toke, Steen]
> * Corrected two spelling errors [Toke]
>
> [1] https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
> [2] https://lore.kernel.org/all/20210624160609.292325-1-toke@redhat.com/
>
> Thanks: Magnus
>
> Magnus Karlsson (5):
> qede: execute xdp_do_flush() before napi_complete_done()
> lan966x: execute xdp_do_flush() before napi_complete_done()
> virtio-net: execute xdp_do_flush() before napi_complete_done()
> dpaa_eth: execute xdp_do_flush() before napi_complete_done()
> dpaa2-eth: execute xdp_do_flush() before napi_complete_done()
>
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 +++---
> drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 9 ++++++---
> drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c | 6 +++---
> drivers/net/ethernet/qlogic/qede/qede_fp.c | 7 ++++---
> drivers/net/virtio_net.c | 6 +++---
> 5 files changed, 19 insertions(+), 15 deletions(-)
>
>
> base-commit: 2a48216cff7a2e3964fbed16f84d33f68b3e5e42
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done()
2023-01-25 7:48 [PATCH net v2 0/5] net: xdp: execute xdp_do_flush() before napi_complete_done() Magnus Karlsson
` (5 preceding siblings ...)
2023-01-27 10:50 ` [PATCH net v2 0/5] net: xdp: " Michael S. Tsirkin
@ 2023-01-28 6:40 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-01-28 6:40 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
maciej.fijalkowski, kuba, toke, pabeni, davem, aelior, manishc,
horatiu.vultur, UNGLinuxDriver, mst, jasowang, ioana.ciornei,
madalin.bucur, bpf
Hello:
This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 25 Jan 2023 08:48:56 +0100 you wrote:
> Make sure that xdp_do_flush() is always executed before
> napi_complete_done(). This is important for two reasons. First, a
> redirect to an XSKMAP assumes that a call to xdp_do_redirect() from
> napi context X on CPU Y will be followed by a xdp_do_flush() from the
> same napi context and CPU. This is not guaranteed if the
> napi_complete_done() is executed before xdp_do_flush(), as it tells
> the napi logic that it is fine to schedule napi context X on another
> CPU. Details from a production system triggering this bug using the
> veth driver can be found in [1].
>
> [...]
Here is the summary with links:
- [net,v2,1/5] qede: execute xdp_do_flush() before napi_complete_done()
https://git.kernel.org/netdev/net/c/2ccce20d51fa
- [net,v2,2/5] lan966x: execute xdp_do_flush() before napi_complete_done()
https://git.kernel.org/netdev/net/c/12b5717990c8
- [net,v2,3/5] virtio-net: execute xdp_do_flush() before napi_complete_done()
https://git.kernel.org/netdev/net/c/ad7e615f646c
- [net,v2,4/5] dpaa_eth: execute xdp_do_flush() before napi_complete_done()
https://git.kernel.org/netdev/net/c/b534013798b7
- [net,v2,5/5] dpaa2-eth: execute xdp_do_flush() before napi_complete_done()
https://git.kernel.org/netdev/net/c/a3191c4d86c5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 10+ messages in thread