[PATCH net-next] samples/bpf: fixup some xdp progs to be able to support xdp multibuffer

[PATCH net-next] samples/bpf: fixup some xdp progs to be able to support xdp multibuffer

[Andy Gospodarek]

[-- Attachment #1: Type: text/plain, Size: 2690 bytes --]

This changes the section name for the bpf program embedded in these
files to "xdp.frags" to allow the programs to be loaded on drivers that
are using an MTU greater than PAGE_SIZE.  Rather than directly accessing
the buffers, the packet data is now accessed via xdp helper functions to
provide an example for those who may need to write more complex
programs.

Signed-off-by: Andy Gospodarek <gospo@broadcom.com>
---
 samples/bpf/xdp1_kern.c            | 13 ++++++++++---
 samples/bpf/xdp2_kern.c            | 13 ++++++++++---
 samples/bpf/xdp_tx_iptunnel_kern.c |  2 +-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/samples/bpf/xdp1_kern.c b/samples/bpf/xdp1_kern.c
index f0c5d95084de..a798553fca3b 100644
--- a/samples/bpf/xdp1_kern.c
+++ b/samples/bpf/xdp1_kern.c
@@ -39,17 +39,24 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
 	return ip6h->nexthdr;
 }
 
-SEC("xdp1")
+#define XDPBUFSIZE	64
+SEC("xdp.frags")
 int xdp_prog1(struct xdp_md *ctx)
 {
-	void *data_end = (void *)(long)ctx->data_end;
-	void *data = (void *)(long)ctx->data;
+	__u8 pkt[XDPBUFSIZE] = {};
+	void *data_end = &pkt[XDPBUFSIZE-1];
+	void *data = pkt;
 	struct ethhdr *eth = data;
 	int rc = XDP_DROP;
 	long *value;
 	u16 h_proto;
 	u64 nh_off;
 	u32 ipproto;
+	int err;
+
+	err = bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt));
+	if (err < 0)
+		return rc;
 
 	nh_off = sizeof(*eth);
 	if (data + nh_off > data_end)
diff --git a/samples/bpf/xdp2_kern.c b/samples/bpf/xdp2_kern.c
index d8a64ab077b0..1502ef820aed 100644
--- a/samples/bpf/xdp2_kern.c
+++ b/samples/bpf/xdp2_kern.c
@@ -55,17 +55,24 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
 	return ip6h->nexthdr;
 }
 
-SEC("xdp1")
+#define XDPBUFSIZE	64
+SEC("xdp.frags")
 int xdp_prog1(struct xdp_md *ctx)
 {
-	void *data_end = (void *)(long)ctx->data_end;
-	void *data = (void *)(long)ctx->data;
+	__u8 pkt[XDPBUFSIZE] = {};
+	void *data_end = &pkt[XDPBUFSIZE-1];
+	void *data = pkt;
 	struct ethhdr *eth = data;
 	int rc = XDP_DROP;
 	long *value;
 	u16 h_proto;
 	u64 nh_off;
 	u32 ipproto;
+	int err;
+
+	err = bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt));
+	if (err < 0)
+		return rc;
 
 	nh_off = sizeof(*eth);
 	if (data + nh_off > data_end)
diff --git a/samples/bpf/xdp_tx_iptunnel_kern.c b/samples/bpf/xdp_tx_iptunnel_kern.c
index 575d57e4b8d6..0e2bca3a3fff 100644
--- a/samples/bpf/xdp_tx_iptunnel_kern.c
+++ b/samples/bpf/xdp_tx_iptunnel_kern.c
@@ -212,7 +212,7 @@ static __always_inline int handle_ipv6(struct xdp_md *xdp)
 	return XDP_TX;
 }
 
-SEC("xdp_tx_iptunnel")
+SEC("xdp.frags")
 int _xdp_tx_iptunnel(struct xdp_md *xdp)
 {
 	void *data_end = (void *)(long)xdp->data_end;
-- 
2.25.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4222 bytes --]

RE: [PATCH net-next] samples/bpf: fixup some xdp progs to be able to support xdp multibuffer

[John Fastabend]

Andy Gospodarek wrote:
> This changes the section name for the bpf program embedded in these
> files to "xdp.frags" to allow the programs to be loaded on drivers that
> are using an MTU greater than PAGE_SIZE.  Rather than directly accessing
> the buffers, the packet data is now accessed via xdp helper functions to
> provide an example for those who may need to write more complex
> programs.
> 
> Signed-off-by: Andy Gospodarek <gospo@broadcom.com>
> ---

OK. Although we lose the non frag example, but I guess that is fine and
highlights we don't maintain samples.

Acked-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH net-next] samples/bpf: fixup some xdp progs to be able to support xdp multibuffer

[Andy Gospodarek]

[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]

