| Message ID | 8acb59077ff51eb58ca164e432be63194a92b0bf.1670924659.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFT] net: ethernet: enetc: do not always access skb_shared_info in the XDP path | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Hi Lorenzo, On Tue, Dec 13, 2022 at 10:46:43AM +0100, Lorenzo Bianconi wrote: > Move XDP skb_shared_info structure initialization in from > enetc_map_rx_buff_to_xdp() to enetc_add_rx_buff_to_xdp() and do not always > access skb_shared_info in the xdp_buff/xdp_frame since it is located in a > different cacheline with respect to hard_start and data xdp pointers. > Rely on XDP_FLAGS_HAS_FRAGS flag to check if it really necessary to access > non-linear part of the xdp_buff/xdp_frame. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > This patch is based on the following series not applied yet to next-next: > https://patchwork.kernel.org/project/netdevbpf/cover/cover.1670680119.git.lorenzo@kernel.org/ > --- > drivers/net/ethernet/freescale/enetc/enetc.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c > index cd8f5f0c6b54..2ed6b163f3c8 100644 > --- a/drivers/net/ethernet/freescale/enetc/enetc.c > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c > @@ -1305,6 +1305,10 @@ static int enetc_xdp_frame_to_xdp_tx_swbd(struct enetc_bdr *tx_ring, > xdp_tx_swbd->xdp_frame = NULL; > > n++; > + > + if (!xdp_frame_has_frags(xdp_frame)) > + goto out; > + Tested this with single-buffer devmap XDP_REDIRECT, can't test with multi-buffer I think. > xdp_tx_swbd = &xdp_tx_arr[n]; > > shinfo = xdp_get_shared_info_from_frame(xdp_frame); > @@ -1334,7 +1338,7 @@ static int enetc_xdp_frame_to_xdp_tx_swbd(struct enetc_bdr *tx_ring, > n++; > xdp_tx_swbd = &xdp_tx_arr[n]; > } > - > +out: > xdp_tx_arr[n - 1].is_eof = true; > xdp_tx_arr[n - 1].xdp_frame = xdp_frame; > > @@ -1390,16 +1394,12 @@ static void enetc_map_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > { > struct enetc_rx_swbd *rx_swbd = enetc_get_rx_buff(rx_ring, i, size); > void *hard_start = page_address(rx_swbd->page) + rx_swbd->page_offset; > - struct skb_shared_info *shinfo; > > /* To be used for XDP_TX */ > rx_swbd->len = size; > > xdp_prepare_buff(xdp_buff, hard_start - rx_ring->buffer_offset, > rx_ring->buffer_offset, size, false); > - > - shinfo = xdp_get_shared_info_from_buff(xdp_buff); > - shinfo->nr_frags = 0; > } > > static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > @@ -1407,7 +1407,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > { > struct skb_shared_info *shinfo = xdp_get_shared_info_from_buff(xdp_buff); > struct enetc_rx_swbd *rx_swbd = enetc_get_rx_buff(rx_ring, i, size); > - skb_frag_t *frag = &shinfo->frags[shinfo->nr_frags]; > + skb_frag_t *frag; > > /* To be used for XDP_TX */ > rx_swbd->len = size; > @@ -1415,6 +1415,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > if (!xdp_buff_has_frags(xdp_buff)) { > xdp_buff_set_frags_flag(xdp_buff); > shinfo->xdp_frags_size = size; > + shinfo->nr_frags = 0; Tested this and enetc_map_rx_buff_to_xdp() with single-buffer and multi-buffer XDP_TX. > } else { > shinfo->xdp_frags_size += size; > } > @@ -1422,6 +1423,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > if (page_is_pfmemalloc(rx_swbd->page)) > xdp_buff_set_frag_pfmemalloc(xdp_buff); > > + frag = &shinfo->frags[shinfo->nr_frags]; > skb_frag_off_set(frag, rx_swbd->page_offset); > skb_frag_size_set(frag, size); > __skb_frag_set_page(frag, rx_swbd->page); > -- > 2.38.1 > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com> Thanks.
> Hi Lorenzo, > > On Tue, Dec 13, 2022 at 10:46:43AM +0100, Lorenzo Bianconi wrote: > > Move XDP skb_shared_info structure initialization in from > > enetc_map_rx_buff_to_xdp() to enetc_add_rx_buff_to_xdp() and do not always > > access skb_shared_info in the xdp_buff/xdp_frame since it is located in a > > different cacheline with respect to hard_start and data xdp pointers. > > Rely on XDP_FLAGS_HAS_FRAGS flag to check if it really necessary to access > > non-linear part of the xdp_buff/xdp_frame. > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > This patch is based on the following series not applied yet to next-next: > > https://patchwork.kernel.org/project/netdevbpf/cover/cover.1670680119.git.lorenzo@kernel.org/ > > --- > > drivers/net/ethernet/freescale/enetc/enetc.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c > > index cd8f5f0c6b54..2ed6b163f3c8 100644 > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c > > @@ -1305,6 +1305,10 @@ static int enetc_xdp_frame_to_xdp_tx_swbd(struct enetc_bdr *tx_ring, > > xdp_tx_swbd->xdp_frame = NULL; > > > > n++; > > + > > + if (!xdp_frame_has_frags(xdp_frame)) > > + goto out; > > + > > Tested this with single-buffer devmap XDP_REDIRECT, can't test with > multi-buffer I think. ack, thx for testing. I will add this patch to the prvious series. In oreder to test it with xdp-mb I think you can redirect into a cpumap and then attach a program on the cpumap to redirect back to the nic, but for the moment you need to comment out this line: https://github.com/torvalds/linux/blob/master/net/core/filter.c#L4291 Regards, Lorenzo > > > xdp_tx_swbd = &xdp_tx_arr[n]; > > > > shinfo = xdp_get_shared_info_from_frame(xdp_frame); > > @@ -1334,7 +1338,7 @@ static int enetc_xdp_frame_to_xdp_tx_swbd(struct enetc_bdr *tx_ring, > > n++; > > xdp_tx_swbd = &xdp_tx_arr[n]; > > } > > - > > +out: > > xdp_tx_arr[n - 1].is_eof = true; > > xdp_tx_arr[n - 1].xdp_frame = xdp_frame; > > > > @@ -1390,16 +1394,12 @@ static void enetc_map_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > > { > > struct enetc_rx_swbd *rx_swbd = enetc_get_rx_buff(rx_ring, i, size); > > void *hard_start = page_address(rx_swbd->page) + rx_swbd->page_offset; > > - struct skb_shared_info *shinfo; > > > > /* To be used for XDP_TX */ > > rx_swbd->len = size; > > > > xdp_prepare_buff(xdp_buff, hard_start - rx_ring->buffer_offset, > > rx_ring->buffer_offset, size, false); > > - > > - shinfo = xdp_get_shared_info_from_buff(xdp_buff); > > - shinfo->nr_frags = 0; > > } > > > > static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > > @@ -1407,7 +1407,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > > { > > struct skb_shared_info *shinfo = xdp_get_shared_info_from_buff(xdp_buff); > > struct enetc_rx_swbd *rx_swbd = enetc_get_rx_buff(rx_ring, i, size); > > - skb_frag_t *frag = &shinfo->frags[shinfo->nr_frags]; > > + skb_frag_t *frag; > > > > /* To be used for XDP_TX */ > > rx_swbd->len = size; > > @@ -1415,6 +1415,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > > if (!xdp_buff_has_frags(xdp_buff)) { > > xdp_buff_set_frags_flag(xdp_buff); > > shinfo->xdp_frags_size = size; > > + shinfo->nr_frags = 0; > > Tested this and enetc_map_rx_buff_to_xdp() with single-buffer and > multi-buffer XDP_TX. > > > } else { > > shinfo->xdp_frags_size += size; > > } > > @@ -1422,6 +1423,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, > > if (page_is_pfmemalloc(rx_swbd->page)) > > xdp_buff_set_frag_pfmemalloc(xdp_buff); > > > > + frag = &shinfo->frags[shinfo->nr_frags]; > > skb_frag_off_set(frag, rx_swbd->page_offset); > > skb_frag_size_set(frag, size); > > __skb_frag_set_page(frag, rx_swbd->page); > > -- > > 2.38.1 > > > > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> > Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > Thanks.
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index cd8f5f0c6b54..2ed6b163f3c8 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -1305,6 +1305,10 @@ static int enetc_xdp_frame_to_xdp_tx_swbd(struct enetc_bdr *tx_ring, xdp_tx_swbd->xdp_frame = NULL; n++; + + if (!xdp_frame_has_frags(xdp_frame)) + goto out; + xdp_tx_swbd = &xdp_tx_arr[n]; shinfo = xdp_get_shared_info_from_frame(xdp_frame); @@ -1334,7 +1338,7 @@ static int enetc_xdp_frame_to_xdp_tx_swbd(struct enetc_bdr *tx_ring, n++; xdp_tx_swbd = &xdp_tx_arr[n]; } - +out: xdp_tx_arr[n - 1].is_eof = true; xdp_tx_arr[n - 1].xdp_frame = xdp_frame; @@ -1390,16 +1394,12 @@ static void enetc_map_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, { struct enetc_rx_swbd *rx_swbd = enetc_get_rx_buff(rx_ring, i, size); void *hard_start = page_address(rx_swbd->page) + rx_swbd->page_offset; - struct skb_shared_info *shinfo; /* To be used for XDP_TX */ rx_swbd->len = size; xdp_prepare_buff(xdp_buff, hard_start - rx_ring->buffer_offset, rx_ring->buffer_offset, size, false); - - shinfo = xdp_get_shared_info_from_buff(xdp_buff); - shinfo->nr_frags = 0; } static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, @@ -1407,7 +1407,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, { struct skb_shared_info *shinfo = xdp_get_shared_info_from_buff(xdp_buff); struct enetc_rx_swbd *rx_swbd = enetc_get_rx_buff(rx_ring, i, size); - skb_frag_t *frag = &shinfo->frags[shinfo->nr_frags]; + skb_frag_t *frag; /* To be used for XDP_TX */ rx_swbd->len = size; @@ -1415,6 +1415,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, if (!xdp_buff_has_frags(xdp_buff)) { xdp_buff_set_frags_flag(xdp_buff); shinfo->xdp_frags_size = size; + shinfo->nr_frags = 0; } else { shinfo->xdp_frags_size += size; } @@ -1422,6 +1423,7 @@ static void enetc_add_rx_buff_to_xdp(struct enetc_bdr *rx_ring, int i, if (page_is_pfmemalloc(rx_swbd->page)) xdp_buff_set_frag_pfmemalloc(xdp_buff); + frag = &shinfo->frags[shinfo->nr_frags]; skb_frag_off_set(frag, rx_swbd->page_offset); skb_frag_size_set(frag, size); __skb_frag_set_page(frag, rx_swbd->page);
Move XDP skb_shared_info structure initialization in from enetc_map_rx_buff_to_xdp() to enetc_add_rx_buff_to_xdp() and do not always access skb_shared_info in the xdp_buff/xdp_frame since it is located in a different cacheline with respect to hard_start and data xdp pointers. Rely on XDP_FLAGS_HAS_FRAGS flag to check if it really necessary to access non-linear part of the xdp_buff/xdp_frame. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- This patch is based on the following series not applied yet to next-next: https://patchwork.kernel.org/project/netdevbpf/cover/cover.1670680119.git.lorenzo@kernel.org/ --- drivers/net/ethernet/freescale/enetc/enetc.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)