| Message ID | 158757164613.1370371.2655437650342381672.stgit@firesoul |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [net-next,01/33] xdp: add frame size to xdp_buff | expand |
Jesper Dangaard Brouer <brouer@redhat.com> writes: > XDP have evolved to support several frame sizes, but xdp_buff was not > updated with this information. The frame size (frame_sz) member of > xdp_buff is introduced to know the real size of the memory the frame is > delivered in. > > When introducing this also make it clear that some tailroom is > reserved/required when creating SKBs using build_skb(). > > It would also have been an option to introduce a pointer to > data_hard_end (with reserved offset). The advantage with frame_sz is > that (like rxq) drivers only need to setup/assign this value once per > NAPI cycle. Due to XDP-generic (and some drivers) it's not possible to > store frame_sz inside xdp_rxq_info, because it's varies per packet as it > can be based/depend on packet length. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> With one possible nit below: Acked-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > include/net/xdp.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/net/xdp.h b/include/net/xdp.h > index 40c6d3398458..1ccf7df98bee 100644 > --- a/include/net/xdp.h > +++ b/include/net/xdp.h > @@ -6,6 +6,8 @@ > #ifndef __LINUX_NET_XDP_H__ > #define __LINUX_NET_XDP_H__ > > +#include <linux/skbuff.h> /* skb_shared_info */ > + > /** > * DOC: XDP RX-queue information > * > @@ -70,8 +72,19 @@ struct xdp_buff { > void *data_hard_start; > unsigned long handle; > struct xdp_rxq_info *rxq; > + u32 frame_sz; /* frame size to deduct data_hard_end/reserved tailroom*/ I think maybe you want to s/deduct/deduce/ here? -Toke
On Fri, 24 Apr 2020 16:00:53 +0200 Toke Høiland-Jørgensen <toke@redhat.com> wrote: > Jesper Dangaard Brouer <brouer@redhat.com> writes: > > > XDP have evolved to support several frame sizes, but xdp_buff was not > > updated with this information. The frame size (frame_sz) member of > > xdp_buff is introduced to know the real size of the memory the frame is > > delivered in. > > > > When introducing this also make it clear that some tailroom is > > reserved/required when creating SKBs using build_skb(). > > > > It would also have been an option to introduce a pointer to > > data_hard_end (with reserved offset). The advantage with frame_sz is > > that (like rxq) drivers only need to setup/assign this value once per > > NAPI cycle. Due to XDP-generic (and some drivers) it's not possible to > > store frame_sz inside xdp_rxq_info, because it's varies per packet as it > > can be based/depend on packet length. > > > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > > With one possible nit below: > > Acked-by: Toke Høiland-Jørgensen <toke@redhat.com> thx > > --- > > include/net/xdp.h | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/include/net/xdp.h b/include/net/xdp.h > > index 40c6d3398458..1ccf7df98bee 100644 > > --- a/include/net/xdp.h > > +++ b/include/net/xdp.h > > @@ -6,6 +6,8 @@ > > #ifndef __LINUX_NET_XDP_H__ > > #define __LINUX_NET_XDP_H__ > > > > +#include <linux/skbuff.h> /* skb_shared_info */ > > + > > /** > > * DOC: XDP RX-queue information > > * > > @@ -70,8 +72,19 @@ struct xdp_buff { > > void *data_hard_start; > > unsigned long handle; > > struct xdp_rxq_info *rxq; > > + u32 frame_sz; /* frame size to deduct data_hard_end/reserved tailroom*/ > > I think maybe you want to s/deduct/deduce/ here? Okay, queued for V2.
diff --git a/include/net/xdp.h b/include/net/xdp.h index 40c6d3398458..1ccf7df98bee 100644 --- a/include/net/xdp.h +++ b/include/net/xdp.h @@ -6,6 +6,8 @@ #ifndef __LINUX_NET_XDP_H__ #define __LINUX_NET_XDP_H__ +#include <linux/skbuff.h> /* skb_shared_info */ + /** * DOC: XDP RX-queue information * @@ -70,8 +72,19 @@ struct xdp_buff { void *data_hard_start; unsigned long handle; struct xdp_rxq_info *rxq; + u32 frame_sz; /* frame size to deduct data_hard_end/reserved tailroom*/ }; +/* Reserve memory area at end-of data area. + * + * This macro reserves tailroom in the XDP buffer by limiting the + * XDP/BPF data access to data_hard_end. Notice same area (and size) + * is used for XDP_PASS, when constructing the SKB via build_skb(). + */ +#define xdp_data_hard_end(xdp) \ + ((xdp)->data_hard_start + (xdp)->frame_sz - \ + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))) + struct xdp_frame { void *data; u16 len;
XDP have evolved to support several frame sizes, but xdp_buff was not updated with this information. The frame size (frame_sz) member of xdp_buff is introduced to know the real size of the memory the frame is delivered in. When introducing this also make it clear that some tailroom is reserved/required when creating SKBs using build_skb(). It would also have been an option to introduce a pointer to data_hard_end (with reserved offset). The advantage with frame_sz is that (like rxq) drivers only need to setup/assign this value once per NAPI cycle. Due to XDP-generic (and some drivers) it's not possible to store frame_sz inside xdp_rxq_info, because it's varies per packet as it can be based/depend on packet length. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- include/net/xdp.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)