| Message ID | 95151f4b8a25ce38243e82f0a82104d0f46fb33a.1638272238.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | mvneta: introduce XDP multi-buffer support | expand |
Lorenzo Bianconi wrote: > Update multi-buffer bit (mb) in xdp_buff to notify XDP/eBPF layer and > XDP remote drivers if this is a "non-linear" XDP buffer. Access > skb_shared_info only if xdp_buff mb is set in order to avoid possible > cache-misses. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> [...] > @@ -2320,8 +2325,12 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, > struct xdp_buff *xdp, u32 desc_status) > { > struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp); > - int i, num_frags = sinfo->nr_frags; > struct sk_buff *skb; > + u8 num_frags; > + int i; > + > + if (unlikely(xdp_buff_is_mb(xdp))) > + num_frags = sinfo->nr_frags; Doesn't really need a respin IMO, but rather an observation. Its not obvious to me the unlikely/likely pair here is wanted. Seems it could be relatively common for some applications sending jumbo frames. Maybe worth some experimenting in the future. > > skb = build_skb(xdp->data_hard_start, PAGE_SIZE); > if (!skb) > @@ -2333,6 +2342,9 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, > skb_put(skb, xdp->data_end - xdp->data); > skb->ip_summed = mvneta_rx_csum(pp, desc_status); > > + if (likely(!xdp_buff_is_mb(xdp))) > + goto out; > + > for (i = 0; i < num_frags; i++) { > skb_frag_t *frag = &sinfo->frags[i];
> Lorenzo Bianconi wrote: > > Update multi-buffer bit (mb) in xdp_buff to notify XDP/eBPF layer and > > XDP remote drivers if this is a "non-linear" XDP buffer. Access > > skb_shared_info only if xdp_buff mb is set in order to avoid possible > > cache-misses. > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > [...] > > > @@ -2320,8 +2325,12 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, > > struct xdp_buff *xdp, u32 desc_status) > > { > > struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp); > > - int i, num_frags = sinfo->nr_frags; > > struct sk_buff *skb; > > + u8 num_frags; > > + int i; > > + > > + if (unlikely(xdp_buff_is_mb(xdp))) > > + num_frags = sinfo->nr_frags; > > Doesn't really need a respin IMO, but rather an observation. Its not > obvious to me the unlikely/likely pair here is wanted. Seems it could > be relatively common for some applications sending jumbo frames. > > Maybe worth some experimenting in the future. Probably for mvneta it will not make any difference but in general I tried to avoid possible cache-misses here (accessing sinfo pointers). I will carry out some comparison to see if I can simplify the code. Regards, Lorenzo > > > > > skb = build_skb(xdp->data_hard_start, PAGE_SIZE); > > if (!skb) > > @@ -2333,6 +2342,9 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, > > skb_put(skb, xdp->data_end - xdp->data); > > skb->ip_summed = mvneta_rx_csum(pp, desc_status); > > > > + if (likely(!xdp_buff_is_mb(xdp))) > > + goto out; > > + > > for (i = 0; i < num_frags; i++) { > > skb_frag_t *frag = &sinfo->frags[i]; >
Lorenzo Bianconi wrote: > > Lorenzo Bianconi wrote: > > > Update multi-buffer bit (mb) in xdp_buff to notify XDP/eBPF layer and > > > XDP remote drivers if this is a "non-linear" XDP buffer. Access > > > skb_shared_info only if xdp_buff mb is set in order to avoid possible > > > cache-misses. > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > [...] > > > > > @@ -2320,8 +2325,12 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, > > > struct xdp_buff *xdp, u32 desc_status) > > > { > > > struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp); > > > - int i, num_frags = sinfo->nr_frags; > > > struct sk_buff *skb; > > > + u8 num_frags; > > > + int i; > > > + > > > + if (unlikely(xdp_buff_is_mb(xdp))) > > > + num_frags = sinfo->nr_frags; > > > > Doesn't really need a respin IMO, but rather an observation. Its not > > obvious to me the unlikely/likely pair here is wanted. Seems it could > > be relatively common for some applications sending jumbo frames. > > > > Maybe worth some experimenting in the future. > > Probably for mvneta it will not make any difference but in general I tried to > avoid possible cache-misses here (accessing sinfo pointers). I will carry out > some comparison to see if I can simplify the code. Agree, I'll predict for mvneta it doesn't make a difference either way and perhaps if you want to optimize small pkt benchmarks on a 100Gbps nic it would show a win.
> Lorenzo Bianconi wrote: > > > Lorenzo Bianconi wrote: > > > > Update multi-buffer bit (mb) in xdp_buff to notify XDP/eBPF layer and > > > > XDP remote drivers if this is a "non-linear" XDP buffer. Access > > > > skb_shared_info only if xdp_buff mb is set in order to avoid possible > > > > cache-misses. > > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > > > [...] > > > > > > > @@ -2320,8 +2325,12 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, > > > > struct xdp_buff *xdp, u32 desc_status) > > > > { > > > > struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp); > > > > - int i, num_frags = sinfo->nr_frags; > > > > struct sk_buff *skb; > > > > + u8 num_frags; > > > > + int i; > > > > + > > > > + if (unlikely(xdp_buff_is_mb(xdp))) > > > > + num_frags = sinfo->nr_frags; > > > > > > Doesn't really need a respin IMO, but rather an observation. Its not > > > obvious to me the unlikely/likely pair here is wanted. Seems it could > > > be relatively common for some applications sending jumbo frames. > > > > > > Maybe worth some experimenting in the future. > > > > Probably for mvneta it will not make any difference but in general I tried to > > avoid possible cache-misses here (accessing sinfo pointers). I will carry out > > some comparison to see if I can simplify the code. > > Agree, I'll predict for mvneta it doesn't make a difference either way and > perhaps if you want to optimize small pkt benchmarks on a 100Gbps nic it would > show a win. > actually it makes a slightly difference on mvneta as well (~45-50Kpps). I will keep the code as it is for the moment. Regards, Lorenzo
Lorenzo Bianconi wrote: > > Lorenzo Bianconi wrote: > > > > Lorenzo Bianconi wrote: > > > > > Update multi-buffer bit (mb) in xdp_buff to notify XDP/eBPF layer and > > > > > XDP remote drivers if this is a "non-linear" XDP buffer. Access > > > > > skb_shared_info only if xdp_buff mb is set in order to avoid possible > > > > > cache-misses. > > > > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > > > > > [...] > > > > > > > > > @@ -2320,8 +2325,12 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, > > > > > struct xdp_buff *xdp, u32 desc_status) > > > > > { > > > > > struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp); > > > > > - int i, num_frags = sinfo->nr_frags; > > > > > struct sk_buff *skb; > > > > > + u8 num_frags; > > > > > + int i; > > > > > + > > > > > + if (unlikely(xdp_buff_is_mb(xdp))) > > > > > + num_frags = sinfo->nr_frags; > > > > > > > > Doesn't really need a respin IMO, but rather an observation. Its not > > > > obvious to me the unlikely/likely pair here is wanted. Seems it could > > > > be relatively common for some applications sending jumbo frames. > > > > > > > > Maybe worth some experimenting in the future. > > > > > > Probably for mvneta it will not make any difference but in general I tried to > > > avoid possible cache-misses here (accessing sinfo pointers). I will carry out > > > some comparison to see if I can simplify the code. > > > > Agree, I'll predict for mvneta it doesn't make a difference either way and > > perhaps if you want to optimize small pkt benchmarks on a 100Gbps nic it would > > show a win. > > > > actually it makes a slightly difference on mvneta as well (~45-50Kpps). > I will keep the code as it is for the moment. OK works for me thanks for checking. > > Regards, > Lorenzo
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 67a644177880..208dc9116147 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -2037,9 +2037,14 @@ mvneta_xdp_put_buff(struct mvneta_port *pp, struct mvneta_rx_queue *rxq, { int i; + if (likely(!xdp_buff_is_mb(xdp))) + goto out; + for (i = 0; i < sinfo->nr_frags; i++) page_pool_put_full_page(rxq->page_pool, skb_frag_page(&sinfo->frags[i]), true); + +out: page_pool_put_page(rxq->page_pool, virt_to_head_page(xdp->data), sync_len, true); } @@ -2241,7 +2246,6 @@ mvneta_swbm_rx_frame(struct mvneta_port *pp, int data_len = -MVNETA_MH_SIZE, len; struct net_device *dev = pp->dev; enum dma_data_direction dma_dir; - struct skb_shared_info *sinfo; if (*size > MVNETA_MAX_RX_BUF_SIZE) { len = MVNETA_MAX_RX_BUF_SIZE; @@ -2261,11 +2265,9 @@ mvneta_swbm_rx_frame(struct mvneta_port *pp, /* Prefetch header */ prefetch(data); + xdp_buff_clear_mb(xdp); xdp_prepare_buff(xdp, data, pp->rx_offset_correction + MVNETA_MH_SIZE, data_len, false); - - sinfo = xdp_get_shared_info_from_buff(xdp); - sinfo->nr_frags = 0; } static void @@ -2299,6 +2301,9 @@ mvneta_swbm_add_rx_fragment(struct mvneta_port *pp, skb_frag_off_set(frag, pp->rx_offset_correction); skb_frag_size_set(frag, data_len); __skb_frag_set_page(frag, page); + + if (!xdp_buff_is_mb(xdp)) + xdp_buff_set_mb(xdp); } else { page_pool_put_full_page(rxq->page_pool, page, true); } @@ -2320,8 +2325,12 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, struct xdp_buff *xdp, u32 desc_status) { struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp); - int i, num_frags = sinfo->nr_frags; struct sk_buff *skb; + u8 num_frags; + int i; + + if (unlikely(xdp_buff_is_mb(xdp))) + num_frags = sinfo->nr_frags; skb = build_skb(xdp->data_hard_start, PAGE_SIZE); if (!skb) @@ -2333,6 +2342,9 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, skb_put(skb, xdp->data_end - xdp->data); skb->ip_summed = mvneta_rx_csum(pp, desc_status); + if (likely(!xdp_buff_is_mb(xdp))) + goto out; + for (i = 0; i < num_frags; i++) { skb_frag_t *frag = &sinfo->frags[i]; @@ -2341,6 +2353,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool, skb_frag_size(frag), PAGE_SIZE); } +out: return skb; }
Update multi-buffer bit (mb) in xdp_buff to notify XDP/eBPF layer and XDP remote drivers if this is a "non-linear" XDP buffer. Access skb_shared_info only if xdp_buff mb is set in order to avoid possible cache-misses. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- drivers/net/ethernet/marvell/mvneta.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)