All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31
@ 2022-01-31 18:31 Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 1/9] i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Tony Nguyen, netdev, alexandr.lobakin, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf

Alexander Lobakin says:

This is an interpolation of [0] to other Intel Ethernet drivers
(and is (re)based on its code).
The main aim is to keep XDP metadata not only in case with
build_skb(), but also when we do napi_alloc_skb() + memcpy().

All Intel drivers suffers from the same here:
 - metadata gets lost on XDP_PASS in legacy-rx;
 - excessive headroom allocation on XSK Rx to skbs;
 - metadata gets lost on XSK Rx to skbs.

Those get especially actual in XDP Hints upcoming.
I couldn't have addressed the first one for all Intel drivers due to
that they don't reserve any headroom for now in legacy-rx mode even
with XDP enabled. This is hugely wrong, but requires quite a bunch
of work and a separate series. Luckily, ice doesn't suffer from
that.
igc has 1 and 3 already fixed in [0].

[0] https://lore.kernel.org/netdev/163700856423.565980.10162564921347693758.stgit@firesoul

The following are changes since commit b43471cc10327f098d5a72918cd59fcb91546ca3:
  Merge branch 'mana-XDP-counters'
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 10GbE

Alexander Lobakin (9):
  i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  i40e: respect metadata on XSK Rx to skb
  ice: respect metadata in legacy-rx/ice_construct_skb()
  ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  ice: respect metadata on XSK Rx to skb
  igc: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  ixgbe: pass bi->xdp to ixgbe_construct_skb_zc() directly
  ixgbe: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  ixgbe: respect metadata on XSK Rx to skb

 drivers/net/ethernet/intel/i40e/i40e_xsk.c   | 16 +++++++-----
 drivers/net/ethernet/intel/ice/ice_txrx.c    | 15 ++++++++---
 drivers/net/ethernet/intel/ice/ice_xsk.c     | 16 +++++++-----
 drivers/net/ethernet/intel/igc/igc_main.c    | 13 +++++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 27 ++++++++++++--------
 5 files changed, 54 insertions(+), 33 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next 1/9] i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 2/9] i40e: respect metadata " Tony Nguyen
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf, sassmann,
	Jesper Dangaard Brouer, Kiran Bhandare

From: Alexander Lobakin <alexandr.lobakin@intel.com>

{__,}napi_alloc_skb() allocates and reserves additional NET_SKB_PAD
+ NET_IP_ALIGN for any skb.
OTOH, i40e_construct_skb_zc() currently allocates and reserves
additional `xdp->data - xdp->data_hard_start`, which is
XDP_PACKET_HEADROOM for XSK frames.
There's no need for that at all as the frame is post-XDP and will
go only to the networking stack core.
Pass the size of the actual data only to __napi_alloc_skb() and
don't reserve anything. This will give enough headroom for stack
processing.

Fixes: 0a714186d3c0 ("i40e: add AF_XDP zero-copy Rx support")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_xsk.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 945b1bb9c6f4..a449c84fe357 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -246,13 +246,11 @@ static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring,
 	struct sk_buff *skb;
 
 	/* allocate a skb to store the frags */
-	skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
-			       xdp->data_end - xdp->data_hard_start,
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, datasize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		goto out;
 
-	skb_reserve(skb, xdp->data - xdp->data_hard_start);
 	memcpy(__skb_put(skb, datasize), xdp->data, datasize);
 	if (metasize)
 		skb_metadata_set(skb, metasize);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 2/9] i40e: respect metadata on XSK Rx to skb
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 1/9] i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 3/9] ice: respect metadata in legacy-rx/ice_construct_skb() Tony Nguyen
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf, sassmann,
	Jesper Dangaard Brouer, Kiran Bhandare

From: Alexander Lobakin <alexandr.lobakin@intel.com>

For now, if the XDP prog returns XDP_PASS on XSK, the metadata will
be lost as it doesn't get copied to the skb.

