Re: [PATCH net-next 2/2] net: atlantic: convert RX path to page_pool

The driver currently allocates RX buffers with dev_alloc_pages(), maps
them with dma_map_page(), and uses a hand-rolled page-flip scheme to
subdivide high-order pages. Behind an IOMMU, the map/unmap churn is a
major RX cost: on a Thunderbolt-attached QNAP QNA-T310G1S, iperf3 -R
over IPv6 tops out at about 2.2 Gbit/s over MTU 1500.

Convert RX buffers to page_pool fragments. Pages are DMA-mapped once
when entering the pool and recycled through the stack or XDP via the
MEM_TYPE_PAGE_POOL memory model. This removes the custom page-flip
accounting, lets page_pool handle fragment reuse, and ensures every RX
path either keeps the ring's fragment reference for reposting or hands
it to the skb/xdp_buff for later recycling.

Register the PTP RX ring's xdp_rxq as well, since it shares the RX
clean paths. Walk the full RX ring during deinit so consumed but not yet
refilled slots cannot leak page-pool references. Drop the ethtool
PageFlips/PageReuses/PageFrees counters which only described the old
scheme.

On the QNA-T310G1S, MTU 1500, TCP over IPv6, iperf3 -R improves from
2.24 Gbit/s to 9.14 Gbit/s. The module was also smoke-tested with native
XDP PASS, DROP, and ABORTED actions; carrier recovered after each
attach/detach cycle and dmesg showed no page_pool/DMA warnings.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Yangyu Chen <redacted>
---
 drivers/net/ethernet/aquantia/Kconfig         |   1 +
 .../ethernet/aquantia/atlantic/aq_ethtool.c   |   3 -
 .../net/ethernet/aquantia/atlantic/aq_ptp.c   |  20 +-
 .../net/ethernet/aquantia/atlantic/aq_ring.c  | 238 ++++++++----------
 .../net/ethernet/aquantia/atlantic/aq_ring.h  |   6 +-
 .../net/ethernet/aquantia/atlantic/aq_vec.c   |  15 +-
 6 files changed, 140 insertions(+), 143 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/Kconfig b/drivers/net/ethernet/aquantia/Kconfig
index cec2018c84a9..c8fb7b33e5b7 100644
--- a/drivers/net/ethernet/aquantia/Kconfig
+++ b/drivers/net/ethernet/aquantia/Kconfig

@@ -20,6 +20,7 @@ config AQTION
        tristate "aQuantia AQtion(tm) Support"
        depends on PCI
        depends on MACSEC || MACSEC=n
+       select PAGE_POOL
        help
          This enables the support for the aQuantia AQtion(tm) Ethernet card.

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
index 420af958d486..0f5125bd2315 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c