On Tue, Jun 21, 2022 at 08:51:34AM -0700, John Fastabend wrote:
> Andy Gospodarek wrote:
> > This changes the section name for the bpf program embedded in these
> > files to "xdp.frags" to allow the programs to be loaded on drivers that
> > are using an MTU greater than PAGE_SIZE.  Rather than directly accessing
> > the buffers, the packet data is now accessed via xdp helper functions to
> > provide an example for those who may need to write more complex
> > programs.
> > 
> > Signed-off-by: Andy Gospodarek <gospo@broadcom.com>
> > ---
> 
> OK. Although we lose the non frag example, but I guess that is fine and
> highlights we don't maintain samples.
> 
> Acked-by: John Fastabend <john.fastabend@gmail.com>

Thanks for taking a look, John.  My original changes had separate
functions for handling MB vs single buffer access.  I went with the
patch I posted for a few reasons:

- I liked the idea of one method for accessing the data via
  bpf_xdp_load_bytes -- even if that was less efficient for the
  single-buffer case.
- I did not love how much code it added nor what it looked like when I
  was done.
- Raw access of a non-frag xdp_buff is still available in
  xdp_rxq_info_kern.c if folks need an example of the most efficient way to
  benchmark an implementation.

-andy



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4222 bytes --]

Re: [PATCH net-next] samples/bpf: fixup some xdp progs to be able to support xdp multibuffer

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 3215 bytes --]

> This changes the section name for the bpf program embedded in these
> files to "xdp.frags" to allow the programs to be loaded on drivers that
> are using an MTU greater than PAGE_SIZE.  Rather than directly accessing
> the buffers, the packet data is now accessed via xdp helper functions to
> provide an example for those who may need to write more complex
> programs.
> 
> Signed-off-by: Andy Gospodarek <gospo@broadcom.com>

Hi Andy,

Just 2 nit inline but the code is fine.

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

> ---
>  samples/bpf/xdp1_kern.c            | 13 ++++++++++---
>  samples/bpf/xdp2_kern.c            | 13 ++++++++++---
>  samples/bpf/xdp_tx_iptunnel_kern.c |  2 +-
>  3 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/samples/bpf/xdp1_kern.c b/samples/bpf/xdp1_kern.c
> index f0c5d95084de..a798553fca3b 100644
> --- a/samples/bpf/xdp1_kern.c
> +++ b/samples/bpf/xdp1_kern.c
> @@ -39,17 +39,24 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
>  	return ip6h->nexthdr;
>  }
>  
> -SEC("xdp1")
> +#define XDPBUFSIZE	64
> +SEC("xdp.frags")
>  int xdp_prog1(struct xdp_md *ctx)
>  {
> -	void *data_end = (void *)(long)ctx->data_end;
> -	void *data = (void *)(long)ctx->data;
> +	__u8 pkt[XDPBUFSIZE] = {};
> +	void *data_end = &pkt[XDPBUFSIZE-1];
> +	void *data = pkt;
>  	struct ethhdr *eth = data;
>  	int rc = XDP_DROP;
>  	long *value;
>  	u16 h_proto;
>  	u64 nh_off;
>  	u32 ipproto;
> +	int err;
> +
> +	err = bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt));
> +	if (err < 0)
> +		return rc;

I guess we do not need err here:

	if (bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt)) < 0)
		return XDP_DROP;

>  
>  	nh_off = sizeof(*eth);
>  	if (data + nh_off > data_end)
> diff --git a/samples/bpf/xdp2_kern.c b/samples/bpf/xdp2_kern.c
> index d8a64ab077b0..1502ef820aed 100644
> --- a/samples/bpf/xdp2_kern.c
> +++ b/samples/bpf/xdp2_kern.c
> @@ -55,17 +55,24 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
>  	return ip6h->nexthdr;
>  }
>  
> -SEC("xdp1")
> +#define XDPBUFSIZE	64
> +SEC("xdp.frags")
>  int xdp_prog1(struct xdp_md *ctx)
>  {
> -	void *data_end = (void *)(long)ctx->data_end;
> -	void *data = (void *)(long)ctx->data;
> +	__u8 pkt[XDPBUFSIZE] = {};
> +	void *data_end = &pkt[XDPBUFSIZE-1];
> +	void *data = pkt;
>  	struct ethhdr *eth = data;
>  	int rc = XDP_DROP;
>  	long *value;
>  	u16 h_proto;
>  	u64 nh_off;
>  	u32 ipproto;
> +	int err;
> +
> +	err = bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt));
> +	if (err < 0)
> +		return rc;

same here

>  
>  	nh_off = sizeof(*eth);
>  	if (data + nh_off > data_end)
> diff --git a/samples/bpf/xdp_tx_iptunnel_kern.c b/samples/bpf/xdp_tx_iptunnel_kern.c
> index 575d57e4b8d6..0e2bca3a3fff 100644
> --- a/samples/bpf/xdp_tx_iptunnel_kern.c
> +++ b/samples/bpf/xdp_tx_iptunnel_kern.c
> @@ -212,7 +212,7 @@ static __always_inline int handle_ipv6(struct xdp_md *xdp)
>  	return XDP_TX;
>  }
>  
> -SEC("xdp_tx_iptunnel")
> +SEC("xdp.frags")
>  int _xdp_tx_iptunnel(struct xdp_md *xdp)
>  {
>  	void *data_end = (void *)(long)xdp->data_end;
> -- 
> 2.25.1
> 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next] samples/bpf: fixup some xdp progs to be able to support xdp multibuffer