Copy it along with the frame headers. Account its size on skb
allocation, and when copying just treat it as a part of the frame
and do a pull after to "move" it to the "reserved" zone.

net_prefetch() xdp->data_meta and align the copy size to speed-up
memcpy() a little and better match i40e_construct_skb().

Fixes: 0a714186d3c0 ("i40e: add AF_XDP zero-copy Rx support")
Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_xsk.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index a449c84fe357..67e9844e2076 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -241,19 +241,25 @@ bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count)
 static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring,
 					     struct xdp_buff *xdp)
 {
+	unsigned int totalsize = xdp->data_end - xdp->data_meta;
 	unsigned int metasize = xdp->data - xdp->data_meta;
-	unsigned int datasize = xdp->data_end - xdp->data;
 	struct sk_buff *skb;
 
+	net_prefetch(xdp->data_meta);
+
 	/* allocate a skb to store the frags */
-	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, datasize,
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, totalsize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		goto out;
 
-	memcpy(__skb_put(skb, datasize), xdp->data, datasize);
-	if (metasize)
+	memcpy(__skb_put(skb, totalsize), xdp->data_meta,
+	       ALIGN(totalsize, sizeof(long)));
+
+	if (metasize) {
 		skb_metadata_set(skb, metasize);
+		__skb_pull(skb, metasize);
+	}
 
 out:
 	xsk_buff_free(xdp);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 3/9] ice: respect metadata in legacy-rx/ice_construct_skb()
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 1/9] i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 2/9] i40e: respect metadata " Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 4/9] ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf,
	Jesper Dangaard Brouer, Kiran Bhandare

From: Alexander Lobakin <alexandr.lobakin@intel.com>

In "legacy-rx" mode represented by ice_construct_skb(), we can
still use XDP (and XDP metadata), but after XDP_PASS the metadata
will be lost as it doesn't get copied to the skb.
Copy it along with the frame headers. Account its size on skb
allocation, and when copying just treat it as a part of the frame
and do a pull after to "move" it to the "reserved" zone.
Point net_prefetch() to xdp->data_meta instead of data. This won't
change anything when the meta is not here, but will save some cache
misses otherwise.

Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_txrx.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 3e38695f1c9d..c2258bee8ecb 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -983,15 +983,17 @@ static struct sk_buff *
 ice_construct_skb(struct ice_rx_ring *rx_ring, struct ice_rx_buf *rx_buf,
 		  struct xdp_buff *xdp)
 {
+	unsigned int metasize = xdp->data - xdp->data_meta;
 	unsigned int size = xdp->data_end - xdp->data;
 	unsigned int headlen;
 	struct sk_buff *skb;
 
 	/* prefetch first cache line of first page */
-	net_prefetch(xdp->data);
+	net_prefetch(xdp->data_meta);
 
 	/* allocate a skb to store the frags */
-	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, ICE_RX_HDR_SIZE,
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
+			       ICE_RX_HDR_SIZE + metasize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		return NULL;
@@ -1003,8 +1005,13 @@ ice_construct_skb(struct ice_rx_ring *rx_ring, struct ice_rx_buf *rx_buf,
 		headlen = eth_get_headlen(skb->dev, xdp->data, ICE_RX_HDR_SIZE);
 
 	/* align pull length to size of long to optimize memcpy performance */
-	memcpy(__skb_put(skb, headlen), xdp->data, ALIGN(headlen,
-							 sizeof(long)));
+	memcpy(__skb_put(skb, headlen + metasize), xdp->data_meta,
+	       ALIGN(headlen + metasize, sizeof(long)));
+
+	if (metasize) {
+		skb_metadata_set(skb, metasize);
+		__skb_pull(skb, metasize);
+	}
 
 	/* if we exhaust the linear part then add what is left as a frag */
 	size -= headlen;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 4/9] ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
                   ` (2 preceding siblings ...)
  2022-01-31 18:31 ` [PATCH net-next 3/9] ice: respect metadata in legacy-rx/ice_construct_skb() Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 5/9] ice: respect metadata " Tony Nguyen
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf, Kiran Bhandare

