diff mbox series

[v3,net-next,06/12] bpf: helpers: add multibuffer support

Message ID 5e248485713d2470d97f36ad67c9b3ceedfc2b3f.1601478613.git.lorenzo@kernel.org
State New
Headers show
Series mvneta: introduce XDP multi-buffer support | expand

Commit Message

Lorenzo Bianconi Sept. 30, 2020, 3:41 p.m. UTC 
From: Sameeh Jubran <sameehj@amazon.com>

The implementation is based on this [0] draft by Jesper D. Brouer.

Provided two new helpers:

* bpf_xdp_get_frag_count()
* bpf_xdp_get_frags_total_size()

[0] xdp mb design - https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 include/uapi/linux/bpf.h       | 14 ++++++++++++
 net/core/filter.c              | 42 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 14 ++++++++++++
 3 files changed, 70 insertions(+)

Comments

Alexei Starovoitov Sept. 30, 2020, 7:11 p.m. UTC | #1 
On Wed, Sep 30, 2020 at 05:41:57PM +0200, Lorenzo Bianconi wrote:
> From: Sameeh Jubran <sameehj@amazon.com>
> 
> The implementation is based on this [0] draft by Jesper D. Brouer.
> 
> Provided two new helpers:
> 
> * bpf_xdp_get_frag_count()
> * bpf_xdp_get_frags_total_size()
> 
> [0] xdp mb design - https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
> Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
> Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  include/uapi/linux/bpf.h       | 14 ++++++++++++
>  net/core/filter.c              | 42 ++++++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h | 14 ++++++++++++
>  3 files changed, 70 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index a22812561064..6f97dce8cccf 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3586,6 +3586,18 @@ union bpf_attr {
>   * 		the data in *dst*. This is a wrapper of **copy_from_user**\ ().
>   * 	Return
>   * 		0 on success, or a negative error in case of failure.
> + *
> + * int bpf_xdp_get_frag_count(struct xdp_buff *xdp_md)
> + *	Description
> + *		Get the number of fragments for a given xdp multi-buffer.
> + *	Return
> + *		The number of fragments
> + *
> + * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)
> + *	Description
> + *		Get the total size of fragments for a given xdp multi-buffer.
> + *	Return
> + *		The total size of fragments for a given xdp multi-buffer.
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -3737,6 +3749,8 @@ union bpf_attr {
>  	FN(inode_storage_delete),	\
>  	FN(d_path),			\
>  	FN(copy_from_user),		\
> +	FN(xdp_get_frag_count),		\
> +	FN(xdp_get_frags_total_size),	\
>  	/* */

Please route the set via bpf-next otherwise merge conflicts will be severe.
Jesper Dangaard Brouer Oct. 1, 2020, 9:47 a.m. UTC | #2 
On Wed, 30 Sep 2020 12:11:21 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Wed, Sep 30, 2020 at 05:41:57PM +0200, Lorenzo Bianconi wrote:
> > From: Sameeh Jubran <sameehj@amazon.com>
> > 
> > The implementation is based on this [0] draft by Jesper D. Brouer.

First of all I think you are giving me too much credit, and this is
both not really relevant and also not specific enough.  The link[0]
contains several proposals (actually from different people) and it is
not clear which of these proposal you reference.

I think this patch need to explain and argue why these BPF-helpers
makes sense... this will become BPF UAPI.

> > Provided two new helpers:
> > 
> > * bpf_xdp_get_frag_count()
> > * bpf_xdp_get_frags_total_size()

Why was the "frag" and "frags" name chosen?

 
> > [0] xdp mb design - https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
> > Signed-off-by: Sameeh Jubran <sameehj@amazon.com>
> > Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  include/uapi/linux/bpf.h       | 14 ++++++++++++
> >  net/core/filter.c              | 42 ++++++++++++++++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h | 14 ++++++++++++
> >  3 files changed, 70 insertions(+)
> > 
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index a22812561064..6f97dce8cccf 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -3586,6 +3586,18 @@ union bpf_attr {
> >   * 		the data in *dst*. This is a wrapper of **copy_from_user**\ ().
> >   * 	Return
> >   * 		0 on success, or a negative error in case of failure.
> > + *
> > + * int bpf_xdp_get_frag_count(struct xdp_buff *xdp_md)
> > + *	Description
> > + *		Get the number of fragments for a given xdp multi-buffer.
> > + *	Return
> > + *		The number of fragments
> > + *
> > + * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)
> > + *	Description
> > + *		Get the total size of fragments for a given xdp multi-buffer.
> > + *	Return
> > + *		The total size of fragments for a given xdp multi-buffer.
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -3737,6 +3749,8 @@ union bpf_attr {
> >  	FN(inode_storage_delete),	\
> >  	FN(d_path),			\
> >  	FN(copy_from_user),		\
> > +	FN(xdp_get_frag_count),		\
> > +	FN(xdp_get_frags_total_size),	\
> >  	/* */  
> 
> Please route the set via bpf-next otherwise merge conflicts will be severe.
>
Lorenzo Bianconi Oct. 1, 2020, 3:05 p.m. UTC | #3 
> On Wed, Sep 30, 2020 at 05:41:57PM +0200, Lorenzo Bianconi wrote:


Hi Alexei,

> > From: Sameeh Jubran <sameehj@amazon.com>

> > 

> > The implementation is based on this [0] draft by Jesper D. Brouer.

> > 

> > Provided two new helpers:

> > 

> > * bpf_xdp_get_frag_count()

> > * bpf_xdp_get_frags_total_size()

> > 

> > + * int bpf_xdp_get_frag_count(struct xdp_buff *xdp_md)

> > + *	Description

> > + *		Get the number of fragments for a given xdp multi-buffer.

> > + *	Return

> > + *		The number of fragments

> > + *

> > + * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)

> > + *	Description

> > + *		Get the total size of fragments for a given xdp multi-buffer.

> > + *	Return

> > + *		The total size of fragments for a given xdp multi-buffer.

> >   */

> >  #define __BPF_FUNC_MAPPER(FN)		\

> >  	FN(unspec),			\

> > @@ -3737,6 +3749,8 @@ union bpf_attr {

> >  	FN(inode_storage_delete),	\

> >  	FN(d_path),			\

> >  	FN(copy_from_user),		\

> > +	FN(xdp_get_frag_count),		\

> > +	FN(xdp_get_frags_total_size),	\

> >  	/* */

> 

> Please route the set via bpf-next otherwise merge conflicts will be severe.


ack, fine

in bpf-next the following two commits (available in net-next) are currently missing:
- 632bb64f126a: net: mvneta: try to use in-irq pp cache in mvneta_txq_bufs_free
- 879456bedbe5: net: mvneta: avoid possible cache misses in mvneta_rx_swbm

is it ok to rebase bpf-next ontop of net-next in order to post all the series
in bpf-next? Or do you prefer to post mvneta patches in net-next and bpf
related changes in bpf-next when it will rebased ontop of net-next?

Regards,
Lorenzo
Alexei Starovoitov Oct. 1, 2020, 3:40 p.m. UTC | #4 
On Thu, Oct 1, 2020 at 8:05 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>

> > On Wed, Sep 30, 2020 at 05:41:57PM +0200, Lorenzo Bianconi wrote:

>

> Hi Alexei,

>

> > > From: Sameeh Jubran <sameehj@amazon.com>

> > >

> > > The implementation is based on this [0] draft by Jesper D. Brouer.

> > >

> > > Provided two new helpers:

> > >

> > > * bpf_xdp_get_frag_count()

> > > * bpf_xdp_get_frags_total_size()

> > >

> > > + * int bpf_xdp_get_frag_count(struct xdp_buff *xdp_md)

> > > + * Description

> > > + *         Get the number of fragments for a given xdp multi-buffer.

> > > + * Return

> > > + *         The number of fragments

> > > + *

> > > + * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)

> > > + * Description

> > > + *         Get the total size of fragments for a given xdp multi-buffer.

> > > + * Return

> > > + *         The total size of fragments for a given xdp multi-buffer.

> > >   */

> > >  #define __BPF_FUNC_MAPPER(FN)              \

> > >     FN(unspec),                     \

> > > @@ -3737,6 +3749,8 @@ union bpf_attr {

> > >     FN(inode_storage_delete),       \

> > >     FN(d_path),                     \

> > >     FN(copy_from_user),             \

> > > +   FN(xdp_get_frag_count),         \

> > > +   FN(xdp_get_frags_total_size),   \

> > >     /* */

> >

> > Please route the set via bpf-next otherwise merge conflicts will be severe.

>

> ack, fine

>

> in bpf-next the following two commits (available in net-next) are currently missing:

> - 632bb64f126a: net: mvneta: try to use in-irq pp cache in mvneta_txq_bufs_free

> - 879456bedbe5: net: mvneta: avoid possible cache misses in mvneta_rx_swbm

>

> is it ok to rebase bpf-next ontop of net-next in order to post all the series

> in bpf-next? Or do you prefer to post mvneta patches in net-next and bpf

> related changes in bpf-next when it will rebased ontop of net-next?


bpf-next will receive these patches later today,
so I prefer the whole thing on top of bpf-next at that time.
Lorenzo Bianconi Oct. 1, 2020, 3:44 p.m. UTC | #5 
[...]
> > >
> > > Please route the set via bpf-next otherwise merge conflicts will be severe.
> >
> > ack, fine
> >
> > in bpf-next the following two commits (available in net-next) are currently missing:
> > - 632bb64f126a: net: mvneta: try to use in-irq pp cache in mvneta_txq_bufs_free
> > - 879456bedbe5: net: mvneta: avoid possible cache misses in mvneta_rx_swbm
> >
> > is it ok to rebase bpf-next ontop of net-next in order to post all the series
> > in bpf-next? Or do you prefer to post mvneta patches in net-next and bpf
> > related changes in bpf-next when it will rebased ontop of net-next?
> 
> bpf-next will receive these patches later today,
> so I prefer the whole thing on top of bpf-next at that time.

sounds good, thx.

Regards,
Lorenzo
diff mbox series

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a22812561064..6f97dce8cccf 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3586,6 +3586,18 @@  union bpf_attr {
  * 		the data in *dst*. This is a wrapper of **copy_from_user**\ ().
  * 	Return
  * 		0 on success, or a negative error in case of failure.
+ *
+ * int bpf_xdp_get_frag_count(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the number of fragments for a given xdp multi-buffer.
+ *	Return
+ *		The number of fragments
+ *
+ * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the total size of fragments for a given xdp multi-buffer.
+ *	Return
+ *		The total size of fragments for a given xdp multi-buffer.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3737,6 +3749,8 @@  union bpf_attr {
 	FN(inode_storage_delete),	\
 	FN(d_path),			\
 	FN(copy_from_user),		\
+	FN(xdp_get_frag_count),		\
+	FN(xdp_get_frags_total_size),	\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/net/core/filter.c b/net/core/filter.c
index 706f8db0ccf8..7f33cfae219c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3475,6 +3475,44 @@  static const struct bpf_func_proto bpf_xdp_adjust_head_proto = {
 	.arg2_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_1(bpf_xdp_get_frag_count, struct  xdp_buff*, xdp)
+{
+	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
+
+	return xdp->mb ? sinfo->nr_frags : 0;
+}
+
+const struct bpf_func_proto bpf_xdp_get_frag_count_proto = {
+	.func		= bpf_xdp_get_frag_count,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
+BPF_CALL_1(bpf_xdp_get_frags_total_size, struct  xdp_buff*, xdp)
+{
+	struct skb_shared_info *sinfo;
+	int nfrags, i, size = 0;
+
+	if (likely(!xdp->mb))
+		return 0;
+
+	sinfo = xdp_get_shared_info_from_buff(xdp);
+	nfrags = min_t(u8, sinfo->nr_frags, MAX_SKB_FRAGS);
+
+	for (i = 0; i < nfrags; i++)
+		size += skb_frag_size(&sinfo->frags[i]);
+
+	return size;
+}
+
+const struct bpf_func_proto bpf_xdp_get_frags_total_size_proto = {
+	.func		= bpf_xdp_get_frags_total_size,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
 BPF_CALL_2(bpf_xdp_adjust_tail, struct xdp_buff *, xdp, int, offset)
 {
 	void *data_hard_end = xdp_data_hard_end(xdp); /* use xdp->frame_sz */
@@ -6824,6 +6862,10 @@  xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_xdp_redirect_map_proto;
 	case BPF_FUNC_xdp_adjust_tail:
 		return &bpf_xdp_adjust_tail_proto;
+	case BPF_FUNC_xdp_get_frag_count:
+		return &bpf_xdp_get_frag_count_proto;
+	case BPF_FUNC_xdp_get_frags_total_size:
+		return &bpf_xdp_get_frags_total_size_proto;
 	case BPF_FUNC_fib_lookup:
 		return &bpf_xdp_fib_lookup_proto;
 #ifdef CONFIG_INET
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a22812561064..6f97dce8cccf 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3586,6 +3586,18 @@  union bpf_attr {
  * 		the data in *dst*. This is a wrapper of **copy_from_user**\ ().
  * 	Return
  * 		0 on success, or a negative error in case of failure.
+ *
+ * int bpf_xdp_get_frag_count(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the number of fragments for a given xdp multi-buffer.
+ *	Return
+ *		The number of fragments
+ *
+ * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the total size of fragments for a given xdp multi-buffer.
+ *	Return
+ *		The total size of fragments for a given xdp multi-buffer.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3737,6 +3749,8 @@  union bpf_attr {
 	FN(inode_storage_delete),	\
 	FN(d_path),			\
 	FN(copy_from_user),		\
+	FN(xdp_get_frag_count),		\
+	FN(xdp_get_frags_total_size),	\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper