| Message ID | 1dc514b266e19b1e5973d038a0189ab6e4acb93a.1670544817.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net-next] net: ethernet: enetc: unlock XDP_REDIRECT for XDP non-linear buffers | expand |
On Fri, Dec 09, 2022 at 01:19:44AM +0100, Lorenzo Bianconi wrote: > Even if full XDP_REDIRECT is not supported yet for non-linear XDP buffers > since we allow redirecting just into CPUMAPs, unlock XDP_REDIRECT for > S/G XDP buffer and rely on XDP stack to properly take care of the > frames. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > Changes since v1: > - drop Fixes tag > - unlock XDP_REDIRECT > - populate missing XDP metadata > > Please note this patch is just compile tested > --- How would you like me to test this patch? > drivers/net/ethernet/freescale/enetc/enetc.c | 27 +++++++++----------- > 1 file changed, 12 insertions(+), 15 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c > index 8671591cb750..9fd15e1e692d 100644 > --- a/drivers/net/ethernet/freescale/enetc/enetc.c > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c > @@ -1412,6 +1412,16 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > /* To be used for XDP_TX */ > rx_swbd->len = size; > > + if (!xdp_buff_has_frags(xdp_buff)) { > + xdp_buff_set_frags_flag(xdp_buff); > + shinfo->xdp_frags_size = size; > + } else { > + shinfo->xdp_frags_size += size; > + } > + > + if (page_is_pfmemalloc(rx_swbd->page)) > + xdp_buff_set_frag_pfmemalloc(xdp_buff); > + > skb_frag_off_set(frag, rx_swbd->page_offset); > skb_frag_size_set(frag, size); > __skb_frag_set_page(frag, rx_swbd->page); > @@ -1601,22 +1611,7 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring, > } > break; > case XDP_REDIRECT: > - /* xdp_return_frame does not support S/G in the sense > - * that it leaks the fragments (__xdp_return should not > - * call page_frag_free only for the initial buffer). > - * Until XDP_REDIRECT gains support for S/G let's keep > - * the code structure in place, but dead. We drop the > - * S/G frames ourselves to avoid memory leaks which > - * would otherwise leave the kernel OOM. > - */ > - if (unlikely(cleaned_cnt - orig_cleaned_cnt != 1)) { > - enetc_xdp_drop(rx_ring, orig_i, i); > - rx_ring->stats.xdp_redirect_sg++; > - break; > - } > - > tmp_orig_i = orig_i; > - > while (orig_i != i) { > enetc_flip_rx_buff(rx_ring, > &rx_ring->rx_swbd[orig_i]); > @@ -1628,6 +1623,8 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring, > enetc_xdp_free(rx_ring, tmp_orig_i, i); > } else { > xdp_redirect_frm_cnt++; > + if (xdp_buff_has_frags(&xdp_buff)) > + rx_ring->stats.xdp_redirect_sg++; Ideally we'd remove this counter altogether. Nothing interesting to see. > rx_ring->stats.xdp_redirect++; > } > } > -- > 2.38.1 >
> On Fri, Dec 09, 2022 at 01:19:44AM +0100, Lorenzo Bianconi wrote: > > Even if full XDP_REDIRECT is not supported yet for non-linear XDP buffers > > since we allow redirecting just into CPUMAPs, unlock XDP_REDIRECT for > > S/G XDP buffer and rely on XDP stack to properly take care of the > > frames. > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > Changes since v1: > > - drop Fixes tag > > - unlock XDP_REDIRECT > > - populate missing XDP metadata > > > > Please note this patch is just compile tested > > --- > > How would you like me to test this patch? Hi Vladimir, you can use xdp_redirect_cpu sample available in the kernel source tree. > > > drivers/net/ethernet/freescale/enetc/enetc.c | 27 +++++++++----------- > > 1 file changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c > > index 8671591cb750..9fd15e1e692d 100644 > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c > > @@ -1412,6 +1412,16 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > > /* To be used for XDP_TX */ > > rx_swbd->len = size; > > > > + if (!xdp_buff_has_frags(xdp_buff)) { > > + xdp_buff_set_frags_flag(xdp_buff); > > + shinfo->xdp_frags_size = size; > > + } else { > > + shinfo->xdp_frags_size += size; > > + } > > + > > + if (page_is_pfmemalloc(rx_swbd->page)) > > + xdp_buff_set_frag_pfmemalloc(xdp_buff); > > + > > skb_frag_off_set(frag, rx_swbd->page_offset); > > skb_frag_size_set(frag, size); > > __skb_frag_set_page(frag, rx_swbd->page); > > @@ -1601,22 +1611,7 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring, > > } > > break; > > case XDP_REDIRECT: > > - /* xdp_return_frame does not support S/G in the sense > > - * that it leaks the fragments (__xdp_return should not > > - * call page_frag_free only for the initial buffer). > > - * Until XDP_REDIRECT gains support for S/G let's keep > > - * the code structure in place, but dead. We drop the > > - * S/G frames ourselves to avoid memory leaks which > > - * would otherwise leave the kernel OOM. > > - */ > > - if (unlikely(cleaned_cnt - orig_cleaned_cnt != 1)) { > > - enetc_xdp_drop(rx_ring, orig_i, i); > > - rx_ring->stats.xdp_redirect_sg++; > > - break; > > - } > > - > > tmp_orig_i = orig_i; > > - > > while (orig_i != i) { > > enetc_flip_rx_buff(rx_ring, > > &rx_ring->rx_swbd[orig_i]); > > @@ -1628,6 +1623,8 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring, > > enetc_xdp_free(rx_ring, tmp_orig_i, i); > > } else { > > xdp_redirect_frm_cnt++; > > + if (xdp_buff_has_frags(&xdp_buff)) > > + rx_ring->stats.xdp_redirect_sg++; > > Ideally we'd remove this counter altogether. Nothing interesting to see. ok, I will get rid of it in the next version. Regards, Lorenzo > > > rx_ring->stats.xdp_redirect++; > > } > > } > > -- > > 2.38.1 > >
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index 8671591cb750..9fd15e1e692d 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -1412,6 +1412,16 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, /* To be used for XDP_TX */ rx_swbd->len = size; + if (!xdp_buff_has_frags(xdp_buff)) { + xdp_buff_set_frags_flag(xdp_buff); + shinfo->xdp_frags_size = size; + } else { + shinfo->xdp_frags_size += size; + } + + if (page_is_pfmemalloc(rx_swbd->page)) + xdp_buff_set_frag_pfmemalloc(xdp_buff); + skb_frag_off_set(frag, rx_swbd->page_offset); skb_frag_size_set(frag, size); __skb_frag_set_page(frag, rx_swbd->page); @@ -1601,22 +1611,7 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring, } break; case XDP_REDIRECT: - /* xdp_return_frame does not support S/G in the sense - * that it leaks the fragments (__xdp_return should not - * call page_frag_free only for the initial buffer). - * Until XDP_REDIRECT gains support for S/G let's keep - * the code structure in place, but dead. We drop the - * S/G frames ourselves to avoid memory leaks which - * would otherwise leave the kernel OOM. - */ - if (unlikely(cleaned_cnt - orig_cleaned_cnt != 1)) { - enetc_xdp_drop(rx_ring, orig_i, i); - rx_ring->stats.xdp_redirect_sg++; - break; - } - tmp_orig_i = orig_i; - while (orig_i != i) { enetc_flip_rx_buff(rx_ring, &rx_ring->rx_swbd[orig_i]); @@ -1628,6 +1623,8 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring, enetc_xdp_free(rx_ring, tmp_orig_i, i); } else { xdp_redirect_frm_cnt++; + if (xdp_buff_has_frags(&xdp_buff)) + rx_ring->stats.xdp_redirect_sg++; rx_ring->stats.xdp_redirect++; } }
Even if full XDP_REDIRECT is not supported yet for non-linear XDP buffers since we allow redirecting just into CPUMAPs, unlock XDP_REDIRECT for S/G XDP buffer and rely on XDP stack to properly take care of the frames. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- Changes since v1: - drop Fixes tag - unlock XDP_REDIRECT - populate missing XDP metadata Please note this patch is just compile tested --- drivers/net/ethernet/freescale/enetc/enetc.c | 27 +++++++++----------- 1 file changed, 12 insertions(+), 15 deletions(-)