From: Alexander Lobakin <alexandr.lobakin@intel.com>

{__,}napi_alloc_skb() allocates and reserves additional NET_SKB_PAD
+ NET_IP_ALIGN for any skb.
OTOH, ice_construct_skb_zc() currently allocates and reserves
additional `xdp->data - xdp->data_hard_start`, which is
XDP_PACKET_HEADROOM for XSK frames.
There's no need for that at all as the frame is post-XDP and will
go only to the networking stack core.
Pass the size of the actual data only to __napi_alloc_skb() and
don't reserve anything. This will give enough headroom for stack
processing.

Fixes: 2d4238f55697 ("ice: Add support for AF_XDP")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_xsk.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 2388837d6d6c..8fd8052edf09 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -428,17 +428,15 @@ static void ice_bump_ntc(struct ice_rx_ring *rx_ring)
 static struct sk_buff *
 ice_construct_skb_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp)
 {
-	unsigned int datasize_hard = xdp->data_end - xdp->data_hard_start;
 	unsigned int metasize = xdp->data - xdp->data_meta;
 	unsigned int datasize = xdp->data_end - xdp->data;
 	struct sk_buff *skb;
 
-	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, datasize_hard,
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, datasize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		return NULL;
 
-	skb_reserve(skb, xdp->data - xdp->data_hard_start);
 	memcpy(__skb_put(skb, datasize), xdp->data, datasize);
 	if (metasize)
 		skb_metadata_set(skb, metasize);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 5/9] ice: respect metadata on XSK Rx to skb
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
                   ` (3 preceding siblings ...)
  2022-01-31 18:31 ` [PATCH net-next 4/9] ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 6/9] igc: don't reserve excessive XDP_PACKET_HEADROOM " Tony Nguyen
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf,
	Jesper Dangaard Brouer, Kiran Bhandare

From: Alexander Lobakin <alexandr.lobakin@intel.com>

For now, if the XDP prog returns XDP_PASS on XSK, the metadata will
be lost as it doesn't get copied to the skb.

Copy it along with the frame headers. Account its size on skb
allocation, and when copying just treat it as a part of the frame
and do a pull after to "move" it to the "reserved" zone.

net_prefetch() xdp->data_meta and align the copy size to speed-up
memcpy() a little and better match ice_construct_skb().

Fixes: 2d4238f55697 ("ice: Add support for AF_XDP")
Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_xsk.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 8fd8052edf09..feb874bde171 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -428,18 +428,24 @@ static void ice_bump_ntc(struct ice_rx_ring *rx_ring)
 static struct sk_buff *
 ice_construct_skb_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp)
 {
+	unsigned int totalsize = xdp->data_end - xdp->data_meta;
 	unsigned int metasize = xdp->data - xdp->data_meta;
-	unsigned int datasize = xdp->data_end - xdp->data;
 	struct sk_buff *skb;
 
-	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, datasize,
+	net_prefetch(xdp->data_meta);
+
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, totalsize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		return NULL;
 
-	memcpy(__skb_put(skb, datasize), xdp->data, datasize);
-	if (metasize)
+	memcpy(__skb_put(skb, totalsize), xdp->data_meta,
+	       ALIGN(totalsize, sizeof(long)));
+
+	if (metasize) {
 		skb_metadata_set(skb, metasize);
+		__skb_pull(skb, metasize);
+	}
 
 	xsk_buff_free(xdp);
 	return skb;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 6/9] igc: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
                   ` (4 preceding siblings ...)
  2022-01-31 18:31 ` [PATCH net-next 5/9] ice: respect metadata " Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 7/9] ixgbe: pass bi->xdp to ixgbe_construct_skb_zc() directly Tony Nguyen
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf, Nechama Kraus