@@ -100,9 +100,6 @@ static const char * const aq_ethtool_queue_rx_stat_names[] = {
        "%sQueue[%d] AllocFails",
        "%sQueue[%d] SkbAllocFails",
        "%sQueue[%d] Polls",
-       "%sQueue[%d] PageFlips",
-       "%sQueue[%d] PageReuses",
-       "%sQueue[%d] PageFrees",
        "%sQueue[%d] XdpAbort",
        "%sQueue[%d] XdpDrop",
        "%sQueue[%d] XdpPass",

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ptp.c b/drivers/net/ethernet/aquantia/atlantic/aq_ptp.c
index 558ac9237f75..5b8a4d0d1cff 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ptp.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ptp.c

@@ -13,6 +13,7 @@
 #include <linux/ptp_classify.h>
 #include <linux/interrupt.h>
 #include <linux/clocksource.h>
+#include <net/xdp.h>

 #include "aq_nic.h"
 #include "aq_ptp.h"

@@ -1192,12 +1193,25 @@ int aq_ptp_ring_alloc(struct aq_nic_s *aq_nic)
        if (err)
                goto err_exit_ptp_tx;

+       err = xdp_rxq_info_reg(&aq_ptp->ptp_rx.xdp_rxq, aq_nic->ndev,
+                              rx_ring_idx, aq_ptp->napi.napi_id);
+       if (err < 0)
+               goto err_exit_ptp_rx;
+
+       err = xdp_rxq_info_reg_mem_model(&aq_ptp->ptp_rx.xdp_rxq,
+                                        MEM_TYPE_PAGE_POOL,
+                                        aq_ptp->ptp_rx.pg_pool);
+       if (err < 0) {
+               xdp_rxq_info_unreg(&aq_ptp->ptp_rx.xdp_rxq);
+               goto err_exit_ptp_rx;

+       }
+
        if (aq_ptp->a1_ptp) {
                err = aq_ring_hwts_rx_alloc(&aq_ptp->hwts_rx, aq_nic, PTP_HWST_RING_IDX,
                                            aq_nic->aq_nic_cfg.rxds,
                                            aq_nic->aq_nic_cfg.aq_hw_caps->rxd_size);
                if (err)
-                       goto err_exit_ptp_rx;
+                       goto err_exit_xdp_rxq;
        }

        err = aq_ptp_skb_ring_init(&aq_ptp->skb_ring, aq_nic->aq_nic_cfg.rxds);

@@ -1217,6 +1231,8 @@ int aq_ptp_ring_alloc(struct aq_nic_s *aq_nic)
 err_exit_hwts_rx:
        if (aq_ptp->a1_ptp)
                aq_ring_hwts_rx_free(&aq_ptp->hwts_rx);
+err_exit_xdp_rxq:
+       xdp_rxq_info_unreg(&aq_ptp->ptp_rx.xdp_rxq);
 err_exit_ptp_rx:
        aq_ring_free(&aq_ptp->ptp_rx);
 err_exit_ptp_tx:

@@ -1233,6 +1249,8 @@ void aq_ptp_ring_free(struct aq_nic_s *aq_nic)
                return;

        aq_ring_free(&aq_ptp->ptp_tx);
+       if (xdp_rxq_info_is_reg(&aq_ptp->ptp_rx.xdp_rxq))
+               xdp_rxq_info_unreg(&aq_ptp->ptp_rx.xdp_rxq);
        aq_ring_free(&aq_ptp->ptp_rx);
        if (aq_ptp->a1_ptp)
                aq_ring_hwts_rx_free(&aq_ptp->hwts_rx);

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index c59b6c0b37f1..e06c502b59f9 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c

@@ -14,120 +14,37 @@
 #include "aq_vec.h"
 #include "aq_main.h"

+#include <net/page_pool/helpers.h>
 #include <net/xdp.h>
 #include <linux/filter.h>
 #include <linux/bpf_trace.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>

-static void aq_get_rxpages_xdp(struct aq_ring_buff_s *buff,
-                              struct xdp_buff *xdp)
-{
-       struct skb_shared_info *sinfo;
-       int i;
-
-       if (xdp_buff_has_frags(xdp)) {
-               sinfo = xdp_get_shared_info_from_buff(xdp);
-
-               for (i = 0; i < sinfo->nr_frags; i++) {
-                       skb_frag_t *frag = &sinfo->frags[i];
-
-                       page_ref_inc(skb_frag_page(frag));
-               }
-       }
-       page_ref_inc(buff->rxdata.page);
-}
-
-static inline void aq_free_rxpage(struct aq_rxpage *rxpage, struct device *dev)
-{
-       unsigned int len = PAGE_SIZE << rxpage->order;
-
-       dma_unmap_page(dev, rxpage->daddr, len, DMA_FROM_DEVICE);
-
-       /* Drop the ref for being in the ring. */
-       __free_pages(rxpage->page, rxpage->order);
-       rxpage->page = NULL;
-}
-
-static int aq_alloc_rxpages(struct aq_rxpage *rxpage, struct aq_ring_s *rx_ring)
+static int aq_get_rxpages(struct aq_ring_s *self, struct aq_ring_buff_s *rxbuf)
 {
-       struct device *dev = aq_nic_get_dev(rx_ring->aq_nic);
-       unsigned int order = rx_ring->page_order;
+       unsigned int size = self->page_offset + self->frame_max +
+                           self->tail_size;
+       unsigned int pg_off;
        struct page *page;
-       int ret = -ENOMEM;
-       dma_addr_t daddr;
-
-       page = dev_alloc_pages(order);
-       if (unlikely(!page))
-               goto err_exit;

-       daddr = dma_map_page(dev, page, 0, PAGE_SIZE << order,
-                            DMA_FROM_DEVICE);
+       /* Buffers whose page was not passed up the stack are reposted
+        * with the data they already carry discarded.
+        */
+       if (rxbuf->rxdata.page)
+               return 0;

-       if (unlikely(dma_mapping_error(dev, daddr)))
-               goto free_page;
-
-       rxpage->page = page;
-       rxpage->daddr = daddr;
-       rxpage->order = order;
-       rxpage->pg_off = rx_ring->page_offset;
-
-       return 0;
-
-free_page:
-       __free_pages(page, order);
-
-err_exit:
-       return ret;
-}
-
-static int aq_get_rxpages(struct aq_ring_s *self, struct aq_ring_buff_s *rxbuf)
-{
-       unsigned int order = self->page_order;
-       u16 page_offset = self->page_offset;
-       u16 frame_max = self->frame_max;
-       u16 tail_size = self->tail_size;
-       int ret;
-
-       if (rxbuf->rxdata.page) {
-               /* One means ring is the only user and can reuse */
-               if (page_ref_count(rxbuf->rxdata.page) > 1) {
-                       /* Try reuse buffer */
-                       rxbuf->rxdata.pg_off += frame_max + page_offset +
-                                               tail_size;
-                       if (rxbuf->rxdata.pg_off + frame_max + tail_size <=
-                           (PAGE_SIZE << order)) {
-                               u64_stats_update_begin(&self->stats.rx.syncp);
-                               self->stats.rx.pg_flips++;
-                               u64_stats_update_end(&self->stats.rx.syncp);
-
-                       } else {
-                               /* Buffer exhausted. We have other users and
-                                * should release this page and realloc
-                                */
-                               aq_free_rxpage(&rxbuf->rxdata,
-                                              aq_nic_get_dev(self->aq_nic));
-                               u64_stats_update_begin(&self->stats.rx.syncp);
-                               self->stats.rx.pg_losts++;
-                               u64_stats_update_end(&self->stats.rx.syncp);
-                       }
-               } else {
-                       rxbuf->rxdata.pg_off = page_offset;
-                       u64_stats_update_begin(&self->stats.rx.syncp);
-                       self->stats.rx.pg_reuses++;
-                       u64_stats_update_end(&self->stats.rx.syncp);
-               }
+       page = page_pool_dev_alloc_frag(self->pg_pool, &pg_off, size);
+       if (unlikely(!page)) {
+               u64_stats_update_begin(&self->stats.rx.syncp);
+               self->stats.rx.alloc_fails++;
+               u64_stats_update_end(&self->stats.rx.syncp);
+               return -ENOMEM;
        }

-       if (!rxbuf->rxdata.page) {
-               ret = aq_alloc_rxpages(&rxbuf->rxdata, self);
-               if (ret) {
-                       u64_stats_update_begin(&self->stats.rx.syncp);
-                       self->stats.rx.alloc_fails++;
-                       u64_stats_update_end(&self->stats.rx.syncp);
-               }
-               return ret;
-       }
+       rxbuf->rxdata.page = page;
+       rxbuf->rxdata.daddr = page_pool_get_dma_addr(page);
+       rxbuf->rxdata.pg_off = pg_off + self->page_offset;

        return 0;
 }