* [PATCH bpf] xsk: Fix handling of invalid descriptors in XSK Tx batching API
@ 2022-06-07 14:22 Maciej Fijalkowski
2022-06-08 9:55 ` Magnus Karlsson
2022-06-08 14:30 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Maciej Fijalkowski @ 2022-06-07 14:22 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: netdev, magnus.karlsson, bjorn, Maciej Fijalkowski
Xdpxceiver run on a AF_XDP ZC enabled driver revealed a problem with XSK
Tx batching API. There is a test that checks how invalid Tx descriptors
are handled by AF_XDP. Each valid descriptor is followed by invalid one
on Tx side whereas the Rx side expects only to receive a set of valid
descriptors.
In current xsk_tx_peek_release_desc_batch() function, the amount of
available descriptors is hidden inside xskq_cons_peek_desc_batch(). This
can be problematic in cases where invalid descriptors are present due to
the fact that xskq_cons_peek_desc_batch() returns only a count of valid
descriptors. This means that it is impossible to properly update XSK
ring state when calling xskq_cons_release_n().
To address this issue, pull out the contents of
xskq_cons_peek_desc_batch() so that callers (currently only
xsk_tx_peek_release_desc_batch()) will always be able to update the
state of ring properly, as total count of entries is now available and
use this value as an argument in xskq_cons_release_n(). By
doing so, xskq_cons_peek_desc_batch() can be dropped altogether.
Fixes: 9349eb3a9d2a ("xsk: Introduce batched Tx descriptor interfaces")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
net/xdp/xsk.c | 5 +++--
net/xdp/xsk_queue.h | 8 --------
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index e0a4526ab66b..19ac872a6624 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -373,7 +373,8 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 max_entries)
goto out;
}
- nb_pkts = xskq_cons_peek_desc_batch(xs->tx, pool, max_entries);
+ max_entries = xskq_cons_nb_entries(xs->tx, max_entries);
+ nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, max_entries);
if (!nb_pkts) {
xs->tx->queue_empty_descs++;
goto out;
@@ -389,7 +390,7 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 max_entries)
if (!nb_pkts)
goto out;
- xskq_cons_release_n(xs->tx, nb_pkts);
+ xskq_cons_release_n(xs->tx, max_entries);
__xskq_cons_release(xs->tx);
xs->sk.sk_write_space(&xs->sk);
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index a794410989cc..fb20bf7207cf 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -282,14 +282,6 @@ static inline bool xskq_cons_peek_desc(struct xsk_queue *q,
return xskq_cons_read_desc(q, desc, pool);
}
-static inline u32 xskq_cons_peek_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
- u32 max)
-{
- u32 entries = xskq_cons_nb_entries(q, max);
-
- return xskq_cons_read_desc_batch(q, pool, entries);
-}
-
/* To improve performance in the xskq_cons_release functions, only update local state here.
* Reflect this to global state when we get new entries from the ring in
* xskq_cons_get_entries() and whenever Rx or Tx processing are completed in the NAPI loop.
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] xsk: Fix handling of invalid descriptors in XSK Tx batching API
2022-06-07 14:22 [PATCH bpf] xsk: Fix handling of invalid descriptors in XSK Tx batching API Maciej Fijalkowski
@ 2022-06-08 9:55 ` Magnus Karlsson
2022-06-08 14:30 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Magnus Karlsson @ 2022-06-08 9:55 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Network Development,
Karlsson, Magnus, Björn Töpel
On Tue, Jun 7, 2022 at 7:16 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> Xdpxceiver run on a AF_XDP ZC enabled driver revealed a problem with XSK
> Tx batching API. There is a test that checks how invalid Tx descriptors
> are handled by AF_XDP. Each valid descriptor is followed by invalid one
> on Tx side whereas the Rx side expects only to receive a set of valid
> descriptors.
>
> In current xsk_tx_peek_release_desc_batch() function, the amount of
> available descriptors is hidden inside xskq_cons_peek_desc_batch(). This
> can be problematic in cases where invalid descriptors are present due to
> the fact that xskq_cons_peek_desc_batch() returns only a count of valid
> descriptors. This means that it is impossible to properly update XSK
> ring state when calling xskq_cons_release_n().
>
> To address this issue, pull out the contents of
> xskq_cons_peek_desc_batch() so that callers (currently only
> xsk_tx_peek_release_desc_batch()) will always be able to update the
> state of ring properly, as total count of entries is now available and
> use this value as an argument in xskq_cons_release_n(). By
> doing so, xskq_cons_peek_desc_batch() can be dropped altogether.
Thank you for catching this Maciej!
Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Fixes: 9349eb3a9d2a ("xsk: Introduce batched Tx descriptor interfaces")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
> net/xdp/xsk.c | 5 +++--
> net/xdp/xsk_queue.h | 8 --------
> 2 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index e0a4526ab66b..19ac872a6624 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -373,7 +373,8 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 max_entries)
> goto out;
> }
>
> - nb_pkts = xskq_cons_peek_desc_batch(xs->tx, pool, max_entries);
> + max_entries = xskq_cons_nb_entries(xs->tx, max_entries);
> + nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, max_entries);
> if (!nb_pkts) {
> xs->tx->queue_empty_descs++;
> goto out;
> @@ -389,7 +390,7 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 max_entries)
> if (!nb_pkts)
> goto out;
>
> - xskq_cons_release_n(xs->tx, nb_pkts);
> + xskq_cons_release_n(xs->tx, max_entries);
> __xskq_cons_release(xs->tx);
> xs->sk.sk_write_space(&xs->sk);
>
> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index a794410989cc..fb20bf7207cf 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h
> @@ -282,14 +282,6 @@ static inline bool xskq_cons_peek_desc(struct xsk_queue *q,
> return xskq_cons_read_desc(q, desc, pool);
> }
>
> -static inline u32 xskq_cons_peek_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
> - u32 max)
> -{
> - u32 entries = xskq_cons_nb_entries(q, max);
> -
> - return xskq_cons_read_desc_batch(q, pool, entries);
> -}
> -
> /* To improve performance in the xskq_cons_release functions, only update local state here.
> * Reflect this to global state when we get new entries from the ring in
> * xskq_cons_get_entries() and whenever Rx or Tx processing are completed in the NAPI loop.
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] xsk: Fix handling of invalid descriptors in XSK Tx batching API
2022-06-07 14:22 [PATCH bpf] xsk: Fix handling of invalid descriptors in XSK Tx batching API Maciej Fijalkowski
2022-06-08 9:55 ` Magnus Karlsson
@ 2022-06-08 14:30 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-06-08 14:30 UTC (permalink / raw)
To: Maciej Fijalkowski; +Cc: bpf, ast, daniel, netdev, magnus.karlsson, bjorn
Hello:
This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Tue, 7 Jun 2022 16:22:00 +0200 you wrote:
> Xdpxceiver run on a AF_XDP ZC enabled driver revealed a problem with XSK
> Tx batching API. There is a test that checks how invalid Tx descriptors
> are handled by AF_XDP. Each valid descriptor is followed by invalid one
> on Tx side whereas the Rx side expects only to receive a set of valid
> descriptors.
>
> In current xsk_tx_peek_release_desc_batch() function, the amount of
> available descriptors is hidden inside xskq_cons_peek_desc_batch(). This
> can be problematic in cases where invalid descriptors are present due to
> the fact that xskq_cons_peek_desc_batch() returns only a count of valid
> descriptors. This means that it is impossible to properly update XSK
> ring state when calling xskq_cons_release_n().
>
> [...]
Here is the summary with links:
- [bpf] xsk: Fix handling of invalid descriptors in XSK Tx batching API
https://git.kernel.org/bpf/bpf/c/d678cbd2f867
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] 3+ messages in thread
end of thread, other threads:[~2022-06-08 14:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 14:22 [PATCH bpf] xsk: Fix handling of invalid descriptors in XSK Tx batching API Maciej Fijalkowski
2022-06-08 9:55 ` Magnus Karlsson
2022-06-08 14:30 ` patchwork-bot+netdevbpf
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).