From: Alexander Lobakin <alexandr.lobakin@intel.com>

{__,}napi_alloc_skb() allocates and reserves additional NET_SKB_PAD
+ NET_IP_ALIGN for any skb.
OTOH, igc_construct_skb_zc() currently allocates and reserves
additional `xdp->data_meta - xdp->data_hard_start`, which is about
XDP_PACKET_HEADROOM for XSK frames.
There's no need for that at all as the frame is post-XDP and will
go only to the networking stack core.
Pass the size of the actual data only (+ meta) to
__napi_alloc_skb() and don't reserve anything. This will give
enough headroom for stack processing.
Also, net_prefetch() xdp->data_meta and align the copy size to
speed-up memcpy() a little and better match igc_construct_skb().

Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Tested-by: Nechama Kraus <nechamax.kraus@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 6b51baadee3d..b965fb809d84 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -2446,19 +2446,20 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
 static struct sk_buff *igc_construct_skb_zc(struct igc_ring *ring,
 					    struct xdp_buff *xdp)
 {
+	unsigned int totalsize = xdp->data_end - xdp->data_meta;
 	unsigned int metasize = xdp->data - xdp->data_meta;
-	unsigned int datasize = xdp->data_end - xdp->data;
-	unsigned int totalsize = metasize + datasize;
 	struct sk_buff *skb;
 
-	skb = __napi_alloc_skb(&ring->q_vector->napi,
-			       xdp->data_end - xdp->data_hard_start,
+	net_prefetch(xdp->data_meta);
+
+	skb = __napi_alloc_skb(&ring->q_vector->napi, totalsize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		return NULL;
 
-	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
-	memcpy(__skb_put(skb, totalsize), xdp->data_meta, totalsize);
+	memcpy(__skb_put(skb, totalsize), xdp->data_meta,
+	       ALIGN(totalsize, sizeof(long)));
+
 	if (metasize) {
 		skb_metadata_set(skb, metasize);
 		__skb_pull(skb, metasize);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 7/9] ixgbe: pass bi->xdp to ixgbe_construct_skb_zc() directly
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
                   ` (5 preceding siblings ...)
  2022-01-31 18:31 ` [PATCH net-next 6/9] igc: don't reserve excessive XDP_PACKET_HEADROOM " Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 8/9] ixgbe: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf,
	Sandeep Penigalapati

From: Alexander Lobakin <alexandr.lobakin@intel.com>

To not dereference bi->xdp each time in ixgbe_construct_skb_zc(),
pass bi->xdp as an argument instead of bi. We can also call
xsk_buff_free() outside of the function as well as assign bi->xdp
to NULL, which seems to make it closer to its name.

Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Tested-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index b3fd8e5cd85b..5447bfb31838 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -207,26 +207,24 @@ bool ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 count)
 }
 
 static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
-					      struct ixgbe_rx_buffer *bi)
+					      const struct xdp_buff *xdp)
 {
-	unsigned int metasize = bi->xdp->data - bi->xdp->data_meta;
-	unsigned int datasize = bi->xdp->data_end - bi->xdp->data;
+	unsigned int metasize = xdp->data - xdp->data_meta;
+	unsigned int datasize = xdp->data_end - xdp->data;
 	struct sk_buff *skb;
 
 	/* allocate a skb to store the frags */
 	skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
-			       bi->xdp->data_end - bi->xdp->data_hard_start,
+			       xdp->data_end - xdp->data_hard_start,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		return NULL;
 
-	skb_reserve(skb, bi->xdp->data - bi->xdp->data_hard_start);
-	memcpy(__skb_put(skb, datasize), bi->xdp->data, datasize);
+	skb_reserve(skb, xdp->data - xdp->data_hard_start);
+	memcpy(__skb_put(skb, datasize), xdp->data, datasize);
 	if (metasize)
 		skb_metadata_set(skb, metasize);
 
-	xsk_buff_free(bi->xdp);
-	bi->xdp = NULL;
 	return skb;
 }
 
