RE: [PATCH v9 bpf-next 08/14] bpf: add multi-buff support to the bpf_xdp_adjust_tail() API

John Fastabend
2021-06-22T23:38:02Z

Lorenzo Bianconi wrote:
> From: Eelco Chaudron <echaudro@redhat.com>
>
> This change adds support for tail growing and shrinking for XDP multi-buff.
>

It would be nice if the commit message gave us some details on how the
growing/shrinking works in the multi-buff support.

> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> include/net/xdp.h | 7 ++++++
> net/core/filter.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++
> net/core/xdp.c | 5 ++--
> 3 files changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 935a6f83115f..3525801c6ed5 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -132,6 +132,11 @@ xdp_get_shared_info_from_buff(struct xdp_buff *xdp)
> return (struct skb_shared_info *)xdp_data_hard_end(xdp);
> }
>
> +static inline unsigned int xdp_get_frag_tailroom(const skb_frag_t *frag)
> +{
> + return PAGE_SIZE - skb_frag_size(frag) - skb_frag_off(frag);
> +}
> +
> struct xdp_frame {
> void *data;
> u16 len;
> @@ -259,6 +264,8 @@ struct xdp_frame *xdp_convert_buff_to_frame(struct xdp_buff *xdp)
> return xdp_frame;
> }
>
> +void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> + struct xdp_buff *xdp);
> void xdp_return_frame(struct xdp_frame *xdpf);
> void xdp_return_frame_rx_napi(struct xdp_frame *xdpf);
> void xdp_return_buff(struct xdp_buff *xdp);
> diff --git a/net/core/filter.c b/net/core/filter.c
> index caa88955562e..05f574a3d690 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3859,11 +3859,73 @@ static const struct bpf_func_proto bpf_xdp_adjust_head_proto = {
> .arg2_type = ARG_ANYTHING,
> };
>
> +static int bpf_xdp_mb_adjust_tail(struct xdp_buff *xdp, int offset)
> +{
> + struct skb_shared_info *sinfo;
> +
> + if (unlikely(!xdp_buff_is_mb(xdp)))
> + return -EINVAL;
> +
> + sinfo = xdp_get_shared_info_from_buff(xdp);
> + if (offset >= 0) {
> + skb_frag_t *frag = &sinfo->frags[sinfo->nr_frags - 1];
> + int size;
> +
> + if (unlikely(offset > xdp_get_frag_tailroom(frag)))
> + return -EINVAL;
> +
> + size = skb_frag_size(frag);
> + memset(skb_frag_address(frag) + size, 0, offset);
> + skb_frag_size_set(frag, size + offset);
> + sinfo->data_len += offset;

Can you add some comment on how this works? So today I call
bpf_xdp_adjust_tail() to add some trailer to my packet.
This looks like it adds tailroom to the last frag? But, then
how do I insert my trailer? I don't think we can without the
extra multi-buffer access support right.

Also data_end will be unchanged yet it will return 0 so my
current programs will likely be a bit confused by this.

> + } else {