Thread (38 messages) 38 messages, 4 authors, 2021-11-11

Re: [PATCH v17 bpf-next 12/23] bpf: add multi-buff support to the bpf_xdp_adjust_tail() API

From: Toke Høiland-Jørgensen <hidden>
Date: 2021-11-08 18:08:11
Also in: bpf

Lorenzo Bianconi [off-list ref] writes:
quoted
On Thu,  4 Nov 2021 18:35:32 +0100 Lorenzo Bianconi wrote:
quoted
This change adds support for tail growing and shrinking for XDP multi-buff.

When called on a multi-buffer packet with a grow request, it will always
work on the last fragment of the packet. So the maximum grow size is the
last fragments tailroom, i.e. no new buffer will be allocated.

When shrinking, it will work from the last fragment, all the way down to
the base buffer depending on the shrinking size. It's important to mention
that once you shrink down the fragment(s) are freed, so you can not grow
again to the original size.
quoted
+static int bpf_xdp_mb_increase_tail(struct xdp_buff *xdp, int offset)
+{
+	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
+	skb_frag_t *frag = &sinfo->frags[sinfo->nr_frags - 1];
+	int size, tailroom;
+
+	tailroom = xdp->frame_sz - skb_frag_size(frag) - skb_frag_off(frag);
I know I complained about this before but the assumption that we can
use all the space up to xdp->frame_sz makes me uneasy.

Drivers may not expect the idea that core may decide to extend the 
last frag.. I don't think the skb path would ever do this.

How do you feel about any of these options: 
 - dropping this part for now (return an error for increase)
 - making this an rxq flag or reading the "reserved frag size"
   from rxq (so that drivers explicitly opt-in)
 - adding a test that can be run on real NICs
?
I think this has been added to be symmetric with bpf_xdp_adjust_tail().
I do think there is a real use-case for it so far so I am fine to just
support the shrink part.

@Eelco, Jesper, Toke: any comments on it?
Well, tail adjust is useful for things like encapsulations that need to
add a trailer. Don't see why that wouldn't be something people would
want to do for jumboframes as well?

Not sure I get what the issue is with this either? But having a test
that can be run to validate this on hardware would be great in any case,
I suppose - we've been discussing more general "compliance tests" for
XDP before...

-Toke
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help