| Message ID | f46a84381037e76ff0e812abd77a0670d0d14767.1631007211.git.lorenzo@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v13,bpf-next,01/18] net: skbuff: add size metadata to skb_shared_info for xdp | expand |
On 07/09/2021 14.35, Lorenzo Bianconi wrote: > Introduce xdp_update_skb_shared_info routine to update frags array > metadata in skb_shared_info data structure converting to a skb from > a xdp_buff or xdp_frame. > According to the current skb_shared_info architecture in > xdp_frame/xdp_buff and to the xdp multi-buff support, there is > no need to run skb_add_rx_frag() and reset frags array converting the buffer > to a skb since the frag array will be in the same position for xdp_buff/xdp_frame > and for the skb, we just need to update memory metadata. > Introduce XDP_FLAGS_PF_MEMALLOC flag in xdp_buff_flags in order to mark > the xdp_buff or xdp_frame as under memory-pressure if pages of the frags array > are under memory pressure. Doing so we can avoid looping over all fragments in > xdp_update_skb_shared_info routine. The driver is expected to set the > flag constructing the xdp_buffer using xdp_buff_set_frag_pfmemalloc > utility routine. > Rely on xdp_update_skb_shared_info in __xdp_build_skb_from_frame routine > converting the multi-buff xdp_frame to a skb after performing a XDP_REDIRECT. > > Acked-by: John Fastabend <john.fastabend@gmail.com> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > include/net/xdp.h | 33 ++++++++++++++++++++++++++++++++- > net/core/xdp.c | 17 +++++++++++++++++ > 2 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/include/net/xdp.h b/include/net/xdp.h > index ed5ea784fd45..53cccdc9528c 100644 > --- a/include/net/xdp.h > +++ b/include/net/xdp.h > @@ -67,7 +67,10 @@ struct xdp_txq_info { > }; > > enum xdp_buff_flags { > - XDP_FLAGS_MULTI_BUFF = BIT(0), /* non-linear xdp buff */ > + XDP_FLAGS_MULTI_BUFF = BIT(0), /* non-linear xdp buff */ > + XDP_FLAGS_FRAGS_PF_MEMALLOC = BIT(1), /* xdp multi-buff paged memory > + * is under pressure > + */ > }; > > struct xdp_buff { > @@ -96,6 +99,16 @@ static __always_inline void xdp_buff_clear_mb(struct xdp_buff *xdp) > xdp->flags &= ~XDP_FLAGS_MULTI_BUFF; > } > > +static __always_inline bool xdp_buff_is_frag_pfmemalloc(struct xdp_buff *xdp) > +{ > + return !!(xdp->flags & XDP_FLAGS_FRAGS_PF_MEMALLOC); > +} > + > +static __always_inline void xdp_buff_set_frag_pfmemalloc(struct xdp_buff *xdp) > +{ > + xdp->flags |= XDP_FLAGS_FRAGS_PF_MEMALLOC; > +} > + > static __always_inline void > xdp_init_buff(struct xdp_buff *xdp, u32 frame_sz, struct xdp_rxq_info *rxq) > { > @@ -151,6 +164,11 @@ static __always_inline bool xdp_frame_is_mb(struct xdp_frame *frame) > return !!(frame->flags & XDP_FLAGS_MULTI_BUFF); > } > > +static __always_inline bool xdp_frame_is_frag_pfmemalloc(struct xdp_frame *frame) > +{ > + return !!(frame->flags & XDP_FLAGS_FRAGS_PF_MEMALLOC); > +} > + > #define XDP_BULK_QUEUE_SIZE 16 > struct xdp_frame_bulk { > int count; > @@ -186,6 +204,19 @@ static inline void xdp_scrub_frame(struct xdp_frame *frame) > frame->dev_rx = NULL; > } > > +static inline void > +xdp_update_skb_shared_info(struct sk_buff *skb, u8 nr_frags, > + unsigned int size, unsigned int truesize, > + bool pfmemalloc) > +{ > + skb_shinfo(skb)->nr_frags = nr_frags; > + > + skb->len += size; > + skb->data_len += size; > + skb->truesize += truesize; > + skb->pfmemalloc |= pfmemalloc; Do we need to clear gso_type here as it is shared/union with xdp_frags_size ? (see below ... it is already cleared before call) > +} > + > /* Avoids inlining WARN macro in fast-path */ > void xdp_warn(const char *msg, const char *func, const int line); > #define XDP_WARN(msg) xdp_warn(msg, __func__, __LINE__) > diff --git a/net/core/xdp.c b/net/core/xdp.c > index cc92ccb38432..504be3ce3ca9 100644 > --- a/net/core/xdp.c > +++ b/net/core/xdp.c > @@ -531,8 +531,20 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf, > struct sk_buff *skb, > struct net_device *dev) > { > + unsigned int frag_size, frag_tsize; > unsigned int headroom, frame_size; > void *hard_start; > + u8 nr_frags; > + > + /* xdp multi-buff frame */ > + if (unlikely(xdp_frame_is_mb(xdpf))) { > + struct skb_shared_info *sinfo; > + > + sinfo = xdp_get_shared_info_from_frame(xdpf); > + frag_tsize = sinfo->xdp_frags_tsize; > + frag_size = sinfo->xdp_frags_size; > + nr_frags = sinfo->nr_frags; > + } > > /* Part of headroom was reserved to xdpf */ > headroom = sizeof(*xdpf) + xdpf->headroom; > @@ -552,6 +564,11 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf, > if (xdpf->metasize) > skb_metadata_set(skb, xdpf->metasize); > > + if (unlikely(xdp_frame_is_mb(xdpf))) > + xdp_update_skb_shared_info(skb, nr_frags, > + frag_size, frag_tsize, > + xdp_frame_is_frag_pfmemalloc(xdpf)); > + There is a build_skb_around() call before this call, which via __build_skb_around() will clear top part of skb_shared_info. (Thus, clearing gso_type not needed ... see above) > /* Essential SKB info: protocol and skb->dev */ > skb->protocol = eth_type_trans(skb, dev); > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
[...] > > Do we need to clear gso_type here as it is shared/union with xdp_frags_size > ? > (see below ... it is already cleared before call) > > > > +} > > + > > /* Avoids inlining WARN macro in fast-path */ > > void xdp_warn(const char *msg, const char *func, const int line); > > #define XDP_WARN(msg) xdp_warn(msg, __func__, __LINE__) > > diff --git a/net/core/xdp.c b/net/core/xdp.c > > index cc92ccb38432..504be3ce3ca9 100644 > > --- a/net/core/xdp.c > > +++ b/net/core/xdp.c > > @@ -531,8 +531,20 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf, > > struct sk_buff *skb, > > struct net_device *dev) > > { > > + unsigned int frag_size, frag_tsize; > > unsigned int headroom, frame_size; > > void *hard_start; > > + u8 nr_frags; > > + > > + /* xdp multi-buff frame */ > > + if (unlikely(xdp_frame_is_mb(xdpf))) { > > + struct skb_shared_info *sinfo; > > + > > + sinfo = xdp_get_shared_info_from_frame(xdpf); > > + frag_tsize = sinfo->xdp_frags_tsize; > > + frag_size = sinfo->xdp_frags_size; > > + nr_frags = sinfo->nr_frags; > > + } > > /* Part of headroom was reserved to xdpf */ > > headroom = sizeof(*xdpf) + xdpf->headroom; > > @@ -552,6 +564,11 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf, > > if (xdpf->metasize) > > skb_metadata_set(skb, xdpf->metasize); > > + if (unlikely(xdp_frame_is_mb(xdpf))) > > + xdp_update_skb_shared_info(skb, nr_frags, > > + frag_size, frag_tsize, > > + xdp_frame_is_frag_pfmemalloc(xdpf)); > > + > > There is a build_skb_around() call before this call, which via > __build_skb_around() will clear top part of skb_shared_info. > (Thus, clearing gso_type not needed ... see above) yes, this is why I need save sinfo->nr_frags, sinfo->xdp_frags_size, ... Regards, Lorenzo > > > /* Essential SKB info: protocol and skb->dev */ > > skb->protocol = eth_type_trans(skb, dev); > > > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com> >
diff --git a/include/net/xdp.h b/include/net/xdp.h index ed5ea784fd45..53cccdc9528c 100644 --- a/include/net/xdp.h +++ b/include/net/xdp.h @@ -67,7 +67,10 @@ struct xdp_txq_info { }; enum xdp_buff_flags { - XDP_FLAGS_MULTI_BUFF = BIT(0), /* non-linear xdp buff */ + XDP_FLAGS_MULTI_BUFF = BIT(0), /* non-linear xdp buff */ + XDP_FLAGS_FRAGS_PF_MEMALLOC = BIT(1), /* xdp multi-buff paged memory + * is under pressure + */ }; struct xdp_buff { @@ -96,6 +99,16 @@ static __always_inline void xdp_buff_clear_mb(struct xdp_buff *xdp) xdp->flags &= ~XDP_FLAGS_MULTI_BUFF; } +static __always_inline bool xdp_buff_is_frag_pfmemalloc(struct xdp_buff *xdp) +{ + return !!(xdp->flags & XDP_FLAGS_FRAGS_PF_MEMALLOC); +} + +static __always_inline void xdp_buff_set_frag_pfmemalloc(struct xdp_buff *xdp) +{ + xdp->flags |= XDP_FLAGS_FRAGS_PF_MEMALLOC; +} + static __always_inline void xdp_init_buff(struct xdp_buff *xdp, u32 frame_sz, struct xdp_rxq_info *rxq) { @@ -151,6 +164,11 @@ static __always_inline bool xdp_frame_is_mb(struct xdp_frame *frame) return !!(frame->flags & XDP_FLAGS_MULTI_BUFF); } +static __always_inline bool xdp_frame_is_frag_pfmemalloc(struct xdp_frame *frame) +{ + return !!(frame->flags & XDP_FLAGS_FRAGS_PF_MEMALLOC); +} + #define XDP_BULK_QUEUE_SIZE 16 struct xdp_frame_bulk { int count; @@ -186,6 +204,19 @@ static inline void xdp_scrub_frame(struct xdp_frame *frame) frame->dev_rx = NULL; } +static inline void +xdp_update_skb_shared_info(struct sk_buff *skb, u8 nr_frags, + unsigned int size, unsigned int truesize, + bool pfmemalloc) +{ + skb_shinfo(skb)->nr_frags = nr_frags; + + skb->len += size; + skb->data_len += size; + skb->truesize += truesize; + skb->pfmemalloc |= pfmemalloc; +} + /* Avoids inlining WARN macro in fast-path */ void xdp_warn(const char *msg, const char *func, const int line); #define XDP_WARN(msg) xdp_warn(msg, __func__, __LINE__) diff --git a/net/core/xdp.c b/net/core/xdp.c index cc92ccb38432..504be3ce3ca9 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -531,8 +531,20 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf, struct sk_buff *skb, struct net_device *dev) { + unsigned int frag_size, frag_tsize; unsigned int headroom, frame_size; void *hard_start; + u8 nr_frags; + + /* xdp multi-buff frame */ + if (unlikely(xdp_frame_is_mb(xdpf))) { + struct skb_shared_info *sinfo; + + sinfo = xdp_get_shared_info_from_frame(xdpf); + frag_tsize = sinfo->xdp_frags_tsize; + frag_size = sinfo->xdp_frags_size; + nr_frags = sinfo->nr_frags; + } /* Part of headroom was reserved to xdpf */ headroom = sizeof(*xdpf) + xdpf->headroom; @@ -552,6 +564,11 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf, if (xdpf->metasize) skb_metadata_set(skb, xdpf->metasize); + if (unlikely(xdp_frame_is_mb(xdpf))) + xdp_update_skb_shared_info(skb, nr_frags, + frag_size, frag_tsize, + xdp_frame_is_frag_pfmemalloc(xdpf)); + /* Essential SKB info: protocol and skb->dev */ skb->protocol = eth_type_trans(skb, dev);