@@ -317,12 +315,15 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
 		}
 
 		/* XDP_PASS path */
-		skb = ixgbe_construct_skb_zc(rx_ring, bi);
+		skb = ixgbe_construct_skb_zc(rx_ring, bi->xdp);
 		if (!skb) {
 			rx_ring->rx_stats.alloc_rx_buff_failed++;
 			break;
 		}
 
+		xsk_buff_free(bi->xdp);
+		bi->xdp = NULL;
+
 		cleaned_count++;
 		ixgbe_inc_ntc(rx_ring);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 8/9] ixgbe: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
                   ` (6 preceding siblings ...)
  2022-01-31 18:31 ` [PATCH net-next 7/9] ixgbe: pass bi->xdp to ixgbe_construct_skb_zc() directly Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-01-31 18:31 ` [PATCH net-next 9/9] ixgbe: respect metadata " Tony Nguyen
  2022-02-01 13:20 ` [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf,
	Sandeep Penigalapati

From: Alexander Lobakin <alexandr.lobakin@intel.com>

{__,}napi_alloc_skb() allocates and reserves additional NET_SKB_PAD
+ NET_IP_ALIGN for any skb.
OTOH, ixgbe_construct_skb_zc() currently allocates and reserves
additional `xdp->data - xdp->data_hard_start`, which is
XDP_PACKET_HEADROOM for XSK frames.
There's no need for that at all as the frame is post-XDP and will
go only to the networking stack core.
Pass the size of the actual data only to __napi_alloc_skb() and
don't reserve anything. This will give enough headroom for stack
processing.

Fixes: d0bcacd0a130 ("ixgbe: add AF_XDP zero-copy Rx support")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Tested-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 5447bfb31838..80078762ed24 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -214,13 +214,11 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
 	struct sk_buff *skb;
 
 	/* allocate a skb to store the frags */
-	skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
-			       xdp->data_end - xdp->data_hard_start,
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, datasize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		return NULL;
 
-	skb_reserve(skb, xdp->data - xdp->data_hard_start);
 	memcpy(__skb_put(skb, datasize), xdp->data, datasize);
 	if (metasize)
 		skb_metadata_set(skb, metasize);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 9/9] ixgbe: respect metadata on XSK Rx to skb
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
                   ` (7 preceding siblings ...)
  2022-01-31 18:31 ` [PATCH net-next 8/9] ixgbe: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
@ 2022-01-31 18:31 ` Tony Nguyen
  2022-02-01 13:20 ` [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: Tony Nguyen @ 2022-01-31 18:31 UTC (permalink / raw)
  To: davem, kuba
  Cc: Alexander Lobakin, netdev, anthony.l.nguyen, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf,
	Jesper Dangaard Brouer, Sandeep Penigalapati

From: Alexander Lobakin <alexandr.lobakin@intel.com>

For now, if the XDP prog returns XDP_PASS on XSK, the metadata
will be lost as it doesn't get copied to the skb.

Copy it along with the frame headers. Account its size on skb
allocation, and when copying just treat it as a part of the frame
and do a pull after to "move" it to the "reserved" zone.

net_prefetch() xdp->data_meta and align the copy size to speed-up
memcpy() a little and better match ixgbe_construct_skb().

Fixes: d0bcacd0a130 ("ixgbe: add AF_XDP zero-copy Rx support")
Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Tested-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 80078762ed24..ee28929b9c5f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -209,19 +209,25 @@ bool ixgbe_alloc_rx_buffers_zc(struct ixgbe_ring *rx_ring, u16 count)
 static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
 					      const struct xdp_buff *xdp)
 {
+	unsigned int totalsize = xdp->data_end - xdp->data_meta;
 	unsigned int metasize = xdp->data - xdp->data_meta;
-	unsigned int datasize = xdp->data_end - xdp->data;
 	struct sk_buff *skb;
 
+	net_prefetch(xdp->data_meta);
+
 	/* allocate a skb to store the frags */
-	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, datasize,
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi, totalsize,
 			       GFP_ATOMIC | __GFP_NOWARN);
 	if (unlikely(!skb))
 		return NULL;
 
