* [Intel-wired-lan] [PATCH RFC v2 24/33] ixgbe: add XDP frame size to driver
2020-04-08 11:50 [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Jesper Dangaard Brouer
@ 2020-04-08 11:52 ` Jesper Dangaard Brouer
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 25/33] ixgbevf: add XDP frame size to VF driver Jesper Dangaard Brouer
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2020-04-08 11:52 UTC (permalink / raw)
To: intel-wired-lan
This driver uses different memory models depending on PAGE_SIZE at
compile time. For PAGE_SIZE 4K it uses page splitting, meaning for
normal MTU frame size is 2048 bytes (and headroom 192 bytes). For
larger MTUs the driver still use page splitting, by allocating
order-1 pages (8192 bytes) for RX frames. For PAGE_SIZE larger than
4K, driver instead advance its rx_buffer->page_offset with the frame
size "truesize".
For XDP frame size calculations, this mean that in PAGE_SIZE larger
than 4K mode the frame_sz change on a per packet basis. For the page
split 4K PAGE_SIZE mode, xdp.frame_sz is more constant and can be
updated once outside the main NAPI loop.
The default setting in the driver uses build_skb(), which provides
the necessary headroom and tailroom for XDP-redirect in RX-frame
(in both modes).
There is one complication, which is legacy-rx mode (configurable via
ethtool priv-flags). There are zero headroom in this mode, which is a
requirement for XDP-redirect to work. The conversion to xdp_frame
(convert_to_xdp_frame) will detect this insufficient space, and
xdp_do_redirect() call will fail. This is deemed acceptable, as it
allows other XDP actions to still work in legacy-mode. In
legacy-mode + larger PAGE_SIZE due to lacking tailroom, we also
accept that xdp_adjust_tail shrink doesn't work.
Cc: intel-wired-lan at lists.osuosl.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 34 +++++++++++++++++++------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ea6834bae04c..eab5934b04f5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2244,20 +2244,30 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
return ERR_PTR(-result);
}
+static unsigned int ixgbe_rx_frame_truesize(struct ixgbe_ring *rx_ring,
+ unsigned int size)
+{
+ unsigned int truesize;
+
+#if (PAGE_SIZE < 8192)
+ truesize = ixgbe_rx_pg_size(rx_ring) / 2; /* Must be power-of-2 */
+#else
+ truesize = ring_uses_build_skb(rx_ring) ?
+ SKB_DATA_ALIGN(IXGBE_SKB_PAD + size) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
+ SKB_DATA_ALIGN(size);
+#endif
+ return truesize;
+}
+
static void ixgbe_rx_buffer_flip(struct ixgbe_ring *rx_ring,
struct ixgbe_rx_buffer *rx_buffer,
unsigned int size)
{
+ unsigned int truesize = ixgbe_rx_frame_truesize(rx_ring, size);
#if (PAGE_SIZE < 8192)
- unsigned int truesize = ixgbe_rx_pg_size(rx_ring) / 2;
-
rx_buffer->page_offset ^= truesize;
#else
- unsigned int truesize = ring_uses_build_skb(rx_ring) ?
- SKB_DATA_ALIGN(IXGBE_SKB_PAD + size) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
- SKB_DATA_ALIGN(size);
-
rx_buffer->page_offset += truesize;
#endif
}
@@ -2291,6 +2301,11 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
xdp.rxq = &rx_ring->xdp_rxq;
+ /* Frame size depend on rx_ring setup when PAGE_SIZE=4K */
+#if (PAGE_SIZE < 8192)
+ xdp.frame_sz = ixgbe_rx_frame_truesize(rx_ring, 0);
+#endif
+
while (likely(total_rx_packets < budget)) {
union ixgbe_adv_rx_desc *rx_desc;
struct ixgbe_rx_buffer *rx_buffer;
@@ -2324,7 +2339,10 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
xdp.data_hard_start = xdp.data -
ixgbe_rx_offset(rx_ring);
xdp.data_end = xdp.data + size;
-
+#if (PAGE_SIZE > 4096)
+ /* At larger PAGE_SIZE, frame_sz depend on len size */
+ xdp.frame_sz = ixgbe_rx_frame_truesize(rx_ring, size);
+#endif
skb = ixgbe_run_xdp(adapter, rx_ring, &xdp);
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Intel-wired-lan] [PATCH RFC v2 25/33] ixgbevf: add XDP frame size to VF driver
2020-04-08 11:50 [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Jesper Dangaard Brouer
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 24/33] ixgbe: add XDP frame size to driver Jesper Dangaard Brouer
@ 2020-04-08 11:52 ` Jesper Dangaard Brouer
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 26/33] i40e: add XDP frame size to driver Jesper Dangaard Brouer
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2020-04-08 11:52 UTC (permalink / raw)
To: intel-wired-lan
This patch mirrors the changes to ixgbe in previous patch.
This VF driver doesn't support XDP_REDIRECT, but correct tailroom is
still necessary for BPF-helper xdp_adjust_tail. In legacy-mode +
larger PAGE_SIZE, due to lacking tailroom, we accept that
xdp_adjust_tail shrink doesn't work.
Cc: intel-wired-lan at lists.osuosl.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 34 +++++++++++++++++----
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 4622c4ea2e46..62bc3e3b5b9c 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1095,19 +1095,31 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter,
return ERR_PTR(-result);
}
+static unsigned int ixgbevf_rx_frame_truesize(struct ixgbevf_ring *rx_ring,
+ unsigned int size)
+{
+ unsigned int truesize;
+
+#if (PAGE_SIZE < 8192)
+ truesize = ixgbevf_rx_pg_size(rx_ring) / 2; /* Must be power-of-2 */
+#else
+ truesize = ring_uses_build_skb(rx_ring) ?
+ SKB_DATA_ALIGN(IXGBEVF_SKB_PAD + size) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
+ SKB_DATA_ALIGN(size);
+#endif
+ return truesize;
+}
+
static void ixgbevf_rx_buffer_flip(struct ixgbevf_ring *rx_ring,
struct ixgbevf_rx_buffer *rx_buffer,
unsigned int size)
{
-#if (PAGE_SIZE < 8192)
- unsigned int truesize = ixgbevf_rx_pg_size(rx_ring) / 2;
+ unsigned int truesize = ixgbevf_rx_frame_truesize(rx_ring, size);
+#if (PAGE_SIZE < 8192)
rx_buffer->page_offset ^= truesize;
#else
- unsigned int truesize = ring_uses_build_skb(rx_ring) ?
- SKB_DATA_ALIGN(IXGBEVF_SKB_PAD + size) :
- SKB_DATA_ALIGN(size);
-
rx_buffer->page_offset += truesize;
#endif
}
@@ -1125,6 +1137,11 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
xdp.rxq = &rx_ring->xdp_rxq;
+ /* Frame size depend on rx_ring setup when PAGE_SIZE=4K */
+#if (PAGE_SIZE < 8192)
+ xdp.frame_sz = ixgbevf_rx_frame_truesize(rx_ring, 0);
+#endif
+
while (likely(total_rx_packets < budget)) {
struct ixgbevf_rx_buffer *rx_buffer;
union ixgbe_adv_rx_desc *rx_desc;
@@ -1157,7 +1174,10 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
xdp.data_hard_start = xdp.data -
ixgbevf_rx_offset(rx_ring);
xdp.data_end = xdp.data + size;
-
+#if (PAGE_SIZE > 4096)
+ /* At larger PAGE_SIZE, frame_sz depend on len size */
+ xdp.frame_sz = ixgbevf_rx_frame_truesize(rx_ring, size);
+#endif
skb = ixgbevf_run_xdp(adapter, rx_ring, &xdp);
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Intel-wired-lan] [PATCH RFC v2 26/33] i40e: add XDP frame size to driver
2020-04-08 11:50 [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Jesper Dangaard Brouer
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 24/33] ixgbe: add XDP frame size to driver Jesper Dangaard Brouer
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 25/33] ixgbevf: add XDP frame size to VF driver Jesper Dangaard Brouer
@ 2020-04-08 11:52 ` Jesper Dangaard Brouer
2020-04-08 21:48 ` David Miller
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 27/33] ice: " Jesper Dangaard Brouer
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2020-04-08 11:52 UTC (permalink / raw)
To: intel-wired-lan
This driver uses different memory models depending on PAGE_SIZE at
compile time. For PAGE_SIZE 4K it uses page splitting, meaning for
normal MTU frame size is 2048 bytes (and headroom 192 bytes). For
larger MTUs the driver still use page splitting, by allocating
order-1 pages (8192 bytes) for RX frames. For PAGE_SIZE larger than
4K, driver instead advance its rx_buffer->page_offset with the frame
size "truesize".
For XDP frame size calculations, this mean that in PAGE_SIZE larger
than 4K mode the frame_sz change on a per packet basis. For the page
split 4K PAGE_SIZE mode, xdp.frame_sz is more constant and can be
updated once outside the main NAPI loop.
The default setting in the driver uses build_skb(), which provides
the necessary headroom and tailroom for XDP-redirect in RX-frame
(in both modes).
There is one complication, which is legacy-rx mode (configurable via
ethtool priv-flags). There are zero headroom in this mode, which is a
requirement for XDP-redirect to work. The conversion to xdp_frame
(convert_to_xdp_frame) will detect this insufficient space, and
xdp_do_redirect() call will fail. This is deemed acceptable, as it
allows other XDP actions to still work in legacy-mode. In
legacy-mode + larger PAGE_SIZE due to lacking tailroom, we also
accept that xdp_adjust_tail shrink doesn't work.
Cc: intel-wired-lan at lists.osuosl.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 31 +++++++++++++++++++++++----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index b8496037ef7f..1fb6b1004dcb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1507,6 +1507,23 @@ static inline unsigned int i40e_rx_offset(struct i40e_ring *rx_ring)
return ring_uses_build_skb(rx_ring) ? I40E_SKB_PAD : 0;
}
+static inline unsigned int i40e_rx_frame_truesize(struct i40e_ring *rx_ring,
+ unsigned int size)
+{
+ unsigned int truesize;
+
+#if (PAGE_SIZE < 8192)
+ truesize = i40e_rx_pg_size(rx_ring) / 2; /* Must be power-of-2 */
+#else
+ truesize = i40e_rx_offset(rx_ring) ?
+ SKB_DATA_ALIGN(size + i40e_rx_offset(rx_ring)) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
+ SKB_DATA_ALIGN(size);
+#endif
+ return truesize;
+}
+
+
/**
* i40e_alloc_mapped_page - recycle or make a new page
* @rx_ring: ring to use
@@ -2246,13 +2263,11 @@ static void i40e_rx_buffer_flip(struct i40e_ring *rx_ring,
struct i40e_rx_buffer *rx_buffer,
unsigned int size)
{
-#if (PAGE_SIZE < 8192)
- unsigned int truesize = i40e_rx_pg_size(rx_ring) / 2;
+ unsigned int truesize = i40e_rx_frame_truesize(rx_ring, size);
+#if (PAGE_SIZE < 8192)
rx_buffer->page_offset ^= truesize;
#else
- unsigned int truesize = SKB_DATA_ALIGN(i40e_rx_offset(rx_ring) + size);
-
rx_buffer->page_offset += truesize;
#endif
}
@@ -2335,6 +2350,9 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
bool failure = false;
struct xdp_buff xdp;
+#if (PAGE_SIZE < 8192)
+ xdp.frame_sz = i40e_rx_frame_truesize(rx_ring, 0);
+#endif
xdp.rxq = &rx_ring->xdp_rxq;
while (likely(total_rx_packets < (unsigned int)budget)) {
@@ -2389,7 +2407,10 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
xdp.data_hard_start = xdp.data -
i40e_rx_offset(rx_ring);
xdp.data_end = xdp.data + size;
-
+#if (PAGE_SIZE > 4096)
+ /* At larger PAGE_SIZE, frame_sz depend on len size */
+ xdp.frame_sz = i40e_rx_frame_truesize(rx_ring, size);
+#endif
skb = i40e_run_xdp(rx_ring, &xdp);
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Intel-wired-lan] [PATCH RFC v2 26/33] i40e: add XDP frame size to driver
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 26/33] i40e: add XDP frame size to driver Jesper Dangaard Brouer
@ 2020-04-08 21:48 ` David Miller
2020-04-14 10:16 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2020-04-08 21:48 UTC (permalink / raw)
To: intel-wired-lan
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Wed, 08 Apr 2020 13:52:46 +0200
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index b8496037ef7f..1fb6b1004dcb 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -1507,6 +1507,23 @@ static inline unsigned int i40e_rx_offset(struct i40e_ring *rx_ring)
> return ring_uses_build_skb(rx_ring) ? I40E_SKB_PAD : 0;
> }
>
> +static inline unsigned int i40e_rx_frame_truesize(struct i40e_ring *rx_ring,
> + unsigned int size)
Please don't use inline in foo.c files. I noticed you properly elided this in
the ice changes so I wonder why it showed up here :-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH RFC v2 26/33] i40e: add XDP frame size to driver
2020-04-08 21:48 ` David Miller
@ 2020-04-14 10:16 ` Jesper Dangaard Brouer
0 siblings, 0 replies; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2020-04-14 10:16 UTC (permalink / raw)
To: intel-wired-lan
On Wed, 08 Apr 2020 14:48:45 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Wed, 08 Apr 2020 13:52:46 +0200
>
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > index b8496037ef7f..1fb6b1004dcb 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > @@ -1507,6 +1507,23 @@ static inline unsigned int i40e_rx_offset(struct i40e_ring *rx_ring)
> > return ring_uses_build_skb(rx_ring) ? I40E_SKB_PAD : 0;
> > }
> >
> > +static inline unsigned int i40e_rx_frame_truesize(struct i40e_ring *rx_ring,
> > + unsigned int size)
>
> Please don't use inline in foo.c files. I noticed you properly elided this in
> the ice changes so I wonder why it showed up here :-)
Yes, I know I should not do this. It got here by copy-paste accident,
as I first had ixgbe function in a header file, and later I decided to
move this into the ixgbe C-file, but I had already copy-pasted this
into i40e driver ;-)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH RFC v2 27/33] ice: add XDP frame size to driver
2020-04-08 11:50 [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Jesper Dangaard Brouer
` (2 preceding siblings ...)
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 26/33] i40e: add XDP frame size to driver Jesper Dangaard Brouer
@ 2020-04-08 11:52 ` Jesper Dangaard Brouer
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 28/33] xdp: for Intel AF_XDP drivers add XDP frame_sz Jesper Dangaard Brouer
2020-04-08 16:55 ` [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Alexei Starovoitov
5 siblings, 0 replies; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2020-04-08 11:52 UTC (permalink / raw)
To: intel-wired-lan
This driver uses different memory models depending on PAGE_SIZE at
compile time. For PAGE_SIZE 4K it uses page splitting, meaning for
normal MTU frame size is 2048 bytes (and headroom 192 bytes). For
larger MTUs the driver still use page splitting, by allocating
order-1 pages (8192 bytes) for RX frames. For PAGE_SIZE larger than
4K, driver instead advance its rx_buffer->page_offset with the frame
size "truesize".
For XDP frame size calculations, this mean that in PAGE_SIZE larger
than 4K mode the frame_sz change on a per packet basis. For the page
split 4K PAGE_SIZE mode, xdp.frame_sz is more constant and can be
updated once outside the main NAPI loop.
The default setting in the driver uses build_skb(), which provides
the necessary headroom and tailroom for XDP-redirect in RX-frame
(in both modes).
There is one complication, which is legacy-rx mode (configurable via
ethtool priv-flags). There are zero headroom in this mode, which is a
requirement for XDP-redirect to work. The conversion to xdp_frame
(convert_to_xdp_frame) will detect this insufficient space, and
xdp_do_redirect() call will fail. This is deemed acceptable, as it
allows other XDP actions to still work in legacy-mode. In
legacy-mode + larger PAGE_SIZE due to lacking tailroom, we also
accept that xdp_adjust_tail shrink doesn't work.
Cc: intel-wired-lan at lists.osuosl.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/ethernet/intel/ice/ice_txrx.c | 34 +++++++++++++++++++++--------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index f67e8362958c..695f86694224 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -423,6 +423,22 @@ static unsigned int ice_rx_offset(struct ice_ring *rx_ring)
return 0;
}
+static unsigned int ice_rx_frame_truesize(struct ice_ring *rx_ring,
+ unsigned int size)
+{
+ unsigned int truesize;
+
+#if (PAGE_SIZE < 8192)
+ truesize = ice_rx_pg_size(rx_ring) / 2; /* Must be power-of-2 */
+#else
+ truesize = ice_rx_offset(rx_ring) ?
+ SKB_DATA_ALIGN(ice_rx_offset(rx_ring) + size) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
+ SKB_DATA_ALIGN(size)
+#endif
+ return truesize;
+}
+
/**
* ice_run_xdp - Executes an XDP program on initialized xdp_buff
* @rx_ring: Rx ring
@@ -991,6 +1007,10 @@ static int ice_clean_rx_irq(struct ice_ring *rx_ring, int budget)
bool failure;
xdp.rxq = &rx_ring->xdp_rxq;
+ /* Frame size depend on rx_ring setup when PAGE_SIZE=4K */
+#if (PAGE_SIZE < 8192)
+ xdp.frame_sz = ice_rx_frame_truesize(rx_ring, 0);
+#endif
/* start the loop to process Rx packets bounded by 'budget' */
while (likely(total_rx_pkts < (unsigned int)budget)) {
@@ -1038,6 +1058,10 @@ static int ice_clean_rx_irq(struct ice_ring *rx_ring, int budget)
xdp.data_hard_start = xdp.data - ice_rx_offset(rx_ring);
xdp.data_meta = xdp.data;
xdp.data_end = xdp.data + size;
+#if (PAGE_SIZE > 4096)
+ /* At larger PAGE_SIZE, frame_sz depend on len size */
+ xdp.frame_sz = ice_rx_frame_truesize(rx_ring, size);
+#endif
rcu_read_lock();
xdp_prog = READ_ONCE(rx_ring->xdp_prog);
@@ -1051,16 +1075,8 @@ static int ice_clean_rx_irq(struct ice_ring *rx_ring, int budget)
if (!xdp_res)
goto construct_skb;
if (xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR)) {
- unsigned int truesize;
-
-#if (PAGE_SIZE < 8192)
- truesize = ice_rx_pg_size(rx_ring) / 2;
-#else
- truesize = SKB_DATA_ALIGN(ice_rx_offset(rx_ring) +
- size);
-#endif
xdp_xmit |= xdp_res;
- ice_rx_buf_adjust_pg_offset(rx_buf, truesize);
+ ice_rx_buf_adjust_pg_offset(rx_buf, xdp.frame_sz);
} else {
rx_buf->pagecnt_bias++;
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Intel-wired-lan] [PATCH RFC v2 28/33] xdp: for Intel AF_XDP drivers add XDP frame_sz
2020-04-08 11:50 [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Jesper Dangaard Brouer
` (3 preceding siblings ...)
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 27/33] ice: " Jesper Dangaard Brouer
@ 2020-04-08 11:52 ` Jesper Dangaard Brouer
2020-04-08 17:31 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?=
2020-04-08 16:55 ` [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Alexei Starovoitov
5 siblings, 1 reply; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2020-04-08 11:52 UTC (permalink / raw)
To: intel-wired-lan
Intel drivers implement native AF_XDP zerocopy in separate C-files,
that have its own invocation of bpf_prog_run_xdp(). The setup of
xdp_buff is also handled in separately from normal code path.
This patch update XDP frame_sz for AF_XDP zerocopy drivers i40e, ice
and ixgbe, as the code changes needed are very similar. Introduce a
helper function xsk_umem_xdp_frame_sz() for calculating frame size.
Cc: intel-wired-lan at lists.osuosl.org
Cc: Bj?rn T?pel <bjorn.topel@intel.com>
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 ++
drivers/net/ethernet/intel/ice/ice_xsk.c | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 2 ++
include/net/xdp_sock.h | 11 +++++++++++
4 files changed, 17 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 0b7d29192b2c..2b9184aead5f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -531,12 +531,14 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
{
unsigned int total_rx_bytes = 0, total_rx_packets = 0;
u16 cleaned_count = I40E_DESC_UNUSED(rx_ring);
+ struct xdp_umem *umem = rx_ring->xsk_umem;
unsigned int xdp_res, xdp_xmit = 0;
bool failure = false;
struct sk_buff *skb;
struct xdp_buff xdp;
xdp.rxq = &rx_ring->xdp_rxq;
+ xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
while (likely(total_rx_packets < (unsigned int)budget)) {
struct i40e_rx_buffer *bi;
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 8279db15e870..23e5515d4527 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -840,11 +840,13 @@ int ice_clean_rx_irq_zc(struct ice_ring *rx_ring, int budget)
{
unsigned int total_rx_bytes = 0, total_rx_packets = 0;
u16 cleaned_count = ICE_DESC_UNUSED(rx_ring);
+ struct xdp_umem *umem = rx_ring->xsk_umem;
unsigned int xdp_xmit = 0;
bool failure = false;
struct xdp_buff xdp;
xdp.rxq = &rx_ring->xdp_rxq;
+ xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
while (likely(total_rx_packets < (unsigned int)budget)) {
union ice_32b_rx_flex_desc *rx_desc;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 74b540ebb3dc..a656ee9a1fae 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -431,12 +431,14 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
unsigned int total_rx_bytes = 0, total_rx_packets = 0;
struct ixgbe_adapter *adapter = q_vector->adapter;
u16 cleaned_count = ixgbe_desc_unused(rx_ring);
+ struct xdp_umem *umem = rx_ring->xsk_umem;
unsigned int xdp_res, xdp_xmit = 0;
bool failure = false;
struct sk_buff *skb;
struct xdp_buff xdp;
xdp.rxq = &rx_ring->xdp_rxq;
+ xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
while (likely(total_rx_packets < budget)) {
union ixgbe_adv_rx_desc *rx_desc;
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index e86ec48ef627..1cd1ec3cea97 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -237,6 +237,12 @@ static inline u64 xsk_umem_adjust_offset(struct xdp_umem *umem, u64 address,
else
return address + offset;
}
+
+static inline u32 xsk_umem_xdp_frame_sz(struct xdp_umem *umem)
+{
+ return umem->chunk_size_nohr + umem->headroom;
+}
+
#else
static inline int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
{
@@ -367,6 +373,11 @@ static inline u64 xsk_umem_adjust_offset(struct xdp_umem *umem, u64 handle,
return 0;
}
+static inline u32 xsk_umem_xdp_frame_sz(struct xdp_umem *umem)
+{
+ return 0;
+}
+
static inline int __xsk_map_redirect(struct xdp_sock *xs, struct xdp_buff *xdp)
{
return -EOPNOTSUPP;
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Intel-wired-lan] [PATCH RFC v2 28/33] xdp: for Intel AF_XDP drivers add XDP frame_sz
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 28/33] xdp: for Intel AF_XDP drivers add XDP frame_sz Jesper Dangaard Brouer
@ 2020-04-08 17:31 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?=
2020-04-09 9:33 ` Maxim Mikityanskiy
0 siblings, 1 reply; 11+ messages in thread
From: =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= @ 2020-04-08 17:31 UTC (permalink / raw)
To: intel-wired-lan
On 2020-04-08 13:52, Jesper Dangaard Brouer wrote:
> Intel drivers implement native AF_XDP zerocopy in separate C-files,
> that have its own invocation of bpf_prog_run_xdp(). The setup of
> xdp_buff is also handled in separately from normal code path.
>
> This patch update XDP frame_sz for AF_XDP zerocopy drivers i40e, ice
> and ixgbe, as the code changes needed are very similar. Introduce a
> helper function xsk_umem_xdp_frame_sz() for calculating frame size.
>
> Cc: intel-wired-lan at lists.osuosl.org
> Cc: Bj?rn T?pel <bjorn.topel@intel.com>
> Cc: Magnus Karlsson <magnus.karlsson@intel.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Thanks for the patch, Jesper! Note that mlx5 has AF_XDP support as well,
and might need similar changes. Adding Max for input!
For the Intel drivers, and core AF_XDP:
Acked-by: Bj?rn T?pel <bjorn.topel@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 2 ++
> drivers/net/ethernet/intel/ice/ice_xsk.c | 2 ++
> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 2 ++
> include/net/xdp_sock.h | 11 +++++++++++
> 4 files changed, 17 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> index 0b7d29192b2c..2b9184aead5f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> @@ -531,12 +531,14 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
> {
> unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> u16 cleaned_count = I40E_DESC_UNUSED(rx_ring);
> + struct xdp_umem *umem = rx_ring->xsk_umem;
> unsigned int xdp_res, xdp_xmit = 0;
> bool failure = false;
> struct sk_buff *skb;
> struct xdp_buff xdp;
>
> xdp.rxq = &rx_ring->xdp_rxq;
> + xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
>
> while (likely(total_rx_packets < (unsigned int)budget)) {
> struct i40e_rx_buffer *bi;
> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
> index 8279db15e870..23e5515d4527 100644
> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> @@ -840,11 +840,13 @@ int ice_clean_rx_irq_zc(struct ice_ring *rx_ring, int budget)
> {
> unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> u16 cleaned_count = ICE_DESC_UNUSED(rx_ring);
> + struct xdp_umem *umem = rx_ring->xsk_umem;
> unsigned int xdp_xmit = 0;
> bool failure = false;
> struct xdp_buff xdp;
>
> xdp.rxq = &rx_ring->xdp_rxq;
> + xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
>
> while (likely(total_rx_packets < (unsigned int)budget)) {
> union ice_32b_rx_flex_desc *rx_desc;
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index 74b540ebb3dc..a656ee9a1fae 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -431,12 +431,14 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
> unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> struct ixgbe_adapter *adapter = q_vector->adapter;
> u16 cleaned_count = ixgbe_desc_unused(rx_ring);
> + struct xdp_umem *umem = rx_ring->xsk_umem;
> unsigned int xdp_res, xdp_xmit = 0;
> bool failure = false;
> struct sk_buff *skb;
> struct xdp_buff xdp;
>
> xdp.rxq = &rx_ring->xdp_rxq;
> + xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
>
> while (likely(total_rx_packets < budget)) {
> union ixgbe_adv_rx_desc *rx_desc;
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index e86ec48ef627..1cd1ec3cea97 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -237,6 +237,12 @@ static inline u64 xsk_umem_adjust_offset(struct xdp_umem *umem, u64 address,
> else
> return address + offset;
> }
> +
> +static inline u32 xsk_umem_xdp_frame_sz(struct xdp_umem *umem)
> +{
> + return umem->chunk_size_nohr + umem->headroom;
> +}
> +
> #else
> static inline int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
> {
> @@ -367,6 +373,11 @@ static inline u64 xsk_umem_adjust_offset(struct xdp_umem *umem, u64 handle,
> return 0;
> }
>
> +static inline u32 xsk_umem_xdp_frame_sz(struct xdp_umem *umem)
> +{
> + return 0;
> +}
> +
> static inline int __xsk_map_redirect(struct xdp_sock *xs, struct xdp_buff *xdp)
> {
> return -EOPNOTSUPP;
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* [Intel-wired-lan] [PATCH RFC v2 28/33] xdp: for Intel AF_XDP drivers add XDP frame_sz
2020-04-08 17:31 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?=
@ 2020-04-09 9:33 ` Maxim Mikityanskiy
0 siblings, 0 replies; 11+ messages in thread
From: Maxim Mikityanskiy @ 2020-04-09 9:33 UTC (permalink / raw)
To: intel-wired-lan
On 2020-04-08 20:31, Bj?rn T?pel wrote:
> On 2020-04-08 13:52, Jesper Dangaard Brouer wrote:
>> Intel drivers implement native AF_XDP zerocopy in separate C-files,
>> that have its own invocation of bpf_prog_run_xdp(). The setup of
>> xdp_buff is also handled in separately from normal code path.
>>
>> This patch update XDP frame_sz for AF_XDP zerocopy drivers i40e, ice
>> and ixgbe, as the code changes needed are very similar.? Introduce a
>> helper function xsk_umem_xdp_frame_sz() for calculating frame size.
>>
>> Cc: intel-wired-lan at lists.osuosl.org
>> Cc: Bj?rn T?pel <bjorn.topel@intel.com>
>> Cc: Magnus Karlsson <magnus.karlsson@intel.com>
>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Thanks for the patch, Jesper! Note that mlx5 has AF_XDP support as well,
> and might need similar changes. Adding Max for input!
Thanks for drawing my attention to this series, Bj?rn! I commented
regarding frame_sz calculation under the mlx5 patch (17/33).
> For the Intel drivers, and core AF_XDP:
> Acked-by: Bj?rn T?pel <bjorn.topel@intel.com>
>
>> ---
>> ? drivers/net/ethernet/intel/i40e/i40e_xsk.c?? |??? 2 ++
>> ? drivers/net/ethernet/intel/ice/ice_xsk.c???? |??? 2 ++
>> ? drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c |??? 2 ++
>> ? include/net/xdp_sock.h?????????????????????? |?? 11 +++++++++++
>> ? 4 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> index 0b7d29192b2c..2b9184aead5f 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> @@ -531,12 +531,14 @@ int i40e_clean_rx_irq_zc(struct i40e_ring
>> *rx_ring, int budget)
>> ? {
>> ????? unsigned int total_rx_bytes = 0, total_rx_packets = 0;
>> ????? u16 cleaned_count = I40E_DESC_UNUSED(rx_ring);
>> +??? struct xdp_umem *umem = rx_ring->xsk_umem;
>> ????? unsigned int xdp_res, xdp_xmit = 0;
>> ????? bool failure = false;
>> ????? struct sk_buff *skb;
>> ????? struct xdp_buff xdp;
>> ????? xdp.rxq = &rx_ring->xdp_rxq;
>> +??? xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
>> ????? while (likely(total_rx_packets < (unsigned int)budget)) {
>> ????????? struct i40e_rx_buffer *bi;
>> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c
>> b/drivers/net/ethernet/intel/ice/ice_xsk.c
>> index 8279db15e870..23e5515d4527 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
>> @@ -840,11 +840,13 @@ int ice_clean_rx_irq_zc(struct ice_ring
>> *rx_ring, int budget)
>> ? {
>> ????? unsigned int total_rx_bytes = 0, total_rx_packets = 0;
>> ????? u16 cleaned_count = ICE_DESC_UNUSED(rx_ring);
>> +??? struct xdp_umem *umem = rx_ring->xsk_umem;
>> ????? unsigned int xdp_xmit = 0;
>> ????? bool failure = false;
>> ????? struct xdp_buff xdp;
>> ????? xdp.rxq = &rx_ring->xdp_rxq;
>> +??? xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
>> ????? while (likely(total_rx_packets < (unsigned int)budget)) {
>> ????????? union ice_32b_rx_flex_desc *rx_desc;
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
>> index 74b540ebb3dc..a656ee9a1fae 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
>> @@ -431,12 +431,14 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector
>> *q_vector,
>> ????? unsigned int total_rx_bytes = 0, total_rx_packets = 0;
>> ????? struct ixgbe_adapter *adapter = q_vector->adapter;
>> ????? u16 cleaned_count = ixgbe_desc_unused(rx_ring);
>> +??? struct xdp_umem *umem = rx_ring->xsk_umem;
>> ????? unsigned int xdp_res, xdp_xmit = 0;
>> ????? bool failure = false;
>> ????? struct sk_buff *skb;
>> ????? struct xdp_buff xdp;
>> ????? xdp.rxq = &rx_ring->xdp_rxq;
>> +??? xdp.frame_sz = xsk_umem_xdp_frame_sz(umem);
>> ????? while (likely(total_rx_packets < budget)) {
>> ????????? union ixgbe_adv_rx_desc *rx_desc;
>> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
>> index e86ec48ef627..1cd1ec3cea97 100644
>> --- a/include/net/xdp_sock.h
>> +++ b/include/net/xdp_sock.h
>> @@ -237,6 +237,12 @@ static inline u64 xsk_umem_adjust_offset(struct
>> xdp_umem *umem, u64 address,
>> ????? else
>> ????????? return address + offset;
>> ? }
>> +
>> +static inline u32 xsk_umem_xdp_frame_sz(struct xdp_umem *umem)
>> +{
>> +??? return umem->chunk_size_nohr + umem->headroom;
>> +}
>> +
This new function may be used in mlx5 for mlx5e_build_xsk_param.
>> ? #else
>> ? static inline int xsk_generic_rcv(struct xdp_sock *xs, struct
>> xdp_buff *xdp)
>> ? {
>> @@ -367,6 +373,11 @@ static inline u64 xsk_umem_adjust_offset(struct
>> xdp_umem *umem, u64 handle,
>> ????? return 0;
>> ? }
>> +static inline u32 xsk_umem_xdp_frame_sz(struct xdp_umem *umem)
>> +{
>> +??? return 0;
>> +}
>> +
>> ? static inline int __xsk_map_redirect(struct xdp_sock *xs, struct
>> xdp_buff *xdp)
>> ? {
>> ????? return -EOPNOTSUPP;
>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size
2020-04-08 11:50 [Intel-wired-lan] [PATCH RFC v2 00/33] XDP extend with knowledge of frame size Jesper Dangaard Brouer
` (4 preceding siblings ...)
2020-04-08 11:52 ` [Intel-wired-lan] [PATCH RFC v2 28/33] xdp: for Intel AF_XDP drivers add XDP frame_sz Jesper Dangaard Brouer
@ 2020-04-08 16:55 ` Alexei Starovoitov
5 siblings, 0 replies; 11+ messages in thread
From: Alexei Starovoitov @ 2020-04-08 16:55 UTC (permalink / raw)
To: intel-wired-lan
On Wed, Apr 08, 2020 at 01:50:34PM +0200, Jesper Dangaard Brouer wrote:
> RFC-note: This is only an RFC because net-next is closed.
> - Please ACK patches you like, then I will collect those for later.
>
> XDP have evolved to support several frame sizes, but xdp_buff was not
> updated with this information. This have caused the side-effect that
> XDP frame data hard end is unknown. This have limited the BPF-helper
> bpf_xdp_adjust_tail to only shrink the packet. This patchset address
> this and add packet tail extend/grow.
>
> The purpose of the patchset is ALSO to reserve a memory area that can be
> used for storing extra information, specifically for extending XDP with
> multi-buffer support. One proposal is to use same layout as
> skb_shared_info, which is why this area is currently 320 bytes.
>
> When converting xdp_frame to SKB (veth and cpumap), the full tailroom
> area can now be used and SKB truesize is now correct. For most
> drivers this result in a much larger tailroom in SKB "head" data
> area. The network stack can now take advantage of this when doing SKB
> coalescing. Thus, a good driver test is to use xdp_redirect_cpu from
> samples/bpf/ and do some TCP stream testing.
I did a quick look through the patches. Overall looks good to me.
Nice to see selftests as well.
If you can add an xdp selftest that uses generic xdp on lo or veth
that would be awesome.
I rarely run test_xdp*.sh tests, but run test_progs on every commit.
So having more comprehensive xdp test as part of test_progs will help us
catch breakage sooner.
^ permalink raw reply [flat|nested] 11+ messages in thread