Skip to content

Commit

Permalink
xsk: Fix handling of invalid descriptors in XSK TX batching API
Browse files Browse the repository at this point in the history
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: 9349eb3 ("xsk: Introduce batched Tx descriptor interfaces")
Signed-off-by: Maciej Fijalkowski <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: Magnus Karlsson <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
  • Loading branch information
mfijalko authored and borkmann committed Jun 8, 2022
1 parent 2cf7b7f commit d678cbd
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 10 deletions.
5 changes: 3 additions & 2 deletions net/xdp/xsk.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand Down
8 changes: 0 additions & 8 deletions net/xdp/xsk_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit d678cbd

Please sign in to comment.