| Message ID | c2665f369ede07328bbf7456def2e2025b9b320e.1597842004.git.lorenzo@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | mvneta: introduce XDP multi-buffer support | expand |
Lorenzo Bianconi <lorenzo@kernel.org> writes: > Introduce multi-buffer bit (mb) in xdp_frame/xdp_buffer to > specify > if shared_info area has been properly initialized for non-linear > xdp buffers > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > include/net/xdp.h | 8 ++++++-- > net/core/xdp.c | 1 + > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/net/xdp.h b/include/net/xdp.h > index 3814fb631d52..42f439f9fcda 100644 > --- a/include/net/xdp.h > +++ b/include/net/xdp.h > @@ -72,7 +72,8 @@ struct xdp_buff { > void *data_hard_start; > struct xdp_rxq_info *rxq; > struct xdp_txq_info *txq; > - u32 frame_sz; /* frame size to deduce > data_hard_end/reserved tailroom*/ > + u32 frame_sz:31; /* frame size to deduce > data_hard_end/reserved tailroom*/ > + u32 mb:1; /* xdp non-linear buffer */ > }; > > /* Reserve memory area at end-of data area. > @@ -96,7 +97,8 @@ struct xdp_frame { > u16 len; > u16 headroom; > u32 metasize:8; > - u32 frame_sz:24; > + u32 frame_sz:23; > + u32 mb:1; /* xdp non-linear frame */ Although this issue wasn't introduced with this patch, why not make frame_sz field to be the same size in xdp_buff and xdp_frame ? thanks, Shay > /* Lifetime of xdp_rxq_info is limited to NAPI/enqueue > time, > * while mem info is valid on remote CPU. > */ > @@ -141,6 +143,7 @@ void xdp_convert_frame_to_buff(struct > xdp_frame *frame, struct xdp_buff *xdp) > xdp->data_end = frame->data + frame->len; > xdp->data_meta = frame->data - frame->metasize; > xdp->frame_sz = frame->frame_sz; > + xdp->mb = frame->mb; > } > > static inline > @@ -167,6 +170,7 @@ int xdp_update_frame_from_buff(struct > xdp_buff *xdp, > xdp_frame->headroom = headroom - sizeof(*xdp_frame); > xdp_frame->metasize = metasize; > xdp_frame->frame_sz = xdp->frame_sz; > + xdp_frame->mb = xdp->mb; > > return 0; > } > diff --git a/net/core/xdp.c b/net/core/xdp.c > index 48aba933a5a8..884f140fc3be 100644 > --- a/net/core/xdp.c > +++ b/net/core/xdp.c > @@ -454,6 +454,7 @@ struct xdp_frame > *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp) > xdpf->headroom = 0; > xdpf->metasize = metasize; > xdpf->frame_sz = PAGE_SIZE; > + xdpf->mb = xdp->mb; > xdpf->mem.type = MEM_TYPE_PAGE_ORDER0; > > xsk_buff_free(xdp);
On Sun, 23 Aug 2020 17:08:30 +0300 Shay Agroskin <shayagr@amazon.com> wrote: > > diff --git a/include/net/xdp.h b/include/net/xdp.h > > index 3814fb631d52..42f439f9fcda 100644 > > --- a/include/net/xdp.h > > +++ b/include/net/xdp.h > > @@ -72,7 +72,8 @@ struct xdp_buff { > > void *data_hard_start; > > struct xdp_rxq_info *rxq; > > struct xdp_txq_info *txq; > > - u32 frame_sz; /* frame size to deduce > > data_hard_end/reserved tailroom*/ > > + u32 frame_sz:31; /* frame size to deduce > > data_hard_end/reserved tailroom*/ > > + u32 mb:1; /* xdp non-linear buffer */ > > }; > > > > /* Reserve memory area at end-of data area. > > @@ -96,7 +97,8 @@ struct xdp_frame { > > u16 len; > > u16 headroom; > > u32 metasize:8; > > - u32 frame_sz:24; > > + u32 frame_sz:23; > > + u32 mb:1; /* xdp non-linear frame */ > > Although this issue wasn't introduced with this patch, why not > make frame_sz field to be the same size in xdp_buff and xdp_frame > ? This is all about struct layout and saving memory size, due to cacheline access. Please read up on this and use the tool pahole to inspect the struct memory layout.
Jesper Dangaard Brouer <brouer@redhat.com> writes: > On Sun, 23 Aug 2020 17:08:30 +0300 > Shay Agroskin <shayagr@amazon.com> wrote: > >> > diff --git a/include/net/xdp.h b/include/net/xdp.h >> > index 3814fb631d52..42f439f9fcda 100644 >> > --- a/include/net/xdp.h >> > +++ b/include/net/xdp.h >> > @@ -72,7 +72,8 @@ struct xdp_buff { >> > void *data_hard_start; >> > struct xdp_rxq_info *rxq; >> > struct xdp_txq_info *txq; >> > - u32 frame_sz; /* frame size to deduce >> > data_hard_end/reserved tailroom*/ >> > + u32 frame_sz:31; /* frame size to deduce >> > data_hard_end/reserved tailroom*/ >> > + u32 mb:1; /* xdp non-linear buffer */ >> > }; >> > >> > /* Reserve memory area at end-of data area. >> > @@ -96,7 +97,8 @@ struct xdp_frame { >> > u16 len; >> > u16 headroom; >> > u32 metasize:8; >> > - u32 frame_sz:24; >> > + u32 frame_sz:23; >> > + u32 mb:1; /* xdp non-linear frame */ >> >> Although this issue wasn't introduced with this patch, why not >> make frame_sz field to be the same size in xdp_buff and >> xdp_frame >> ? > > This is all about struct layout and saving memory size, due to > cacheline access. Please read up on this and use the tool pahole > to > inspect the struct memory layout. I actually meant reducing the size of frame_sz in xdp_buff (without changing xdp_frame so that it still fits 64 byte cache line). Reducing a field size shouldn't affect cache alignment as far as I can see. Doesn't matter all that much to me, I simply find it a better practice that the same field would have same size in different structs. Shay
diff --git a/include/net/xdp.h b/include/net/xdp.h index 3814fb631d52..42f439f9fcda 100644 --- a/include/net/xdp.h +++ b/include/net/xdp.h @@ -72,7 +72,8 @@ struct xdp_buff { void *data_hard_start; struct xdp_rxq_info *rxq; struct xdp_txq_info *txq; - u32 frame_sz; /* frame size to deduce data_hard_end/reserved tailroom*/ + u32 frame_sz:31; /* frame size to deduce data_hard_end/reserved tailroom*/ + u32 mb:1; /* xdp non-linear buffer */ }; /* Reserve memory area at end-of data area. @@ -96,7 +97,8 @@ struct xdp_frame { u16 len; u16 headroom; u32 metasize:8; - u32 frame_sz:24; + u32 frame_sz:23; + u32 mb:1; /* xdp non-linear frame */ /* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time, * while mem info is valid on remote CPU. */ @@ -141,6 +143,7 @@ void xdp_convert_frame_to_buff(struct xdp_frame *frame, struct xdp_buff *xdp) xdp->data_end = frame->data + frame->len; xdp->data_meta = frame->data - frame->metasize; xdp->frame_sz = frame->frame_sz; + xdp->mb = frame->mb; } static inline @@ -167,6 +170,7 @@ int xdp_update_frame_from_buff(struct xdp_buff *xdp, xdp_frame->headroom = headroom - sizeof(*xdp_frame); xdp_frame->metasize = metasize; xdp_frame->frame_sz = xdp->frame_sz; + xdp_frame->mb = xdp->mb; return 0; } diff --git a/net/core/xdp.c b/net/core/xdp.c index 48aba933a5a8..884f140fc3be 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -454,6 +454,7 @@ struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp) xdpf->headroom = 0; xdpf->metasize = metasize; xdpf->frame_sz = PAGE_SIZE; + xdpf->mb = xdp->mb; xdpf->mem.type = MEM_TYPE_PAGE_ORDER0; xsk_buff_free(xdp);
Introduce multi-buffer bit (mb) in xdp_frame/xdp_buffer to specify if shared_info area has been properly initialized for non-linear xdp buffers Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- include/net/xdp.h | 8 ++++++-- net/core/xdp.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)