-	memcpy(__skb_put(skb, datasize), xdp->data, datasize);
-	if (metasize)
+	memcpy(__skb_put(skb, totalsize), xdp->data_meta,
+	       ALIGN(totalsize, sizeof(long)));
+
+	if (metasize) {
 		skb_metadata_set(skb, metasize);
+		__skb_pull(skb, metasize);
+	}
 
 	return skb;
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31
  2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
                   ` (8 preceding siblings ...)
  2022-01-31 18:31 ` [PATCH net-next 9/9] ixgbe: respect metadata " Tony Nguyen
@ 2022-02-01 13:20 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-01 13:20 UTC (permalink / raw)
  To: Tony Nguyen
  Cc: davem, kuba, netdev, alexandr.lobakin, ast, daniel, hawk,
	john.fastabend, bjorn, maciej.fijalkowski, michal.swiatkowski,
	kafai, songliubraving, kpsingh, yhs, andrii, bpf

Hello:

This series was applied to netdev/net-next.git (master)
by Tony Nguyen <anthony.l.nguyen@intel.com>:

On Mon, 31 Jan 2022 10:31:43 -0800 you wrote:
> Alexander Lobakin says:
> 
> This is an interpolation of [0] to other Intel Ethernet drivers
> (and is (re)based on its code).
> The main aim is to keep XDP metadata not only in case with
> build_skb(), but also when we do napi_alloc_skb() + memcpy().
> 
> [...]

Here is the summary with links:
  - [net-next,1/9] i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
    https://git.kernel.org/netdev/net-next/c/bc97f9c6f988
  - [net-next,2/9] i40e: respect metadata on XSK Rx to skb
    https://git.kernel.org/netdev/net-next/c/6dba29537c0f
  - [net-next,3/9] ice: respect metadata in legacy-rx/ice_construct_skb()
    https://git.kernel.org/netdev/net-next/c/ee803dca967a
  - [net-next,4/9] ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
    https://git.kernel.org/netdev/net-next/c/dc44572d195e
  - [net-next,5/9] ice: respect metadata on XSK Rx to skb
    https://git.kernel.org/netdev/net-next/c/45a34ca68070
  - [net-next,6/9] igc: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
    https://git.kernel.org/netdev/net-next/c/f9e61d365baf
  - [net-next,7/9] ixgbe: pass bi->xdp to ixgbe_construct_skb_zc() directly
    https://git.kernel.org/netdev/net-next/c/1fbdaa133868
  - [net-next,8/9] ixgbe: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
    https://git.kernel.org/netdev/net-next/c/8f405221a73a
  - [net-next,9/9] ixgbe: respect metadata on XSK Rx to skb
    https://git.kernel.org/netdev/net-next/c/f322a620be69

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-02-01 13:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-31 18:31 [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 1/9] i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 2/9] i40e: respect metadata " Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 3/9] ice: respect metadata in legacy-rx/ice_construct_skb() Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 4/9] ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 5/9] ice: respect metadata " Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 6/9] igc: don't reserve excessive XDP_PACKET_HEADROOM " Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 7/9] ixgbe: pass bi->xdp to ixgbe_construct_skb_zc() directly Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 8/9] ixgbe: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb Tony Nguyen
2022-01-31 18:31 ` [PATCH net-next 9/9] ixgbe: respect metadata " Tony Nguyen
2022-02-01 13:20 ` [PATCH net-next 0/9][pull request] 10GbE Intel Wired LAN Driver Updates 2022-01-31 patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.