ixgbe: convert Rx to libeth and page_pool#6
Open
Magda-Pytel wants to merge 15 commits into
Open
Conversation
Implement xsk_buff_pool configuration and supporting functionality, such as a single queue pair reconfiguration. Also, properly initialize Rx buffers. Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
Add code that handles Tx ZC queues inside of napi_poll(), utilize libeth. As NIC's multiple buffer conventions do not play nicely with AF_XDP's, leave handling of segments for later. Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
Add code that handles AF_XDP ZC Rx queues inside of napi_poll(), utilize libeth helpers. Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
To finalize basic AF_XDP implementation, set features and add .ndo_xsk_wakeup() handler. Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
Transmitting multi-buffer AF_XDP packets is not very straightforward given HW limitations in ixgbevf, namely that the first data descriptor must contain the length of the whole packet. Use private data of an sqe to store the length of an unfinished packet so far and the first descriptor index. Once EoP zero-copy descriptor is processed, write the accumulated length into the saved first descriptor. Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
Similarly, like for virtual function in ixgbevf, remove ixgbe_construct_skb(), the legacy-rx private flag, and all ring_uses_build_skb() conditionals. build_skb is now the only RX code path. This is a preparation for the conversion to libeth and page_pool based Rx buffer management. Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
Stop sharing pages between multiple Rx buffers. Convert ixgbe Rx path from page-sharing/recycling to a simple one-page-per-packet model. Remove logic that refers to page-sharing buffer management. This is a preparatory step before converting the Rx path to page_pool and libeth. Fixed XDP_DROP/ABORTED: reaches ixgbe_put_rx_buffer() but page is never freed. Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
Convert ixgbe Rx path from manual page-per-packet DMA management to libeth/page_pool infrastructure, and integrates libeth XDP helpers. Each packet gets its own buffer instead of sharing pages between packets. Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
* Added branch prediction hints to the ixgbe Rx path and clean up the packet accounting. * Moved total_rx_bytes and total_rx_packets accounting together. Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
Implement XDP multi-buffer support for Rx fragmented packets
using libeth_xdp.
* Added MODULE_IMPORT_NS("LIBETH_XDP")
and #include <net/libeth/types.h>
* Replaced struct sk_buff *skb in ixgbe_ring with
struct libeth_xdp_buff_stash
and ixgbe_clean_rx_ring skb cleanup with libeth_xdp_return_stash
* Removed ixgbe_add_rx_frag()
* Reorganized ixgbe_clean_rx_irq() and ixgbe_is_non_eop()
* Updated ixgbe_xdp_setup() to allow multi-buffer XDP programs
* Added NETDEV_XDP_ACT_RX_SG in xdp_features
Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
Use libeth to support XDP_TX action for fragmented packets, replacing the legacy per-frame DMA mapping path with libeth page_pool-based bulk submission. Replaced ixgbe_xmit_xdp_ring() with the libeth XDP TX infrastructure. The XDP Tx ring now uses libeth_sqe instead of ixgbe_tx_buffer for its buffer tracking. Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
* Replaced the driver-specific static_branch locking mechanism with libeth_xdpsq_lock infrastructure for XDP queue sharing. * Updated ixgbe_xdp_xmit() to use libeth_xdp_xmit_do_bulk(). * Adjusted ixgbe_xdp_xmit_desc() to handle both XDP_TX buffers and XDP_XMIT frames. * Updated the XSK path to use libeth_xdpsq_lock/libeth_xdpsq_unlock. Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
Introduce pseudo header split support in the ixgbe driver. * Added IXGBE_RX_SRRCTL_BUF_SIZE macro and IXGBE_FLAG2_HSPLIT flag * Added hdr_truesize, hdr_fqes, hdr_pp, hdr_buf_len to ixgbe_ring * Added ixgbe_rx_hsplit_wa() to copy header from data to header buffer * Added ixgbe_rx_create_pp()/ixgbe_rx_destroy_pp() helpers * Updated ixgbe_alloc_rx_buffers to allocate header buffers * Updated ixgbe_clean_rx_irq to process header split * Refactored ixgbe_setup_rx_resources to use ixgbe_rx_create_pp * Refactored ixgbe_free_rx_resources to use ixgbe_rx_destroy_pp * Set IXGBE_FLAG2_HSPLIT for 82599 in ixgbe_sw_init * Updated ixgbe_xdp_setup to require mbuf frags with hsplit Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
* Moved page pool creation from ixgbe_setup_rx_resources() to ixgbe_configure_rx_ring() * Moved page pool destruction to ixgbe_clean_all_rx_rings() and ixgbe_free_rx_resources() * Added ixgbe_dma_dev_from_ring() helper to obtain DMA device without relying on page pool pointer. * Handled XSK path in ixgbe_configure_rx_ring() Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
Previously, MTU changes were restricted to a fixed max frame size when XDP was active. With libeth page pool and multi-buffer XDP support, the restriction can be loose. * Added ixgbe_xdp_mtu_ok() to check if the given MTU is compatible with the attached XDP program. * Updated ixgbe_change_mtu() to use ixgbe_xdp_mtu_ok() instead of the hard-coded IXGBE_RXBUFFER_3K limit. * Updated ixgbe_xdp_setup() to use ixgbe_xdp_mtu_ok() instead of inline requires_mbuf logic. * Added error handling in ixgbe_configure_rx_ring() Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the ixgbe driver’s Rx path away from legacy page-sharing/recycling to the libeth + page_pool infrastructure, and updates the XDP integration accordingly (including multi-buffer/XDP helpers and pseudo header split support).
Changes:
- Switch Rx buffer management to
libethfill/completion queues backed bypage_pool(and integratelibethXDP helpers). - Rework XDP Tx queue handling/locking and add new ring state (
__IXGBE_TX_XDP_RING_PRIMED), plus MTU/XDP frags capability gating. - Remove the legacy-rx private flag and update various supporting paths (self-tests, ring setup, register programming).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| drivers/net/ethernet/intel/Kconfig | Selects LIBETH_XDP for ixgbe builds. |
| drivers/net/ethernet/intel/ixgbe/ixgbe.h | Adopts libeth types, updates ring layout/state for page_pool + libeth queues. |
| drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | Adjusts AF_XDP ZC path to new buffer structs and libeth XDP locking. |
| drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | Renames SRRCTL buffer-size conversion macro to ..._STEP. |
| drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | Core Rx conversion to libeth/page_pool + XDP helpers, plus ring/resource lifecycle updates. |
| drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | Updates loopback/self-test Rx buffer access to libeth_fqe/page_pool buffers. |
Comments suppressed due to low confidence (1)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2316
- In the main Rx path, the skb protocol is never set (the
eth_type_trans()call was removed fromixgbe_process_skb_fields(), andixgbe_clean_rx_irq()doesn’t set it before passing the skb up). This can break upper stack parsing. Setskb->protocolviaeth_type_trans()before callingixgbe_process_skb_fields().
/* populate checksum, timestamp, VLAN, and protocol */
ixgbe_process_skb_fields(rx_ring, rx_desc, skb);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1744
to
+1748
| rx_desc->read.pkt_addr = cpu_to_le64(addr); | ||
|
|
||
| /* | ||
| * Refresh the desc even if buffer_addrs didn't change | ||
| * because each write-back erases this info. | ||
| */ | ||
| rx_desc->read.pkt_addr = cpu_to_le64(bi->dma + bi->page_offset); | ||
| if (hdr_fq.pp) { | ||
| addr = libeth_rx_alloc(&hdr_fq, ntu); | ||
| if (addr == DMA_MAPPING_ERROR) { |
Comment on lines
+2075
to
+2079
| if (unlikely(!test_bit(__IXGBE_TX_XDP_RING_PRIMED, | ||
| xdp_ring->state))) { | ||
| struct ixgbe_adv_tx_context_desc *context_desc; | ||
|
|
||
| return rx_buffer; | ||
| } | ||
| set_bit(__IXGBE_TX_XDP_RING_PRIMED, xdp_ring->state); |
Comment on lines
+3687
to
+3688
| clear_bit(__IXGBE_HANG_CHECK_ARMED, ring->state); | ||
| clear_bit(__IXGBE_TX_XDP_RING_PRIMED, ring->state); |
Comment on lines
+4227
to
4233
| __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev, | ||
| ring->queue_index, | ||
| ixgbe_rx_napi_id(ring), 0); | ||
| WARN_ON(xdp_rxq_info_reg_mem_model(&ring->xdp_rxq, | ||
| MEM_TYPE_XSK_BUFF_POOL, | ||
| NULL)); | ||
| xsk_pool_set_rxq_info(ring->xsk_pool, &ring->xdp_rxq); |
walking-machine
force-pushed
the
ixgbevf-xdp-public
branch
from
July 6, 2026 06:32
3176c41 to
d9c60d3
Compare
walking-machine
requested changes
Jul 21, 2026
walking-machine
left a comment
walking-machine
left a comment
Owner
There was a problem hiding this comment.
Patch 1:
Aside from a silly '&' mistake, code looks OK, though I would make a few changes.
| */ | ||
| if (ring_uses_build_skb(ring) && | ||
| !test_bit(__IXGBE_RX_3K_BUFFER, &ring->state)) | ||
| if (!test_bit(__IXGBE_RX_3K_BUFFER, ring->state)) |
Owner
There was a problem hiding this comment.
This patch does not compile. The rule is that kernel must compile correctly in any intermediate step.
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c: In function ‘ixgbe_configure_rx_ring’:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:4484:57: error: passing argument 2 of ‘const_test_bit’ makes pointer from integer without a cast [-Wint-conversion]
4484 | if (!test_bit(__IXGBE_RX_3K_BUFFER, ring->state))
| ~~~~^~~~~~~
| |
| long unsigned int
./include/linux/bitops.h:47:24: note: in definition of macro ‘bitop’
47 | const##op(nr, addr) : op(nr, addr))
| ^~~~
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:4484:22: note: in expansion of macro ‘test_bit’
4484 | if (!test_bit(__IXGBE_RX_3K_BUFFER, ring->state))
| ^~~~~~~~
In file included from ./include/linux/bitops.h:28:
./include/asm-generic/bitops/generic-non-atomic.h:166:64: note: expected ‘const volatile long unsigned int *’ but argument is of type ‘long unsigned int’
166 | const_test_bit(unsigned long nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:4484:57: error: passing argument 2 of ‘_test_bit’ makes pointer from integer without a cast [-Wint-conversion]
4484 | if (!test_bit(__IXGBE_RX_3K_BUFFER, ring->state))
| ~~~~^~~~~~~
| |
| long unsigned int
./include/linux/bitops.h:47:39: note: in definition of macro ‘bitop’
47 | const##op(nr, addr) : op(nr, addr))
| ^~~~
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:4484:22: note: in expansion of macro ‘test_bit’
4484 | if (!test_bit(__IXGBE_RX_3K_BUFFER, ring->state))
| ^~~~~~~~
In file included from ./arch/x86/include/asm/bitops.h:424,
from ./include/linux/bitops.h:67:
./include/asm-generic/bitops/instrumented-non-atomic.h:139:59: note: expected ‘const volatile long unsigned int *’ but argument is of type ‘long unsigned int’
139 | _test_bit(unsigned long nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c: In function ‘ixgbe_set_rx_buffer_len’:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:4659:56: error: passing argument 2 of ‘clear_bit’ makes pointer from integer without a cast [-Wint-conversion]
4659 | clear_bit(__IXGBE_RX_3K_BUFFER, rx_ring->state);
| ~~~~~~~^~~~~~~
| |
| long unsigned int
In file included from ./arch/x86/include/asm/bitops.h:423:
./include/asm-generic/bitops/instrumented-atomic.h:39:72: note: expected ‘volatile long unsigned int *’ but argument is of type ‘long unsigned int’
39 | static __always_inline void clear_bit(long nr, volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
Owner
There was a problem hiding this comment.
Fixing diff:
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 7a4b22d813ba..55e4f0522e75 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4481,7 +4481,7 @@ void ixgbe_configure_rx_ring(struct ixgbe_adapter *adapter,
* This can happen in SRIOV mode when the MTU of the VF is
* higher than the MTU of the PF.
*/
- if (!test_bit(__IXGBE_RX_3K_BUFFER, ring->state))
+ if (!test_bit(__IXGBE_RX_3K_BUFFER, &ring->state))
rxdctl |= IXGBE_MAX_2K_FRAME_BUILD_SKB |
IXGBE_RXDCTL_RLPML_EN;
#endif
@@ -4656,7 +4656,7 @@ static void ixgbe_set_rx_buffer_len(struct ixgbe_adapter *adapter)
rx_ring = adapter->rx_ring[i];
clear_ring_rsc_enabled(rx_ring);
- clear_bit(__IXGBE_RX_3K_BUFFER, rx_ring->state);
+ clear_bit(__IXGBE_RX_3K_BUFFER, &rx_ring->state);
if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
set_ring_rsc_enabled(rx_ring);
walking-machine
requested changes
Jul 21, 2026
walking-machine
left a comment
walking-machine
left a comment
Owner
There was a problem hiding this comment.
Patch 2
It wouldn't hurt to mention the transition from skb-saved-in-buffer to skb-recorded-on-ring model in the commit message.
Fixed XDP_DROP/ABORTED: reaches ixgbe_put_rx_buffer() but page
is never
Either you have fixed an existing problem in code, in which case you should submit a patch to a stable tree (net-queue in this case) prior to sending the -next series.
Or you have fixed an issue compared to the previous version of the series, in which case you have 2 options:
- changelog in the cover letter (PR comment in this case)
- changelog under '---' in the formatted patch (right under
Signed-off-by: Magdalena Pytel <magdalena.pytel@intel.com>)
Fix this by calling vlan_vids_del_by_dev(port->dev, master->dev) in
hsr_del_port() before unlinking slave A or slave B ports, matching the
propagation logic in hsr_ndo_vlan_rx_add_vid() / hsr_ndo_vlan_rx_kill_vid()
and the cleanup behavior in bonding and team drivers.
Fixes: 1a8a63a5305e ("net: hsr: Add VLAN CTAG filter support")
Reported-by: syzbot+456957213f32970c0762@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6a4cb6ca.57639fcc.86d58.000b.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Felix Maurer <fmaurer@redhat.com>
Cc: Fernando Fernandez Mancera <fmancera@suse.de>
---
v2: incorporated Sashiko's feedback
v1: https://lore.kernel.org/netdev/20260707082327.3238690-1-edumazet@google.com/
net/hsr/hsr_slave.c | 2 ++
1 file changed, 2 insertions(+)
You can leave the changelog inside of the commit message for the github review, but best distinguish it in a way that makes it immediately clear that this is not a real part of a commit message, smth like [INTERNAL ONLY!!!], so it is straightforward to delete later
| * this is twice the size of a half page we need to double the page order | ||
| * for FCoE enabled Rx queues. | ||
| */ | ||
| static inline unsigned int ixgbe_rx_bufsz(struct ixgbe_ring *ring) |
Owner
There was a problem hiding this comment.
Deletion of this function leads to compilation errors
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c: In function ‘ixgbe_clean_test_rings’:
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c:2122:41: error: implicit declaration of function ‘ixgbe_rx_bufsz’ [-Wimplicit-function-declaration]
2122 | ixgbe_rx_bufsz(rx_ring),
| ^~~~~~~~~~~~~~
make[7]: *** [scripts/Makefile.build:289: drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.o] Error 1
diff to fix:
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index eaf50ce47fe5..c5ad98ca1a66 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2119,7 +2119,7 @@ static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
/* sync Rx buffer for CPU read */
dma_sync_single_for_cpu(rx_ring->dev,
rx_buffer->dma,
- ixgbe_rx_bufsz(rx_ring),
+ IXGBE_RXBUFFER_3K,
DMA_FROM_DEVICE);
/* verify contents of skb */
@@ -2131,7 +2131,7 @@ static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
/* sync Rx buffer for device write */
dma_sync_single_for_device(rx_ring->dev,
rx_buffer->dma,
- ixgbe_rx_bufsz(rx_ring),
+ IXGBE_RXBUFFER_3K,
DMA_FROM_DEVICE);
/* increment Rx next to clean counter */
| page_ref_add(page, USHRT_MAX - 1); | ||
| bi->pagecnt_bias = USHRT_MAX; | ||
| bi->page_offset = rx_ring->xdp_prog ? XDP_PACKET_HEADROOM : | ||
| IXGBE_SKB_PAD; |
Owner
There was a problem hiding this comment.
Text is not aligned correctly
| #endif /* IXGBE_FCOE */ | ||
| u16 cleaned_count = ixgbe_desc_unused(rx_ring); | ||
| unsigned int offset = rx_ring->rx_offset; | ||
| unsigned int offset = rx_ring->xdp_prog ? XDP_PACKET_HEADROOM : |
Owner
There was a problem hiding this comment.
This is the same as page_offset now
| @@ -2476,20 +2242,13 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector, | |||
| rx_buffer->page_offset - offset; | |||
Owner
There was a problem hiding this comment.
rx_buffer->page_offset - offset equals zero now.
| dma_unmap_page_attrs(rx_ring->dev, rx_buffer->dma, PAGE_SIZE, | ||
| DMA_FROM_DEVICE, IXGBE_RX_DMA_ATTR); | ||
| if (put_page_flag) | ||
| put_page(rx_buffer->page); |
Owner
There was a problem hiding this comment.
Any particular reason you use put_page instead of __free_page() here?
| if ((adapter->flags & IXGBE_FLAG_FCOE_ENABLED) && | ||
| (max_frame < IXGBE_FCOE_JUMBO_FRAME_SIZE)) | ||
| max_frame = IXGBE_FCOE_JUMBO_FRAME_SIZE; | ||
|
|
| /* RXDCTL.RLPML does not work on 82599 */ | ||
| } else if (hw->mac.type != ixgbe_mac_82599EB) { | ||
| rxdctl &= ~(IXGBE_RXDCTL_RLPMLMASK | | ||
| IXGBE_RXDCTL_RLPML_EN); |
Owner
There was a problem hiding this comment.
I think we still have to disable rxdctl, if buffer is 3K
| } | ||
|
|
||
| /* initialize rx_buffer_info */ | ||
| memset(ring->rx_buffer_info, 0, |
Owner
There was a problem hiding this comment.
We are still reusing the array, so best to zero it
|
|
||
| clear_ring_rsc_enabled(rx_ring); | ||
| if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) | ||
| set_ring_rsc_enabled(rx_ring); |
Owner
There was a problem hiding this comment.
ixgbe_configure_rx_ring() is called in at least 2 other places. I think they need RSC flag too.
walking-machine
requested changes
Jul 21, 2026
| /* exit if we failed to retrieve a buffer */ | ||
| if (!xdp_res && !skb) { | ||
| rx_ring->rx_stats.alloc_rx_buff_failed++; | ||
| ixgbe_put_rx_buffer(rx_ring, rx_buffer, true); |
Owner
There was a problem hiding this comment.
This does not compile :(
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c: In function ‘ixgbe_clean_rx_irq’:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2159:25: error: implicit declaration of function ‘ixgbe_put_rx_buffer’; did you mean ‘ixgbe_print_buffer’? [-Wimplicit-function-declaration]
2159 | ixgbe_put_rx_buffer(rx_ring, rx_buffer, true);
| ^~~~~~~~~~~~~~~~~~~
| ixgbe_print_buffer
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c: In function ‘ixgbe_setup_rx_resources’:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:6669:19: error: ‘struct libeth_fq’ has no member named ‘no_napi’
6669 | fq.no_napi = true;
| ^
To fix:
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aa682cb23b2b..2d83bfb309db 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2156,7 +2156,6 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
/* exit if we failed to retrieve a buffer */
if (!xdp_res && !skb) {
rx_ring->rx_stats.alloc_rx_buff_failed++;
- ixgbe_put_rx_buffer(rx_ring, rx_buffer, true);
cleaned_count++;
break;
}
AND
cherry-pick commit "libeth: allow to create fill queues without NAPI" to the beginning of the series
|
|
||
| MODULE_IMPORT_NS("LIBIE_FWLOG"); | ||
| MODULE_DESCRIPTION("Intel(R) 10 Gigabit PCI Express Network Driver"); | ||
| MODULE_IMPORT_NS("LIBETH"); |
Owner
There was a problem hiding this comment.
ERROR: modpost: module ixgbe uses symbol libeth_xdp_buff_add_frag from namespace LIBETH_XDP, but does not import it.
Please do:
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index fa7bd4a666ae..3cef92c1a559 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -176,6 +176,7 @@ MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
MODULE_IMPORT_NS("LIBIE_FWLOG");
MODULE_DESCRIPTION("Intel(R) 10 Gigabit PCI Express Network Driver");
MODULE_IMPORT_NS("LIBETH");
+MODULE_IMPORT_NS("LIBETH_XDP");
MODULE_LICENSE("GPL v2");
DEFINE_STATIC_KEY_FALSE(ixgbe_xdp_locking_key);
| /* Setup Rx Descriptor ring and Rx buffers */ | ||
| rx_ring->count = IXGBE_DEFAULT_RXD; | ||
| rx_ring->queue_index = 0; | ||
| rx_ring->dev = &adapter->pdev->dev; |
Owner
There was a problem hiding this comment.
There is still ring->dev = ... for an Rx ring in ixgbe_alloc_q_vector(). This is a mistake that you have admittedly inherited from me ;)
| if (ret < 0) | ||
| goto err_free_desc; | ||
|
|
||
| WRITE_ONCE(rx_ring->xdp_prog, adapter->xdp_prog); |
Owner
There was a problem hiding this comment.
You have made xdp prog an _rcu pointer here, so to avoid sparse warnings, use rcu_assign_pointer()
| } else { | ||
| WARN_ON(xdp_rxq_info_reg_mem_model(&ring->xdp_rxq, | ||
| MEM_TYPE_PAGE_SHARED, NULL)); | ||
| xdp_rxq_info_attach_page_pool(&ring->xdp_rxq, ring->pp); |
Owner
There was a problem hiding this comment.
rxq_info is not registered at this point, and xdp_rxq_info_attach_page_pool() does not perform this itself
| srrctl |= IXGBE_RXBUFFER_3K >> IXGBE_SRRCTL_BSIZEPKT_SHIFT; | ||
| } | ||
| if (ring->xsk_pool) | ||
| srrctl |= DIV_ROUND_UP(xsk_pool_get_rx_frame_size(ring->xsk_pool), |
Owner
There was a problem hiding this comment.
I think you shouldn't have touched the xsk srrctl programming in this patch.
But if you want to. We cannot control xsk pool buffer size, unlike page_pool. User provides us with xsk buffers. Those are usually 2K or 4K in total, so HW-writable size is always less, because we have both headroom and tailroom, so without RLPML there will be overlap, if we do DIV_ROUND_UP.
What I would do here in case of ixgbe_mac_82599EB, is just do regular DIV(), so that e.g. 3400 frame size will become 3K.
Owner
There was a problem hiding this comment.
This is a little bit more complex, since in ixgbevf we could just not support this particular HW, because the implementation was new.
| ring->rx_buf_len = xsk_buf_len; | ||
| rxdctl &= ~(IXGBE_RXDCTL_RLPMLMASK | IXGBE_RXDCTL_RLPML_EN); | ||
| if (pkt_len <= IXGBE_RXDCTL_RLPMLMASK) | ||
| rxdctl |= pkt_len | IXGBE_RXDCTL_RLPML_EN; |
Owner
There was a problem hiding this comment.
Those are not correct rlpml calculations for an xsk ZC ring
| { | ||
| struct ixgbe_hw *hw = &adapter->hw; | ||
| struct net_device *netdev = adapter->netdev; | ||
| int max_frame = netdev->mtu + ETH_HLEN + ETH_FCS_LEN; |
Owner
There was a problem hiding this comment.
Why not READ_ONCE(adapter->netdev->mtu) + LIBETH_RX_LL_LEN;?
| int orig_node = dev_to_node(dev); | ||
| void *napi_dev; | ||
| int ring_node = NUMA_NO_NODE; | ||
| int size; |
Owner
There was a problem hiding this comment.
We generally try to follow so called Reverse Christmas Tree ordering of declarations
Owner
|
Patch 4 looks fine, but I advise to squash it into patch 3, so we can be sure XDP and AF_XDP will fit into 1 upstream PR (15 patches) |
Owner
|
Build errors in each patch, which I have not yet reviewed: patch 6 patch 7 patch 8 patch 9 patch 10 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Convert the ixgbe Rx path from the legacy page-sharing buffer
management to libeth and page_pool, following the same approach
used in the ixgbevf conversion.
Patch 1 removes the legacy-rx ethtool private flag and makes
build_skb the only Rx code path, simplifying the driver for the
conversion.
Patch 2 converts ixgbe Rx path from page-sharing/recycling
to a simple one-page-per-packet model. Remove logic that refers
to page-sharing buffer management. This is a preparatory step
before converting the Rx path to page_pool and libeth.
Patch 3 Convert ixgbe Rx path from manual page-per-packet DMA
management to libeth/page_pool infrastructure, and integrates
libeth XDP helpers. Each packet gets its own buffer instead of
sharing pages between packets.
Patch 4 adds branch prediction hints and minor cleanups to the
Rx path, improving performance after the libeth conversion.
Patch 5 adds XDP multi-buffer support on the Rx path, allowing
XDP programs with frags support to handle packets that span
multiple buffers.
Patch 6 adds XDP_TX support in multi-buffer mode through libeth,
enabling XDP programs to transmit multi-buffer packets back out
the same interface.
Patch 7 adds XDP_REDIRECT and .ndo_xdp_xmit support through
libeth, enabling redirection of packets between interfaces via
XDP.
Patch 8 introduces pseudo header split support, copying packet
headers into a separate buffer
Patch 9 adds error handling for page pool creation in
ixgbe_configure_rx_ring() and allows buffer reallocation when
the page pool is not present.
Patch 10 eases the MTU restriction when an XDP program is
attached. With libeth page pool and multi-buffer XDP support,
MTU changes are allowed as long as the XDP program supports
frags or the frame fits in a single buffer.
Patch 1 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m5a77d9a3d6495b21d80e1f90d49470914ed950dc
Patch 2 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m4956a9538e51e0e0486b5d577fa3a1899897ba28
Patch 3 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m67766454f9439e608779ad85291e43e9de4b568d
Patch 4 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#mb10a5ccd87dc79065e6f8bcc723101b9b4966d27
Patch 5 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m74bb0fc2103acefa7a222b94bcd00cc4dae61eb1
Patch 6 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#mcc688bbe93c754d0a5e36e18e706505037f7ac51
Patch 7 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m7f0a243d405642def7f22e97a9490a2a85851886
Patch 8 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m81aae544a7e63736f2d8897d3cbaf78005bc8bd3
Patch 9 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m172f6be60eec974111c5065e8c8c2fab6f0d855b
Patch 10 based on:
commit: https://lore.kernel.org/netdev/20260304160345.1340940-1-larysa.zaremba@intel.com/T/#m5e0629f3a8d72c85b1527ef8727c9ee07a7b1428