[Andy Gospodarek]

[-- Attachment #1: Type: text/plain, Size: 3431 bytes --]

On Tue, Jun 21, 2022 at 06:25:59PM +0200, Lorenzo Bianconi wrote:
> > This changes the section name for the bpf program embedded in these
> > files to "xdp.frags" to allow the programs to be loaded on drivers that
> > are using an MTU greater than PAGE_SIZE.  Rather than directly accessing
> > the buffers, the packet data is now accessed via xdp helper functions to
> > provide an example for those who may need to write more complex
> > programs.
> > 
> > Signed-off-by: Andy Gospodarek <gospo@broadcom.com>
> 
> Hi Andy,
> 
> Just 2 nit inline but the code is fine.

Thanks, Lorenzo.  I'll re-spin now.


> Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> > ---
> >  samples/bpf/xdp1_kern.c            | 13 ++++++++++---
> >  samples/bpf/xdp2_kern.c            | 13 ++++++++++---
> >  samples/bpf/xdp_tx_iptunnel_kern.c |  2 +-
> >  3 files changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/samples/bpf/xdp1_kern.c b/samples/bpf/xdp1_kern.c
> > index f0c5d95084de..a798553fca3b 100644
> > --- a/samples/bpf/xdp1_kern.c
> > +++ b/samples/bpf/xdp1_kern.c
> > @@ -39,17 +39,24 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
> >  	return ip6h->nexthdr;
> >  }
> >  
> > -SEC("xdp1")
> > +#define XDPBUFSIZE	64
> > +SEC("xdp.frags")
> >  int xdp_prog1(struct xdp_md *ctx)
> >  {
> > -	void *data_end = (void *)(long)ctx->data_end;
> > -	void *data = (void *)(long)ctx->data;
> > +	__u8 pkt[XDPBUFSIZE] = {};
> > +	void *data_end = &pkt[XDPBUFSIZE-1];
> > +	void *data = pkt;
> >  	struct ethhdr *eth = data;
> >  	int rc = XDP_DROP;
> >  	long *value;
> >  	u16 h_proto;
> >  	u64 nh_off;
> >  	u32 ipproto;
> > +	int err;
> > +
> > +	err = bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt));
> > +	if (err < 0)
> > +		return rc;
> 
> I guess we do not need err here:
> 
> 	if (bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt)) < 0)
> 		return XDP_DROP;
> 
> >  
> >  	nh_off = sizeof(*eth);
> >  	if (data + nh_off > data_end)
> > diff --git a/samples/bpf/xdp2_kern.c b/samples/bpf/xdp2_kern.c
> > index d8a64ab077b0..1502ef820aed 100644
> > --- a/samples/bpf/xdp2_kern.c
> > +++ b/samples/bpf/xdp2_kern.c
> > @@ -55,17 +55,24 @@ static int parse_ipv6(void *data, u64 nh_off, void *data_end)
> >  	return ip6h->nexthdr;
> >  }
> >  
> > -SEC("xdp1")
> > +#define XDPBUFSIZE	64
> > +SEC("xdp.frags")
> >  int xdp_prog1(struct xdp_md *ctx)
> >  {
> > -	void *data_end = (void *)(long)ctx->data_end;
> > -	void *data = (void *)(long)ctx->data;
> > +	__u8 pkt[XDPBUFSIZE] = {};
> > +	void *data_end = &pkt[XDPBUFSIZE-1];
> > +	void *data = pkt;
> >  	struct ethhdr *eth = data;
> >  	int rc = XDP_DROP;
> >  	long *value;
> >  	u16 h_proto;
> >  	u64 nh_off;
> >  	u32 ipproto;
> > +	int err;
> > +
> > +	err = bpf_xdp_load_bytes(ctx, 0, pkt, sizeof(pkt));
> > +	if (err < 0)
> > +		return rc;
> 
> same here
> 
> >  
> >  	nh_off = sizeof(*eth);
> >  	if (data + nh_off > data_end)
> > diff --git a/samples/bpf/xdp_tx_iptunnel_kern.c b/samples/bpf/xdp_tx_iptunnel_kern.c
> > index 575d57e4b8d6..0e2bca3a3fff 100644
> > --- a/samples/bpf/xdp_tx_iptunnel_kern.c
> > +++ b/samples/bpf/xdp_tx_iptunnel_kern.c
> > @@ -212,7 +212,7 @@ static __always_inline int handle_ipv6(struct xdp_md *xdp)
> >  	return XDP_TX;
> >  }
> >  
> > -SEC("xdp_tx_iptunnel")
> > +SEC("xdp.frags")
> >  int _xdp_tx_iptunnel(struct xdp_md *xdp)
> >  {
> >  	void *data_end = (void *)(long)xdp->data_end;
> > -- 
> > 2.25.1
> > 
> 
> 



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4222 bytes --]