diff mbox series

[v9,bpf-next,07/14] net: xdp: add multi-buff support to xdp_build_skb_from_frame

Message ID 7f61f8f7d38cf819383db739c14c874ccd3b53e2.1623674025.git.lorenzo@kernel.org
State New
Headers show
Series [v9,bpf-next,01/14] net: skbuff: add data_len field to skb_shared_info | expand

Commit Message

Lorenzo Bianconi June 14, 2021, 12:49 p.m. UTC
Introduce xdp multi-buff support to
__xdp_build_skb_from_frame/xdp_build_skb_from_frame
utility routines.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 net/core/xdp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Alexander Duyck June 28, 2021, 9:05 p.m. UTC | #1
On Mon, Jun 14, 2021 at 5:51 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>

> Introduce xdp multi-buff support to

> __xdp_build_skb_from_frame/xdp_build_skb_from_frame

> utility routines.

>

> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---

>  net/core/xdp.c | 13 +++++++++++++

>  1 file changed, 13 insertions(+)

>

> diff --git a/net/core/xdp.c b/net/core/xdp.c

> index f61c63115c95..71bedf6049a1 100644

> --- a/net/core/xdp.c

> +++ b/net/core/xdp.c

> @@ -582,9 +582,15 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,

>                                            struct sk_buff *skb,

>                                            struct net_device *dev)

>  {

> +       struct skb_shared_info *sinfo = xdp_get_shared_info_from_frame(xdpf);

>         unsigned int headroom, frame_size;

> +       int i, num_frags = 0;

>         void *hard_start;

>

> +       /* xdp multi-buff frame */

> +       if (unlikely(xdp_frame_is_mb(xdpf)))

> +               num_frags = sinfo->nr_frags;

> +

>         /* Part of headroom was reserved to xdpf */

>         headroom = sizeof(*xdpf) + xdpf->headroom;

>

> @@ -603,6 +609,13 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,

>         if (xdpf->metasize)

>                 skb_metadata_set(skb, xdpf->metasize);

>

> +       for (i = 0; i < num_frags; i++)

> +               skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,

> +                               skb_frag_page(&sinfo->frags[i]),

> +                               skb_frag_off(&sinfo->frags[i]),

> +                               skb_frag_size(&sinfo->frags[i]),

> +                               xdpf->frame_sz);

> +


So this is assuming the header frame and all of the frags are using
the same size. Rather than reading the frags out and then writing them
back, why not just directly rewrite the nr_frags, add the total size
to skb->len and skb->data_len, and then update the truesize?

Actually, I think you might need to store the truesize somewhere in
addition to the data_len that you were storing in the shared info.
Lorenzo Bianconi June 29, 2021, 6:34 p.m. UTC | #2
> On Mon, Jun 14, 2021 at 5:51 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> >

> > Introduce xdp multi-buff support to

> > __xdp_build_skb_from_frame/xdp_build_skb_from_frame

> > utility routines.

> >

> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

> > ---

> >  net/core/xdp.c | 13 +++++++++++++

> >  1 file changed, 13 insertions(+)

> >

> > diff --git a/net/core/xdp.c b/net/core/xdp.c

> > index f61c63115c95..71bedf6049a1 100644

> > --- a/net/core/xdp.c

> > +++ b/net/core/xdp.c

> > @@ -582,9 +582,15 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,

> >                                            struct sk_buff *skb,

> >                                            struct net_device *dev)

> >  {

> > +       struct skb_shared_info *sinfo = xdp_get_shared_info_from_frame(xdpf);

> >         unsigned int headroom, frame_size;

> > +       int i, num_frags = 0;

> >         void *hard_start;

> >

> > +       /* xdp multi-buff frame */

> > +       if (unlikely(xdp_frame_is_mb(xdpf)))

> > +               num_frags = sinfo->nr_frags;

> > +

> >         /* Part of headroom was reserved to xdpf */

> >         headroom = sizeof(*xdpf) + xdpf->headroom;

> >

> > @@ -603,6 +609,13 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,

> >         if (xdpf->metasize)

> >                 skb_metadata_set(skb, xdpf->metasize);

> >

> > +       for (i = 0; i < num_frags; i++)

> > +               skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,

> > +                               skb_frag_page(&sinfo->frags[i]),

> > +                               skb_frag_off(&sinfo->frags[i]),

> > +                               skb_frag_size(&sinfo->frags[i]),

> > +                               xdpf->frame_sz);

> > +

> 

> So this is assuming the header frame and all of the frags are using

> the same size. Rather than reading the frags out and then writing them

> back, why not just directly rewrite the nr_frags, add the total size

> to skb->len and skb->data_len, and then update the truesize?


ack, thx. I will look into it.

Regards,
Lorenzo

> 

> Actually, I think you might need to store the truesize somewhere in

> addition to the data_len that you were storing in the shared info.

>
diff mbox series

Patch

diff --git a/net/core/xdp.c b/net/core/xdp.c
index f61c63115c95..71bedf6049a1 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -582,9 +582,15 @@  struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
 					   struct sk_buff *skb,
 					   struct net_device *dev)
 {
+	struct skb_shared_info *sinfo = xdp_get_shared_info_from_frame(xdpf);
 	unsigned int headroom, frame_size;
+	int i, num_frags = 0;
 	void *hard_start;
 
+	/* xdp multi-buff frame */
+	if (unlikely(xdp_frame_is_mb(xdpf)))
+		num_frags = sinfo->nr_frags;
+
 	/* Part of headroom was reserved to xdpf */
 	headroom = sizeof(*xdpf) + xdpf->headroom;
 
@@ -603,6 +609,13 @@  struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
 	if (xdpf->metasize)
 		skb_metadata_set(skb, xdpf->metasize);
 
+	for (i = 0; i < num_frags; i++)
+		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+				skb_frag_page(&sinfo->frags[i]),
+				skb_frag_off(&sinfo->frags[i]),
+				skb_frag_size(&sinfo->frags[i]),
+				xdpf->frame_sz);
+
 	/* Essential SKB info: protocol and skb->dev */
 	skb->protocol = eth_type_trans(skb, dev);