* [PATCH net-next 1/2] igc: AF_XDP zero-copy metadata adjust breaks SKBs on XDP_PASS
2021-11-30 17:59 [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2021-11-30 Tony Nguyen
@ 2021-11-30 17:59 ` Tony Nguyen
2021-11-30 17:59 ` [PATCH net-next 2/2] igc: enable XDP metadata in driver Tony Nguyen
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Tony Nguyen @ 2021-11-30 17:59 UTC (permalink / raw)
To: davem, kuba
Cc: Jesper Dangaard Brouer, netdev, anthony.l.nguyen, sasha.neftin,
vitaly.lifshits, maciej.fijalkowski, magnus.karlsson, ast, daniel,
hawk, john.fastabend, bpf, andrii, kpsingh, kafai, yhs,
songliubraving, Nechama Kraus
From: Jesper Dangaard Brouer <brouer@redhat.com>
Driver already implicitly supports XDP metadata access in AF_XDP
zero-copy mode, as xsk_buff_pool's xp_alloc() naturally set xdp_buff
data_meta equal data.
This works fine for XDP and AF_XDP, but if a BPF-prog adjust via
bpf_xdp_adjust_meta() and choose to call XDP_PASS, then igc function
igc_construct_skb_zc() will construct an invalid SKB packet. The
function correctly include the xdp->data_meta area in the memcpy, but
forgot to pull header to take metasize into account.
Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Tested-by: Nechama Kraus <nechamax.kraus@linux.intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/igc/igc_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 8e448288ee26..76b0a7311369 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -2448,8 +2448,10 @@ static struct sk_buff *igc_construct_skb_zc(struct igc_ring *ring,
skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
memcpy(__skb_put(skb, totalsize), xdp->data_meta, totalsize);
- if (metasize)
+ if (metasize) {
skb_metadata_set(skb, metasize);
+ __skb_pull(skb, metasize);
+ }
return skb;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net-next 2/2] igc: enable XDP metadata in driver
2021-11-30 17:59 [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2021-11-30 Tony Nguyen
2021-11-30 17:59 ` [PATCH net-next 1/2] igc: AF_XDP zero-copy metadata adjust breaks SKBs on XDP_PASS Tony Nguyen
@ 2021-11-30 17:59 ` Tony Nguyen
2021-12-01 10:25 ` [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2021-11-30 Jesper Dangaard Brouer
2021-12-01 14:50 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Tony Nguyen @ 2021-11-30 17:59 UTC (permalink / raw)
To: davem, kuba
Cc: Jesper Dangaard Brouer, netdev, anthony.l.nguyen, sasha.neftin,
vitaly.lifshits, maciej.fijalkowski, magnus.karlsson, ast, daniel,
hawk, john.fastabend, bpf, andrii, kpsingh, kafai, yhs,
songliubraving, Nechama Kraus, Alexander Lobakin
From: Jesper Dangaard Brouer <brouer@redhat.com>
Enabling the XDP bpf_prog access to data_meta area is a very small
change. Hint passing 'true' to xdp_prepare_buff().
The SKB layers can also access data_meta area, which required more
driver changes to support. Reviewers, notice the igc driver have two
different functions that can create SKBs, depending on driver config.
Hint for testers, ethtool priv-flags legacy-rx enables
the function igc_construct_skb()
ethtool --set-priv-flags DEV legacy-rx on
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Tested-by: Nechama Kraus <nechamax.kraus@linux.intel.com>
Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/igc/igc_main.c | 33 +++++++++++++++--------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 76b0a7311369..142c57b7a451 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1718,24 +1718,26 @@ static void igc_add_rx_frag(struct igc_ring *rx_ring,
static struct sk_buff *igc_build_skb(struct igc_ring *rx_ring,
struct igc_rx_buffer *rx_buffer,
- union igc_adv_rx_desc *rx_desc,
- unsigned int size)
+ struct xdp_buff *xdp)
{
- void *va = page_address(rx_buffer->page) + rx_buffer->page_offset;
+ unsigned int size = xdp->data_end - xdp->data;
unsigned int truesize = igc_get_rx_frame_truesize(rx_ring, size);
+ unsigned int metasize = xdp->data - xdp->data_meta;
struct sk_buff *skb;
/* prefetch first cache line of first page */
- net_prefetch(va);
+ net_prefetch(xdp->data_meta);
/* build an skb around the page buffer */
- skb = build_skb(va - IGC_SKB_PAD, truesize);
+ skb = build_skb(xdp->data_hard_start, truesize);
if (unlikely(!skb))
return NULL;
/* update pointers within the skb to store the data */
- skb_reserve(skb, IGC_SKB_PAD);
+ skb_reserve(skb, xdp->data - xdp->data_hard_start);
__skb_put(skb, size);
+ if (metasize)
+ skb_metadata_set(skb, metasize);
igc_rx_buffer_flip(rx_buffer, truesize);
return skb;
@@ -1746,6 +1748,7 @@ static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring,
struct xdp_buff *xdp,
ktime_t timestamp)
{
+ unsigned int metasize = xdp->data - xdp->data_meta;
unsigned int size = xdp->data_end - xdp->data;
unsigned int truesize = igc_get_rx_frame_truesize(rx_ring, size);
void *va = xdp->data;
@@ -1753,10 +1756,11 @@ static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring,
struct sk_buff *skb;
/* prefetch first cache line of first page */
- net_prefetch(va);
+ net_prefetch(xdp->data_meta);
/* allocate a skb to store the frags */
- skb = napi_alloc_skb(&rx_ring->q_vector->napi, IGC_RX_HDR_LEN);
+ skb = napi_alloc_skb(&rx_ring->q_vector->napi,
+ IGC_RX_HDR_LEN + metasize);
if (unlikely(!skb))
return NULL;
@@ -1769,7 +1773,13 @@ static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring,
headlen = eth_get_headlen(skb->dev, va, IGC_RX_HDR_LEN);
/* align pull length to size of long to optimize memcpy performance */
- memcpy(__skb_put(skb, headlen), va, 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);
+ }
/* update all of the pointers */
size -= headlen;
@@ -2354,7 +2364,8 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
if (!skb) {
xdp_init_buff(&xdp, truesize, &rx_ring->xdp_rxq);
xdp_prepare_buff(&xdp, pktbuf - igc_rx_offset(rx_ring),
- igc_rx_offset(rx_ring) + pkt_offset, size, false);
+ igc_rx_offset(rx_ring) + pkt_offset,
+ size, true);
skb = igc_xdp_run_prog(adapter, &xdp);
}
@@ -2378,7 +2389,7 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
} else if (skb)
igc_add_rx_frag(rx_ring, rx_buffer, skb, size);
else if (ring_uses_build_skb(rx_ring))
- skb = igc_build_skb(rx_ring, rx_buffer, rx_desc, size);
+ skb = igc_build_skb(rx_ring, rx_buffer, &xdp);
else
skb = igc_construct_skb(rx_ring, rx_buffer, &xdp,
timestamp);
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2021-11-30
2021-11-30 17:59 [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2021-11-30 Tony Nguyen
` (2 preceding siblings ...)
2021-12-01 10:25 ` [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2021-11-30 Jesper Dangaard Brouer
@ 2021-12-01 14:50 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-12-01 14:50 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, netdev, sasha.neftin, vitaly.lifshits,
maciej.fijalkowski, magnus.karlsson, ast, daniel, hawk,
john.fastabend, bpf, andrii, kpsingh, kafai, yhs, songliubraving
Hello:
This series was applied to netdev/net-next.git (master)
by Tony Nguyen <anthony.l.nguyen@intel.com>:
On Tue, 30 Nov 2021 09:59:16 -0800 you wrote:
> Jesper Dangaard Brouer says:
>
> Changes to fix and enable XDP metadata to a specific Intel driver igc.
> Tested with hardware i225 that uses driver igc, while testing AF_XDP
> access to metadata area.
>
> The following are changes since commit 196073f9c44be0b4758ead11e51bc2875f98df29:
> net: ixp4xx_hss: drop kfree for memory allocated with devm_kzalloc
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 1GbE
>
> [...]
Here is the summary with links:
- [net-next,1/2] igc: AF_XDP zero-copy metadata adjust breaks SKBs on XDP_PASS
https://git.kernel.org/netdev/net-next/c/4fa8fcd34401
- [net-next,2/2] igc: enable XDP metadata in driver
https://git.kernel.org/netdev/net-next/c/f51b5e2b5943
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] 5+ messages in thread