On 11/25/20 11:09 AM, Magnus Karlsson wrote:
On Wed, Nov 25, 2020 at 11:07 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
On 11/25/20 10:13 AM, Magnus Karlsson wrote:
On Wed, Nov 25, 2020 at 10:02 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
On 11/25/20 9:30 AM, Magnus Karlsson wrote:
On Tue, Nov 24, 2020 at 10:58 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
On 11/24/20 9:12 AM, Magnus Karlsson wrote:
On Tue, Nov 24, 2020 at 8:33 AM Li RongQing <lirongqing@xxxxxxxxx> wrote:
Add a new function for returning descriptors the user received
after an xsk_ring_cons__peek call. After the application has
gotten a number of descriptors from a ring, it might not be able
to or want to process them all for various reasons. Therefore,
it would be useful to have an interface for returning or
cancelling a number of them so that they are returned to the ring.
This patch adds a new function called xsk_ring_cons__cancel that
performs this operation on nb descriptors counted from the end of
the batch of descriptors that was received through the peek call.
Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>
[ Magnus Karlsson: rewrote changelog ]
Cc: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
---
diff with v1: fix the building, and rewrote changelog
tools/lib/bpf/xsk.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 1069c46364ff..1719a327e5f9 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -153,6 +153,12 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
return entries;
}
+static inline void xsk_ring_cons__cancel(struct xsk_ring_cons *cons,
+ size_t nb)
+{
+ cons->cached_cons -= nb;
+}
+
static inline void xsk_ring_cons__release(struct xsk_ring_cons *cons, size_t nb)
{
/* Make sure data has been read before indicating we are done
--
2.17.3
Thank you RongQing.
Acked-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
@Magnus: shouldn't the xsk_ring_cons__cancel() nb type be '__u32 nb' instead?
All the other interfaces have size_t as the type for "nb". It is kind
of weird as a __u32 would have made more sense, but cannot actually
remember why I chose a size_t two years ago. But for consistency with
the other interfaces, let us keep it a size_t for now. I will do some
research around the reason.
It's actually a bit of a mix currently which is what got me confused:
static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod, size_t nb, __u32 *idx)
static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons, size_t nb, __u32 *idx)
static inline void xsk_ring_cons__release(struct xsk_ring_cons *cons, size_t nb)
(I can take it in as-is, but would be nice to clean it up a bit to avoid confusion.)
Hmm, that is confusing indeed. Well, the best choice would be __u32
everywhere since the ring pointers themselves are __u32. But I am
somewhat afraid of changing an API. Can we guarantee that a change
from size_t to __u32 will not break some user's compilation? Another
option would be to clean this up next year when we will very likely
produce a 1.0 version of this API and at that point we can change some
things. What do you think would be the best approach?
Given they're all inlines, imho, risk should be fairly low to switch all to __u32.
I would probably go and verify first with DPDK as main user of the lib and/or write
some test cases to see if compiler spills any new warnings and the like, but if not
the case then we should do it for bpf-next so this has plenty of exposure in the
meantime. Any nb large than u32 max is a bug in any case.
Sounds good. Will do and get back to you.
Great, thanks, I took in the current patch to bpf-next in that case and the rest can
be followed-up as discussed.
Thanks,
Daniel
