| Message ID | 20220617220738.3593-1-gospo@broadcom.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [net-next] samples/bpf: fixup some xdp progs to be able to support xdp multibuffer | expand |
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>
> 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 >
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
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 > > > >
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;
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(-)