| Message ID | 155508165793.23650.5590462213777770722.stgit@firesoul |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Bulk optimization for XDP cpumap redirect | expand |
On Fri, Apr 12, 2019 at 8:08 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote: > > The function build_skb() also have the responsibility to allocate and clear > the SKB structure. Introduce a new function build_skb_around(), that moves > the responsibility of allocation and clearing to the caller. This allows > caller to use kmem_cache (slab/slub) bulk allocation API. > > Next patch use this function combined with kmem_cache_alloc_bulk. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> Acked-by: Song Liu <songliubraving@fb.com> > --- > include/linux/skbuff.h | 2 + > net/core/skbuff.c | 71 +++++++++++++++++++++++++++++++++++------------- > 2 files changed, 54 insertions(+), 19 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index a06275a618f0..e81f2b0e8a83 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -1042,6 +1042,8 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t priority, int flags, > int node); > struct sk_buff *__build_skb(void *data, unsigned int frag_size); > struct sk_buff *build_skb(void *data, unsigned int frag_size); > +struct sk_buff *build_skb_around(struct sk_buff *skb, > + void *data, unsigned int frag_size); > > /** > * alloc_skb - allocate a network buffer > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 9901f5322852..087622298d77 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -258,6 +258,33 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, > } > EXPORT_SYMBOL(__alloc_skb); > > +/* Caller must provide SKB that is memset cleared */ > +static struct sk_buff *__build_skb_around(struct sk_buff *skb, > + void *data, unsigned int frag_size) > +{ > + struct skb_shared_info *shinfo; > + unsigned int size = frag_size ? : ksize(data); > + > + size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > + > + /* Assumes caller memset cleared SKB */ > + skb->truesize = SKB_TRUESIZE(size); > + refcount_set(&skb->users, 1); > + skb->head = data; > + skb->data = data; > + skb_reset_tail_pointer(skb); > + skb->end = skb->tail + size; > + skb->mac_header = (typeof(skb->mac_header))~0U; > + skb->transport_header = (typeof(skb->transport_header))~0U; > + > + /* make sure we initialize shinfo sequentially */ > + shinfo = skb_shinfo(skb); > + memset(shinfo, 0, offsetof(struct skb_shared_info, dataref)); > + atomic_set(&shinfo->dataref, 1); > + > + return skb; > +} > + > /** > * __build_skb - build a network buffer > * @data: data buffer provided by caller > @@ -279,32 +306,15 @@ EXPORT_SYMBOL(__alloc_skb); > */ > struct sk_buff *__build_skb(void *data, unsigned int frag_size) > { > - struct skb_shared_info *shinfo; > struct sk_buff *skb; > - unsigned int size = frag_size ? : ksize(data); > > skb = kmem_cache_alloc(skbuff_head_cache, GFP_ATOMIC); > - if (!skb) > + if (unlikely(!skb)) > return NULL; > > - size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > - > memset(skb, 0, offsetof(struct sk_buff, tail)); > - skb->truesize = SKB_TRUESIZE(size); > - refcount_set(&skb->users, 1); > - skb->head = data; > - skb->data = data; > - skb_reset_tail_pointer(skb); > - skb->end = skb->tail + size; > - skb->mac_header = (typeof(skb->mac_header))~0U; > - skb->transport_header = (typeof(skb->transport_header))~0U; > > - /* make sure we initialize shinfo sequentially */ > - shinfo = skb_shinfo(skb); > - memset(shinfo, 0, offsetof(struct skb_shared_info, dataref)); > - atomic_set(&shinfo->dataref, 1); > - > - return skb; > + return __build_skb_around(skb, data, frag_size); > } > > /* build_skb() is wrapper over __build_skb(), that specifically > @@ -325,6 +335,29 @@ struct sk_buff *build_skb(void *data, unsigned int frag_size) > } > EXPORT_SYMBOL(build_skb); > > +/** > + * build_skb_around - build a network buffer around provided skb > + * @skb: sk_buff provide by caller, must be memset cleared > + * @data: data buffer provided by caller > + * @frag_size: size of data, or 0 if head was kmalloced > + */ > +struct sk_buff *build_skb_around(struct sk_buff *skb, > + void *data, unsigned int frag_size) > +{ > + if (unlikely(!skb)) > + return NULL; > + > + skb = __build_skb_around(skb, data, frag_size); > + > + if (skb && frag_size) { > + skb->head_frag = 1; > + if (page_is_pfmemalloc(virt_to_head_page(data))) > + skb->pfmemalloc = 1; > + } > + return skb; > +} > +EXPORT_SYMBOL(build_skb_around); > + > #define NAPI_SKB_CACHE_SIZE 64 > > struct napi_alloc_cache { >
On Fri, Apr 12, 2019 at 8:07 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote: > > The function build_skb() also have the responsibility to allocate and clear > the SKB structure. Introduce a new function build_skb_around(), that moves > the responsibility of allocation and clearing to the caller. This allows > caller to use kmem_cache (slab/slub) bulk allocation API. > > Next patch use this function combined with kmem_cache_alloc_bulk. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > include/linux/skbuff.h | 2 + > net/core/skbuff.c | 71 +++++++++++++++++++++++++++++++++++------------- > 2 files changed, 54 insertions(+), 19 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index a06275a618f0..e81f2b0e8a83 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -1042,6 +1042,8 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t priority, int flags, > int node); > struct sk_buff *__build_skb(void *data, unsigned int frag_size); > struct sk_buff *build_skb(void *data, unsigned int frag_size); > +struct sk_buff *build_skb_around(struct sk_buff *skb, > + void *data, unsigned int frag_size); > > /** > * alloc_skb - allocate a network buffer > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 9901f5322852..087622298d77 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -258,6 +258,33 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, > } > EXPORT_SYMBOL(__alloc_skb); > > +/* Caller must provide SKB that is memset cleared */ > +static struct sk_buff *__build_skb_around(struct sk_buff *skb, > + void *data, unsigned int frag_size) > +{ > + struct skb_shared_info *shinfo; > + unsigned int size = frag_size ? : ksize(data); > + > + size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > + > + /* Assumes caller memset cleared SKB */ > + skb->truesize = SKB_TRUESIZE(size); > + refcount_set(&skb->users, 1); > + skb->head = data; > + skb->data = data; > + skb_reset_tail_pointer(skb); > + skb->end = skb->tail + size; > + skb->mac_header = (typeof(skb->mac_header))~0U; > + skb->transport_header = (typeof(skb->transport_header))~0U; > + > + /* make sure we initialize shinfo sequentially */ > + shinfo = skb_shinfo(skb); > + memset(shinfo, 0, offsetof(struct skb_shared_info, dataref)); > + atomic_set(&shinfo->dataref, 1); > + > + return skb; > +} > + > /** > * __build_skb - build a network buffer > * @data: data buffer provided by caller > @@ -279,32 +306,15 @@ EXPORT_SYMBOL(__alloc_skb); > */ > struct sk_buff *__build_skb(void *data, unsigned int frag_size) > { > - struct skb_shared_info *shinfo; > struct sk_buff *skb; > - unsigned int size = frag_size ? : ksize(data); > > skb = kmem_cache_alloc(skbuff_head_cache, GFP_ATOMIC); > - if (!skb) > + if (unlikely(!skb)) > return NULL; > > - size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > - > memset(skb, 0, offsetof(struct sk_buff, tail)); > - skb->truesize = SKB_TRUESIZE(size); > - refcount_set(&skb->users, 1); > - skb->head = data; > - skb->data = data; > - skb_reset_tail_pointer(skb); > - skb->end = skb->tail + size; > - skb->mac_header = (typeof(skb->mac_header))~0U; > - skb->transport_header = (typeof(skb->transport_header))~0U; > > - /* make sure we initialize shinfo sequentially */ > - shinfo = skb_shinfo(skb); > - memset(shinfo, 0, offsetof(struct skb_shared_info, dataref)); > - atomic_set(&shinfo->dataref, 1); > - > - return skb; > + return __build_skb_around(skb, data, frag_size); > } It looks good to me, but I'd like to see Eric's Ack before applying.
On Tue, Apr 16, 2019 at 8:01 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > It looks good to me, but I'd like to see Eric's Ack before applying. This seems just fine, thanks ! Acked-by: Eric Dumazet <edumazet@google.com>
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index a06275a618f0..e81f2b0e8a83 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1042,6 +1042,8 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t priority, int flags, int node); struct sk_buff *__build_skb(void *data, unsigned int frag_size); struct sk_buff *build_skb(void *data, unsigned int frag_size); +struct sk_buff *build_skb_around(struct sk_buff *skb, + void *data, unsigned int frag_size); /** * alloc_skb - allocate a network buffer diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 9901f5322852..087622298d77 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -258,6 +258,33 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, } EXPORT_SYMBOL(__alloc_skb); +/* Caller must provide SKB that is memset cleared */ +static struct sk_buff *__build_skb_around(struct sk_buff *skb, + void *data, unsigned int frag_size) +{ + struct skb_shared_info *shinfo; + unsigned int size = frag_size ? : ksize(data); + + size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); + + /* Assumes caller memset cleared SKB */ + skb->truesize = SKB_TRUESIZE(size); + refcount_set(&skb->users, 1); + skb->head = data; + skb->data = data; + skb_reset_tail_pointer(skb); + skb->end = skb->tail + size; + skb->mac_header = (typeof(skb->mac_header))~0U; + skb->transport_header = (typeof(skb->transport_header))~0U; + + /* make sure we initialize shinfo sequentially */ + shinfo = skb_shinfo(skb); + memset(shinfo, 0, offsetof(struct skb_shared_info, dataref)); + atomic_set(&shinfo->dataref, 1); + + return skb; +} + /** * __build_skb - build a network buffer * @data: data buffer provided by caller @@ -279,32 +306,15 @@ EXPORT_SYMBOL(__alloc_skb); */ struct sk_buff *__build_skb(void *data, unsigned int frag_size) { - struct skb_shared_info *shinfo; struct sk_buff *skb; - unsigned int size = frag_size ? : ksize(data); skb = kmem_cache_alloc(skbuff_head_cache, GFP_ATOMIC); - if (!skb) + if (unlikely(!skb)) return NULL; - size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); - memset(skb, 0, offsetof(struct sk_buff, tail)); - skb->truesize = SKB_TRUESIZE(size); - refcount_set(&skb->users, 1); - skb->head = data; - skb->data = data; - skb_reset_tail_pointer(skb); - skb->end = skb->tail + size; - skb->mac_header = (typeof(skb->mac_header))~0U; - skb->transport_header = (typeof(skb->transport_header))~0U; - /* make sure we initialize shinfo sequentially */ - shinfo = skb_shinfo(skb); - memset(shinfo, 0, offsetof(struct skb_shared_info, dataref)); - atomic_set(&shinfo->dataref, 1); - - return skb; + return __build_skb_around(skb, data, frag_size); } /* build_skb() is wrapper over __build_skb(), that specifically @@ -325,6 +335,29 @@ struct sk_buff *build_skb(void *data, unsigned int frag_size) } EXPORT_SYMBOL(build_skb); +/** + * build_skb_around - build a network buffer around provided skb + * @skb: sk_buff provide by caller, must be memset cleared + * @data: data buffer provided by caller + * @frag_size: size of data, or 0 if head was kmalloced + */ +struct sk_buff *build_skb_around(struct sk_buff *skb, + void *data, unsigned int frag_size) +{ + if (unlikely(!skb)) + return NULL; + + skb = __build_skb_around(skb, data, frag_size); + + if (skb && frag_size) { + skb->head_frag = 1; + if (page_is_pfmemalloc(virt_to_head_page(data))) + skb->pfmemalloc = 1; + } + return skb; +} +EXPORT_SYMBOL(build_skb_around); + #define NAPI_SKB_CACHE_SIZE 64 struct napi_alloc_cache {
The function build_skb() also have the responsibility to allocate and clear the SKB structure. Introduce a new function build_skb_around(), that moves the responsibility of allocation and clearing to the caller. This allows caller to use kmem_cache (slab/slub) bulk allocation API. Next patch use this function combined with kmem_cache_alloc_bulk. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- include/linux/skbuff.h | 2 + net/core/skbuff.c | 71 +++++++++++++++++++++++++++++++++++------------- 2 files changed, 54 insertions(+), 19 deletions(-)