> Lorenzo Bianconi <lorenzo@xxxxxxxxxx> writes:
>
> > Similar to native xdp, do not always linearize the skb in
> > netif_receive_generic_xdp routine but create a non-linear xdp_buff to be
> > processed by the eBPF program. This allow to add multi-buffer support
> > for xdp running in generic mode.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
>
> With one nit below:
>
> Reviewed-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
>
> > ---
> > net/core/dev.c | 28 +++++++++++++++++++++++-----
> > 1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 3950ced396b5..5a58f3e28657 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4853,6 +4853,12 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
> > xdp_init_buff(xdp, frame_sz, &rxqueue->xdp_rxq);
> > xdp_prepare_buff(xdp, hard_start, skb_headroom(skb) - mac_len,
> > skb_headlen(skb) + mac_len, true);
> > + if (skb_is_nonlinear(skb)) {
> > + skb_shinfo(skb)->xdp_frags_size = skb->data_len;
> > + xdp_buff_set_frags_flag(xdp);
> > + } else {
> > + xdp_buff_clear_frags_flag(xdp);
> > + }
> >
> > orig_data_end = xdp->data_end;
> > orig_data = xdp->data;
> > @@ -4882,6 +4888,14 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
> > skb->len += off; /* positive on grow, negative on shrink */
> > }
> >
> > + /* XDP frag metadata (e.g. nr_frags) are updated in eBPF helpers
> > + * (e.g. bpf_xdp_adjust_tail), we need to update data_len here.
> > + */
> > + if (xdp_buff_has_frags(xdp))
> > + skb->data_len = skb_shinfo(skb)->xdp_frags_size;
> > + else
> > + skb->data_len = 0;
> > +
> > /* check if XDP changed eth hdr such SKB needs update */
> > eth = (struct ethhdr *)xdp->data;
> > if ((orig_eth_type != eth->h_proto) ||
> > @@ -4927,9 +4941,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> > if (skb_is_redirected(skb))
> > return XDP_PASS;
> >
> > - /* XDP packets must be linear and must have sufficient headroom
> > - * of XDP_PACKET_HEADROOM bytes. This is the guarantee that also
> > - * native XDP provides, thus we need to do it here as well.
> > + /* XDP packets must have sufficient headroom of XDP_PACKET_HEADROOM
> > + * bytes. This is the guarantee that also native XDP provides,
> > + * thus we need to do it here as well.
> > */
> > if (skb_cloned(skb) || skb_is_nonlinear(skb) ||
> > skb_headroom(skb) < XDP_PACKET_HEADROOM) {
> > @@ -4943,8 +4957,12 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> > hroom > 0 ? ALIGN(hroom, NET_SKB_PAD) : 0,
> > troom > 0 ? troom + 128 : 0, GFP_ATOMIC))
> > goto do_drop;
>
> There's a comment above the pskb_expand_head() call that isn't quite
> part of the context here, that should also be adjusted now that we don't
> always linearise:
>
> /* In case we have to go down the path and also linearize,
> * then lets do the pskb_expand_head() work just once here.
> */
>
> Actually, I think maybe just dropping that comment entirely with this
> change would make sense?
ack, I will get rid of it. Thx.
Regards,
Lorenzo
>
>
> > - if (skb_linearize(skb))
> > - goto do_drop;
> > +
> > + /* XDP does not support fraglist */
> > + if (skb_has_frag_list(skb) || !xdp_prog->aux->xdp_has_frags) {
> > + if (skb_linearize(skb))
> > + goto do_drop;
> > + }
> > }
> >
> > act = bpf_prog_run_generic_xdp(skb, xdp, xdp_prog);
> > --
> > 2.43.0
>
Attachment:
signature.asc
Description: PGP signature
