* [RFT] net: ethernet: enetc: do not always access skb_shared_info in the XDP path
@ 2022-12-13 9:46 Lorenzo Bianconi
2022-12-13 19:55 ` Vladimir Oltean
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2022-12-13 9:46 UTC (permalink / raw)
To: netdev
Cc: claudiu.manoil, davem, edumazet, kuba, pabeni, vladimir.oltean,
lorenzo.bianconi
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;
+
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);
--
2.38.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFT] net: ethernet: enetc: do not always access skb_shared_info in the XDP path
2022-12-13 9:46 [RFT] net: ethernet: enetc: do not always access skb_shared_info in the XDP path Lorenzo Bianconi
@ 2022-12-13 19:55 ` Vladimir Oltean
2022-12-14 15:02 ` Lorenzo Bianconi
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Oltean @ 2022-12-13 19:55 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: netdev, claudiu.manoil, davem, edumazet, kuba, pabeni,
lorenzo.bianconi
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFT] net: ethernet: enetc: do not always access skb_shared_info in the XDP path
2022-12-13 19:55 ` Vladimir Oltean
@ 2022-12-14 15:02 ` Lorenzo Bianconi
0 siblings, 0 replies; 3+ messages in thread
From: Lorenzo Bianconi @ 2022-12-14 15:02 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, claudiu.manoil, davem, edumazet, kuba, pabeni,
lorenzo.bianconi
[-- Attachment #1: Type: text/plain, Size: 4307 bytes --]
> 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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-14 15:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-13 9:46 [RFT] net: ethernet: enetc: do not always access skb_shared_info in the XDP path Lorenzo Bianconi
2022-12-13 19:55 ` Vladimir Oltean
2022-12-14 15:02 ` Lorenzo Bianconi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).