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].
From v3 ([1]):
- fix driver name and ixgbe_construct_skb() function name in the
commit message of #9 (Jesper);
- no functional changes.
From v2 (unreleased upstream):
- tweaked 007 to pass bi->xdp directly and simplify code (Maciej);
- picked Michal's Reviewed-by.
From v1 (unreleased upstream):
- drop "fixes" of legacy-rx for i40e, igb and ixgbe since they have
another flaw regarding headroom (see above);
- drop igc cosmetic fixes since they landed upstream incorporated
into Jesper's commits;
- picked one Acked-by from Maciej.
[0] https://lore.kernel.org/netdev/163700856423.565980.10162564921347693758.stgit@firesoul
[1] https://lore.kernel.org/netdev/[email protected]
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(-)
--
Testing hints:
Setup an XDP and AF_XDP program which will prepend metadata at the
front of the frames and return XDP_PASS, then check that metadata
is present after frames reach kernel network stack.
--
2.33.1
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_costruct_skb().
Fixes: 0a714186d3c0 ("i40e: add AF_XDP zero-copy Rx support")
Suggested-by: Jesper Dangaard Brouer <[email protected]>
Suggested-by: Maciej Fijalkowski <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 9564906b7da8..0e8cf275e084 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -240,19 +240,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.33.1
{__,}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 <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 f08d19b8c554..9564906b7da8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -245,13 +245,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.33.1
{__,}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_costruct_skb().
Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 142c57b7a451..a2e8d43be3a1 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.33.1
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_costruct_skb().
Fixes: 2d4238f55697 ("ice: Add support for AF_XDP")
Suggested-by: Jesper Dangaard Brouer <[email protected]>
Suggested-by: Maciej Fijalkowski <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 f0bd8e1953bf..57e49e652439 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -428,18 +428,24 @@ static struct sk_buff *
ice_construct_skb_zc(struct ice_rx_ring *rx_ring, struct xdp_buff **xdp_arr)
{
struct xdp_buff *xdp = *xdp_arr;
+ 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);
*xdp_arr = NULL;
--
2.33.1
{__,}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 <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 f8ea6b0633eb..f0bd8e1953bf 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -430,15 +430,13 @@ ice_construct_skb_zc(struct ice_rx_ring *rx_ring, struct xdp_buff **xdp_arr)
struct xdp_buff *xdp = *xdp_arr;
unsigned int metasize = xdp->data - xdp->data_meta;
unsigned int datasize = xdp->data_end - xdp->data;
- unsigned int datasize_hard = xdp->data_end - xdp->data_hard_start;
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.33.1
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_costruct_skb().
Fixes: d0bcacd0a130 ("ixgbe: add AF_XDP zero-copy Rx support")
Suggested-by: Jesper Dangaard Brouer <[email protected]>
Suggested-by: Maciej Fijalkowski <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 db20dc4c2488..ec1e2da72676 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.33.1
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 <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
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 db2bc58dfcfd..1d74a7980d81 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.33.1
{__,}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 <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 1d74a7980d81..db20dc4c2488 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.33.1
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 <[email protected]>
Suggested-by: Maciej Fijalkowski <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Michal Swiatkowski <[email protected]>
---
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 bc3ba19dc88f..d724b6376c43 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -968,15 +968,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;
@@ -988,8 +990,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.33.1
On 08/12/2021 15.06, Alexander Lobakin wrote:
> {__,}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.
I disagree with this assumption, that headroom is not needed by netstack.
Why "no need for that at all" for netstack?
Having headroom is important for netstack in general. When packet will
grow we avoid realloc of SKB. Use-case could also be cpumap or veth
redirect, or XDP-generic, that expect this headroom.
> 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 <[email protected]>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> ---
> 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 f08d19b8c554..9564906b7da8 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> @@ -245,13 +245,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);
>
On 08/12/2021 15.06, Alexander Lobakin wrote:
> 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.
I have an urge to add a newline here, when reading this, as IMHO it is a
paragraph with the problem statement.
> 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.
Also newline here, as next paragraph are some extra details, you felt a
need to explain to the reader.
> net_prefetch() xdp->data_meta and align the copy size to speed-up
> memcpy() a little and better match i40e_costruct_skb().
^^^^^^xx^^^^^^^^^
You have a general misspelling of this function name in all of your
commit messages.
--Jesper
From: Jesper Dangaard Brouer <[email protected]>
Date: Thu, 9 Dec 2021 09:19:46 +0100
> On 08/12/2021 15.06, Alexander Lobakin wrote:
> > {__,}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.
>
> I disagree with this assumption, that headroom is not needed by netstack.
> Why "no need for that at all" for netstack?
napi_alloc_skb() in our particular case will reserve 64 bytes, it is
sufficient for {TCP,UDP,SCTP,...}/IPv{4,6} etc.
>
> Having headroom is important for netstack in general. When packet will
> grow we avoid realloc of SKB. Use-case could also be cpumap or veth
> redirect, or XDP-generic, that expect this headroom.
Well, those are not common cases at all.
Allocating 256 bytes more for some hypothetical usecases (and having
320 in total) is more expensive than expanding headroom in-place.
I don't know any other drivers or ifaces which reserve
XDP_PACKET_HEADROOM just for the case of using both driver-side
and generic XDP at the same time. To be more precise, I can't
remember any driver which would check whether generic XDP is enabled
for its netdev(s).
As a second option, I was trying to get exactly XDP_PACKET_HEADROOM
of headroom, but this involves either __alloc_skb() which is slower
than napi_alloc_skb(), or
skb = napi_alloc_skb(napi, xdp->data_end -
xdp->data_hard_start -
NET_SKB_PAD);
skb_reserve(skb, xdp->data_meta - xdp->data_hard_start -
NET_SKB_PAD);
Doesn't look good for me.
We could probably introduce a version of napi_alloc_skb() which
wouldn't reserve any headroom for you to have more control over it,
but that's more global material than these local fixes I'd say.
>
>
> > 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 <[email protected]>
> > Reviewed-by: Michal Swiatkowski <[email protected]>
> > ---
> > 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 f08d19b8c554..9564906b7da8 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> > @@ -245,13 +245,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);
Thanks,
Al
From: Jesper Dangaard Brouer <[email protected]>
Date: Thu, 9 Dec 2021 09:27:37 +0100
> On 08/12/2021 15.06, Alexander Lobakin wrote:
> > 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.
>
> I have an urge to add a newline here, when reading this, as IMHO it is a
> paragraph with the problem statement.
>
> > 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.
>
> Also newline here, as next paragraph are some extra details, you felt a
> need to explain to the reader.
>
> > net_prefetch() xdp->data_meta and align the copy size to speed-up
> > memcpy() a little and better match i40e_costruct_skb().
> ^^^^^^xx^^^^^^^^^
>
> You have a general misspelling of this function name in all of your
> commit messages.
Oh gosh, I thought I don't have attention deficit. Thanks, maybe
Tony will fix it for me or I could send a follow-up (or resend if
needed, I saw those were already applied to dev-queue).
>
> --Jesper
Al
On Thu, 2021-12-09 at 18:38 +0100, Alexander Lobakin wrote:
> From: Jesper Dangaard Brouer <[email protected]>
> Date: Thu, 9 Dec 2021 09:27:37 +0100
>
> > On 08/12/2021 15.06, Alexander Lobakin wrote:
> > > 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.
> >
> > I have an urge to add a newline here, when reading this, as IMHO it
> > is a
> > paragraph with the problem statement.
> >
> > > 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.
> >
> > Also newline here, as next paragraph are some extra details, you
> > felt a
> > need to explain to the reader.
> >
> > > net_prefetch() xdp->data_meta and align the copy size to speed-up
> > > memcpy() a little and better match i40e_costruct_skb().
> > ^^^^^^xx^^^^^^^^^
> >
> > commit messages.
>
> Oh gosh, I thought I don't have attention deficit. Thanks, maybe
> Tony will fix it for me or I could send a follow-up (or resend if
> needed, I saw those were already applied to dev-queue).
If there's no need for follow-ups beyond this change, I'll fix it up.
Thanks,
Tony
> > --Jesper
>
> Al
From: Tony Nguyen <[email protected]>
Date: Thu, 9 Dec 2021 18:50:07 +0000
> On Thu, 2021-12-09 at 18:38 +0100, Alexander Lobakin wrote:
> > From: Jesper Dangaard Brouer <[email protected]>
> > Date: Thu, 9 Dec 2021 09:27:37 +0100
> >
> > > On 08/12/2021 15.06, Alexander Lobakin wrote:
> > > > 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.
> > >
> > > I have an urge to add a newline here, when reading this, as IMHO it
> > > is a
> > > paragraph with the problem statement.
> > >
> > > > 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.
> > >
> > > Also newline here, as next paragraph are some extra details, you
> > > felt a
> > > need to explain to the reader.
> > >
> > > > net_prefetch() xdp->data_meta and align the copy size to speed-up
> > > > memcpy() a little and better match i40e_costruct_skb().
> > > ^^^^^^xx^^^^^^^^^
> > >
> > > commit messages.
> >
> > Oh gosh, I thought I don't have attention deficit. Thanks, maybe
> > Tony will fix it for me or I could send a follow-up (or resend if
> > needed, I saw those were already applied to dev-queue).
>
> If there's no need for follow-ups beyond this change, I'll fix it up.
The rest is fine, thank you!
> Thanks,
> Tony
>
> > > --Jesper
> >
> > Al
Al
On 09/12/2021 18.33, Alexander Lobakin wrote:
> From: Jesper Dangaard Brouer <[email protected]>
> Date: Thu, 9 Dec 2021 09:19:46 +0100
>
>> On 08/12/2021 15.06, Alexander Lobakin wrote:
>>> {__,}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.
>>
>> I disagree with this assumption, that headroom is not needed by netstack.
>> Why "no need for that at all" for netstack?
>
> napi_alloc_skb() in our particular case will reserve 64 bytes, it is
> sufficient for {TCP,UDP,SCTP,...}/IPv{4,6} etc.
My bad, I misunderstood you. I now see (looking at code) that (as you
say) 64 bytes of headroom *is* reserved (in bottom of __napi_alloc_skb).
Thus, the SKB *do* have headroom, so this patch should be fine.
Acked-by: Jesper Dangaard Brouer <[email protected]>
Do watch out that 64 bytes is not always enough. Notice the define
LL_MAX_HEADER and MAX_HEADER in include/linux/netdevice.h (that tries to
determine worst-case header length) which is above 64 bytes. It is also
affected by HyperV and WiFi configs.
On 12/8/2021 16:06, Alexander Lobakin wrote:
> {__,}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_costruct_skb().
>
> Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> Signed-off-by: Alexander Lobakin <[email protected]>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
Tested-by: Nechama Kraus <[email protected]>
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Alexander Lobakin
> Sent: Wednesday, December 8, 2021 7:37 PM
> To: [email protected]
> Cc: Song Liu <[email protected]>; Jesper Dangaard Brouer
> <[email protected]>; Daniel Borkmann <[email protected]>; Yonghong
> Song <[email protected]>; Martin KaFai Lau <[email protected]>; John Fastabend
> <[email protected]>; Alexei Starovoitov <[email protected]>; Andrii
> Nakryiko <[email protected]>; Bj?rn T?pel <[email protected]>;
> [email protected]; Jakub Kicinski <[email protected]>; KP Singh
> <[email protected]>; [email protected]; David S. Miller
> <[email protected]>; [email protected]
> Subject: [Intel-wired-lan] [PATCH v4 net-next 0/9] net: intel: napi_alloc_skb()
> vs metadata
>
> 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].
>
> From v3 ([1]):
> - fix driver name and ixgbe_construct_skb() function name in the
> commit message of #9 (Jesper);
> - no functional changes.
>
> From v2 (unreleased upstream):
> - tweaked 007 to pass bi->xdp directly and simplify code (Maciej);
> - picked Michal's Reviewed-by.
>
> From v1 (unreleased upstream):
> - drop "fixes" of legacy-rx for i40e, igb and ixgbe since they have
> another flaw regarding headroom (see above);
> - drop igc cosmetic fixes since they landed upstream incorporated
> into Jesper's commits;
> - picked one Acked-by from Maciej.
>
> [0]
> https://lore.kernel.org/netdev/163700856423.565980.101625649213476937
> 58.stgit@firesoul
> [1] https://lore.kernel.org/netdev/20211207205536.563550-1-
> [email protected]
>
> 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(-)
>
Tested-by: Kiran Bhandare <[email protected]> A Contingent Worker at Intel
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Alexander Lobakin
> Sent: Wednesday, December 8, 2021 7:37 PM
> To: [email protected]
> Cc: Song Liu <[email protected]>; Jesper Dangaard Brouer
> <[email protected]>; Daniel Borkmann <[email protected]>; Yonghong
> Song <[email protected]>; Martin KaFai Lau <[email protected]>; John Fastabend
> <[email protected]>; Alexei Starovoitov <[email protected]>; Andrii
> Nakryiko <[email protected]>; Bj?rn T?pel <[email protected]>;
> [email protected]; Jakub Kicinski <[email protected]>; KP Singh
> <[email protected]>; [email protected]; David S. Miller
> <[email protected]>; [email protected]
> Subject: [Intel-wired-lan] [PATCH v4 net-next 4/9] ice: don't reserve excessive
> XDP_PACKET_HEADROOM on XSK Rx to skb
>
> {__,}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 <[email protected]>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_xsk.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
Tested-by: Kiran Bhandare <[email protected]> A Contingent Worker at Intel
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Alexander Lobakin
> Sent: Wednesday, December 8, 2021 7:37 PM
> To: [email protected]
> Cc: Song Liu <[email protected]>; Alexei Starovoitov <[email protected]>;
> Andrii Nakryiko <[email protected]>; Daniel Borkmann
> <[email protected]>; John Fastabend <[email protected]>;
> Jesper Dangaard Brouer <[email protected]>; Yonghong Song
> <[email protected]>; Jesper Dangaard Brouer <[email protected]>; KP Singh
> <[email protected]>; Jakub Kicinski <[email protected]>;
> [email protected]; [email protected]; David S. Miller
> <[email protected]>; Bj?rn T?pel <[email protected]>;
> [email protected]; Martin KaFai Lau <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v4 net-next 3/9] ice: respect metadata in
> legacy-rx/ice_construct_skb()
>
> 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 <[email protected]>
> Suggested-by: Maciej Fijalkowski <[email protected]>
> Signed-off-by: Alexander Lobakin <[email protected]>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_txrx.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
Tested-by: Kiran Bhandare <[email protected]> A Contingent Worker at Intel
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Lobakin, Alexandr
> Sent: Wednesday, December 8, 2021 7:37 PM
> To: [email protected]
> Cc: Song Liu <[email protected]>; Alexei Starovoitov <[email protected]>;
> Andrii Nakryiko <[email protected]>; Daniel Borkmann
> <[email protected]>; John Fastabend <[email protected]>;
> Brouer, Jesper <[email protected]>; Yonghong Song <[email protected]>;
> Jesper Dangaard Brouer <[email protected]>; KP Singh
> <[email protected]>; Jakub Kicinski <[email protected]>;
> [email protected]; [email protected]; David S. Miller
> <[email protected]>; Bj?rn T?pel <[email protected]>;
> [email protected]; Martin KaFai Lau <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v4 net-next 2/9] i40e: respect metadata on
> XSK Rx to skb
>
> 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_costruct_skb().
>
> Fixes: 0a714186d3c0 ("i40e: add AF_XDP zero-copy Rx support")
> Suggested-by: Jesper Dangaard Brouer <[email protected]>
> Suggested-by: Maciej Fijalkowski <[email protected]>
> Signed-off-by: Alexander Lobakin <[email protected]>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
Tested-by: Kiran Bhandare <[email protected]> A Contingent Worker at Intel
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Alexander Lobakin
> Sent: Wednesday, December 8, 2021 7:37 PM
> To: [email protected]
> Cc: Song Liu <[email protected]>; Alexei Starovoitov <[email protected]>;
> Andrii Nakryiko <[email protected]>; Daniel Borkmann
> <[email protected]>; John Fastabend <[email protected]>;
> Jesper Dangaard Brouer <[email protected]>; Yonghong Song
> <[email protected]>; Jesper Dangaard Brouer <[email protected]>; KP Singh
> <[email protected]>; Jakub Kicinski <[email protected]>;
> [email protected]; [email protected]; David S. Miller
> <[email protected]>; Bj?rn T?pel <[email protected]>;
> [email protected]; Martin KaFai Lau <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v4 net-next 5/9] ice: respect metadata on
> XSK Rx to skb
>
> 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_costruct_skb().
>
> Fixes: 2d4238f55697 ("ice: Add support for AF_XDP")
> Suggested-by: Jesper Dangaard Brouer <[email protected]>
> Suggested-by: Maciej Fijalkowski <[email protected]>
> Signed-off-by: Alexander Lobakin <[email protected]>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_xsk.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
Tested-by: Kiran Bhandare <[email protected]> A Contingent Worker at Intel
>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of
>Alexander Lobakin
>Sent: Wednesday, December 8, 2021 7:37 PM
>To: [email protected]
>Cc: Song Liu <[email protected]>; Jesper Dangaard Brouer
><[email protected]>; Daniel Borkmann <[email protected]>; Yonghong
>Song <[email protected]>; Martin KaFai Lau <[email protected]>; John Fastabend
><[email protected]>; Alexei Starovoitov <[email protected]>; Andrii
>Nakryiko <[email protected]>; Bj?rn T?pel <[email protected]>;
>[email protected]; Jakub Kicinski <[email protected]>; KP Singh
><[email protected]>; [email protected]; David S. Miller
><[email protected]>; [email protected]
>Subject: [Intel-wired-lan] [PATCH v4 net-next 7/9] ixgbe: pass bi->xdp to
>ixgbe_construct_skb_zc() directly
>
>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 <[email protected]>
>Signed-off-by: Alexander Lobakin <[email protected]>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
Tested-by: Sandeep Penigalapati <[email protected]>
>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of
>Alexander Lobakin
>Sent: Wednesday, December 8, 2021 7:37 PM
>To: [email protected]
>Cc: Song Liu <[email protected]>; Jesper Dangaard Brouer
><[email protected]>; Daniel Borkmann <[email protected]>; Yonghong
>Song <[email protected]>; Martin KaFai Lau <[email protected]>; John Fastabend
><[email protected]>; Alexei Starovoitov <[email protected]>; Andrii
>Nakryiko <[email protected]>; Bj?rn T?pel <[email protected]>;
>[email protected]; Jakub Kicinski <[email protected]>; KP Singh
><[email protected]>; [email protected]; David S. Miller
><[email protected]>; [email protected]
>Subject: [Intel-wired-lan] [PATCH v4 net-next 8/9] ixgbe: don't reserve
>excessive XDP_PACKET_HEADROOM on XSK Rx to skb
>
>{__,}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 <[email protected]>
>Reviewed-by: Michal Swiatkowski <[email protected]>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
Tested-by: Sandeep Penigalapati <[email protected]>
>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of
>Alexander Lobakin
>Sent: Wednesday, December 8, 2021 7:37 PM
>To: [email protected]
>Cc: Song Liu <[email protected]>; Alexei Starovoitov <[email protected]>;
>Andrii Nakryiko <[email protected]>; Daniel Borkmann
><[email protected]>; John Fastabend <[email protected]>;
>Jesper Dangaard Brouer <[email protected]>; Yonghong Song
><[email protected]>; Jesper Dangaard Brouer <[email protected]>; KP Singh
><[email protected]>; Jakub Kicinski <[email protected]>;
>[email protected]; [email protected]; David S. Miller
><[email protected]>; Bj?rn T?pel <[email protected]>;
>[email protected]; Martin KaFai Lau <[email protected]>
>Subject: [Intel-wired-lan] [PATCH v4 net-next 9/9] ixgbe: respect metadata on
>XSK Rx to skb
>
>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_costruct_skb().
>
>Fixes: d0bcacd0a130 ("ixgbe: add AF_XDP zero-copy Rx support")
>Suggested-by: Jesper Dangaard Brouer <[email protected]>
>Suggested-by: Maciej Fijalkowski <[email protected]>
>Signed-off-by: Alexander Lobakin <[email protected]>
>Reviewed-by: Michal Swiatkowski <[email protected]>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
Tested-by: Sandeep Penigalapati <[email protected]>
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Alexander Lobakin
> Sent: Wednesday, December 8, 2021 7:37 PM
> To: [email protected]
> Cc: Song Liu <[email protected]>; Jesper Dangaard Brouer
> <[email protected]>; Daniel Borkmann <[email protected]>; Yonghong
> Song <[email protected]>; Martin KaFai Lau <[email protected]>; John Fastabend
> <[email protected]>; Alexei Starovoitov <[email protected]>; Andrii
> Nakryiko <[email protected]>; Bj?rn T?pel <[email protected]>;
> [email protected]; Jakub Kicinski <[email protected]>; KP Singh
> <[email protected]>; [email protected]; David S. Miller
> <[email protected]>; [email protected]
> Subject: [Intel-wired-lan] [PATCH v4 net-next 1/9] i40e: don't reserve
> excessive XDP_PACKET_HEADROOM on XSK Rx to skb
>
> {__,}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 <[email protected]>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
Tested-by: Kiran Bhandare <[email protected]> A Contingent Worker at Intel