[PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool

[PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool

[Brett Creeley]

From: Shannon Nelson <shannon.nelson@amd.com>

Our home-grown buffer management needs to go away and we need to play
nicely with the page_pool infrastructure.  This patchset cleans up some
of our API use and converts the Rx traffic queues to use page_pool.
The first few patches are for tidying up things before the last patch
which does the actual conversion.

The result is code that more closely follows current patterns, as well as
a either a performance boost or equivalent performance as seen with
iperf testing:

  mss   netio    tx_pps      rx_pps      total_pps      tx_bw    rx_bw    total_bw
  ----  -------  ----------  ----------  -----------  -------  -------  ----------
Before:
  256   bidir    13,839,293  15,515,227  29,354,520        34       38          71
  512   bidir    13,913,249  14,671,693  28,584,942        62       65         127
  1024  bidir    13,006,189  13,695,413  26,701,602       109      115         224
  1448  bidir    12,489,905  12,791,734  25,281,639       145      149         294
  2048  bidir    9,195,622   9,247,649   18,443,271       148      149         297
  4096  bidir    5,149,716   5,247,917   10,397,633       160      163         323
  8192  bidir    3,029,993   3,008,882   6,038,875        179      179         358
  9000  bidir    2,789,358   2,800,744   5,590,102        181      180         361

After:
  256   bidir    21,540,037  21,344,644  42,884,681        52       52         104
  512   bidir    23,170,014  19,207,260  42,377,274       103       85         188
  1024  bidir    17,934,280  17,819,247  35,753,527       150      149         299
  1448  bidir    15,242,515  14,907,030  30,149,545       167      174         341
  2048  bidir    10,692,542  10,663,023  21,355,565       177      176         353
  4096  bidir    6,024,977   6,083,580   12,108,557       187      180         367
  8192  bidir    3,090,449   3,048,266   6,138,715        180      176         356
  9000  bidir    2,859,146   2,864,226   5,723,372        178      180         358

v2:
 - split out prep work into separate patches
 - reworked to always use rxq_info structs
 - be sure we're in NAPI when calling xdp_return_frame_rx_napi()
 - fixed up issues with buffer size and lifetime management

v1/RFC:
 - Link: https://lore.kernel.org/netdev/20240625165658.34598-1-shannon.nelson@amd.com/

Shannon Nelson (5):
  ionic: debug line for Tx completion errors
  ionic: rename ionic_xdp_rx_put_bufs
  ionic: use per-queue xdp_prog
  ionic: always use rxq_info
  ionic: convert Rx queue buffers to use page_pool

 drivers/net/ethernet/pensando/Kconfig         |   1 +
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  21 +-
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 121 +++---
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 372 +++++++++---------
 4 files changed, 267 insertions(+), 248 deletions(-)

-- 
2.17.1

[PATCH v2 net-next 1/5] ionic: debug line for Tx completion errors

[Brett Creeley]

From: Shannon Nelson <shannon.nelson@amd.com>

Here's a little debugging aid in case the device starts throwing
Tx completion errors.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index fc79baad4561..ccdc0eefabe4 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -648,7 +648,14 @@ static void ionic_rx_clean(struct ionic_queue *q,
 
 	stats = q_to_rx_stats(q);
 
-	if (comp->status) {
+	if (unlikely(comp->status)) {
+		/* Most likely status==2 and the pkt received was bigger
+		 * than the buffer available: comp->len will show the
+		 * pkt size received that didn't fit the advertised desc.len
+		 */
+		dev_dbg(q->dev, "q%d drop comp->status %d comp->len %d desc->len %d\n",
+			q->index, comp->status, comp->len, q->rxq[q->head_idx].len);
+
 		stats->dropped++;
 		return;
 	}
-- 
2.17.1

[PATCH v2 net-next 2/5] ionic: rename ionic_xdp_rx_put_bufs

[Brett Creeley]

From: Shannon Nelson <shannon.nelson@amd.com>

We aren't "putting" buf, we're just unlinking them from our tracking in
order to let the XDP_TX and XDP_REDIRECT tx clean paths take care of the
pages when they are done with them.  This rename clears up the intent.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index ccdc0eefabe4..d62b2b60b133 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -481,9 +481,9 @@ int ionic_xdp_xmit(struct net_device *netdev, int n,
 	return nxmit;
 }
 
-static void ionic_xdp_rx_put_bufs(struct ionic_queue *q,
-				  struct ionic_buf_info *buf_info,
-				  int nbufs)
+static void ionic_xdp_rx_unlink_bufs(struct ionic_queue *q,
+				     struct ionic_buf_info *buf_info,
+				     int nbufs)
 {
 	int i;
 
@@ -600,7 +600,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 			netdev_dbg(netdev, "tx ionic_xdp_post_frame err %d\n", err);
 			goto out_xdp_abort;
 		}
-		ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs);
+		ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
 		stats->xdp_tx++;
 
 		/* the Tx completion will free the buffers */
@@ -612,7 +612,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 			netdev_dbg(netdev, "xdp_do_redirect err %d\n", err);
 			goto out_xdp_abort;
 		}
-		ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs);
+		ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
 		rxq->xdp_flush = true;
 		stats->xdp_redirect++;
 		break;
-- 
2.17.1

[PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog

[Brett Creeley]

From: Shannon Nelson <shannon.nelson@amd.com>

We originally were using a per-interface xdp_prog variable to track
a loaded XDP program since we knew there would never be support for a
per-queue XDP program.  With that, we only built the per queue rxq_info
struct when an XDP program was loaded and removed it on XDP program unload,
and used the pointer as an indicator in the Rx hotpath to know to how build
the buffers.  However, that's really not the model generally used, and
makes a conversion to page_pool Rx buffer cacheing a little problematic.

This patch converts the driver to use the more common approach of using
a per-queue xdp_prog pointer to work out buffer allocations and need
for bpf_prog_run_xdp().  We jostle a couple of fields in the queue struct
in order to keep the new xdp_prog pointer in a warm cacheline.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  7 +++++--
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 14 +++++++------
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 21 +++++++++----------
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index c647033f3ad2..19ae68a86a0b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -238,9 +238,8 @@ struct ionic_queue {
 	unsigned int index;
 	unsigned int num_descs;
 	unsigned int max_sg_elems;
+
 	u64 features;
-	unsigned int type;
-	unsigned int hw_index;
 	unsigned int hw_type;
 	bool xdp_flush;
 	union {
@@ -261,7 +260,11 @@ struct ionic_queue {
 		struct ionic_rxq_sg_desc *rxq_sgl;
 	};
 	struct xdp_rxq_info *xdp_rxq_info;
+	struct bpf_prog *xdp_prog;
 	struct ionic_queue *partner;
+
+	unsigned int type;
+	unsigned int hw_index;
 	dma_addr_t base_pa;
 	dma_addr_t cmb_base_pa;
 	dma_addr_t sg_base_pa;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index aa0cc31dfe6e..0fba2df33915 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2700,24 +2700,24 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
 
 static int ionic_xdp_queues_config(struct ionic_lif *lif)
 {
+	struct bpf_prog *xdp_prog;
 	unsigned int i;
 	int err;
 
 	if (!lif->rxqcqs)
 		return 0;
 
-	/* There's no need to rework memory if not going to/from NULL program.
-	 * If there is no lif->xdp_prog, there should also be no q.xdp_rxq_info
-	 * This way we don't need to keep an *xdp_prog in every queue struct.
-	 */
-	if (!lif->xdp_prog == !lif->rxqcqs[0]->q.xdp_rxq_info)
+	/* There's no need to rework memory if not going to/from NULL program.  */
+	xdp_prog = READ_ONCE(lif->xdp_prog);
+	if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
 		return 0;
 
 	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
 		struct ionic_queue *q = &lif->rxqcqs[i]->q;
 
-		if (q->xdp_rxq_info) {
+		if (q->xdp_prog) {
 			ionic_xdp_unregister_rxq_info(q);
+			q->xdp_prog = NULL;
 			continue;
 		}
 
@@ -2727,6 +2727,7 @@ static int ionic_xdp_queues_config(struct ionic_lif *lif)
 				i, err);
 			goto err_out;
 		}
+		q->xdp_prog = xdp_prog;
 	}
 
 	return 0;
@@ -2878,6 +2879,7 @@ static void ionic_swap_queues(struct ionic_qcq *a, struct ionic_qcq *b)
 	swap(a->q.base,       b->q.base);
 	swap(a->q.base_pa,    b->q.base_pa);
 	swap(a->q.info,       b->q.info);
+	swap(a->q.xdp_prog,   b->q.xdp_prog);
 	swap(a->q.xdp_rxq_info, b->q.xdp_rxq_info);
 	swap(a->q.partner,    b->q.partner);
 	swap(a->q_base,       b->q_base);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index d62b2b60b133..858ab4fd9218 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -190,7 +190,7 @@ static bool ionic_rx_buf_recycle(struct ionic_queue *q,
 	if (page_to_nid(buf_info->page) != numa_mem_id())
 		return false;
 
-	size = ALIGN(len, q->xdp_rxq_info ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
+	size = ALIGN(len, q->xdp_prog ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
 	buf_info->page_offset += size;
 	if (buf_info->page_offset >= IONIC_PAGE_SIZE)
 		return false;
@@ -639,8 +639,7 @@ static void ionic_rx_clean(struct ionic_queue *q,
 	struct net_device *netdev = q->lif->netdev;
 	struct ionic_qcq *qcq = q_to_qcq(q);
 	struct ionic_rx_stats *stats;
-	struct bpf_prog *xdp_prog;
-	unsigned int headroom;
+	unsigned int headroom = 0;
 	struct sk_buff *skb;
 	bool synced = false;
 	bool use_copybreak;
@@ -664,14 +663,13 @@ static void ionic_rx_clean(struct ionic_queue *q,
 	stats->pkts++;
 	stats->bytes += len;
 
-	xdp_prog = READ_ONCE(q->lif->xdp_prog);
-	if (xdp_prog) {
-		if (ionic_run_xdp(stats, netdev, xdp_prog, q, desc_info->bufs, len))
+	if (q->xdp_prog) {
+		if (ionic_run_xdp(stats, netdev, q->xdp_prog, q, desc_info->bufs, len))
 			return;
 		synced = true;
+		headroom = XDP_PACKET_HEADROOM;
 	}
 
-	headroom = q->xdp_rxq_info ? XDP_PACKET_HEADROOM : 0;
 	use_copybreak = len <= q->lif->rx_copybreak;
 	if (use_copybreak)
 		skb = ionic_rx_copybreak(netdev, q, desc_info,
@@ -814,7 +812,7 @@ void ionic_rx_fill(struct ionic_queue *q)
 	len = netdev->mtu + VLAN_ETH_HLEN;
 
 	for (i = n_fill; i; i--) {
-		unsigned int headroom;
+		unsigned int headroom = 0;
 		unsigned int buf_len;
 
 		nfrags = 0;
@@ -835,11 +833,12 @@ void ionic_rx_fill(struct ionic_queue *q)
 		 * XDP uses space in the first buffer, so account for
 		 * head room, tail room, and ip header in the first frag size.
 		 */
-		headroom = q->xdp_rxq_info ? XDP_PACKET_HEADROOM : 0;
-		if (q->xdp_rxq_info)
+		if (q->xdp_prog) {
 			buf_len = IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN;
-		else
+			headroom = XDP_PACKET_HEADROOM;
+		} else {
 			buf_len = ionic_rx_buf_size(buf_info);
+		}
 		frag_len = min_t(u16, len, buf_len);
 
 		desc->addr = cpu_to_le64(ionic_rx_buf_pa(buf_info) + headroom);
-- 
2.17.1

Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog

[Larysa Zaremba]

On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
> From: Shannon Nelson <shannon.nelson@amd.com>
> 
> We originally were using a per-interface xdp_prog variable to track
> a loaded XDP program since we knew there would never be support for a
> per-queue XDP program.  With that, we only built the per queue rxq_info
> struct when an XDP program was loaded and removed it on XDP program unload,
> and used the pointer as an indicator in the Rx hotpath to know to how build
> the buffers.  However, that's really not the model generally used, and
> makes a conversion to page_pool Rx buffer cacheing a little problematic.
> 
> This patch converts the driver to use the more common approach of using
> a per-queue xdp_prog pointer to work out buffer allocations and need
> for bpf_prog_run_xdp().  We jostle a couple of fields in the queue struct
> in order to keep the new xdp_prog pointer in a warm cacheline.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> Signed-off-by: Brett Creeley <brett.creeley@amd.com>

Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>

If you happen to send another version, please include in a commit message a note 
about READ_ONCE() removal. The removal itself is OK, but an indication that this 
was intentional would be nice.

> ---
>  .../net/ethernet/pensando/ionic/ionic_dev.h   |  7 +++++--
>  .../net/ethernet/pensando/ionic/ionic_lif.c   | 14 +++++++------
>  .../net/ethernet/pensando/ionic/ionic_txrx.c  | 21 +++++++++----------
>  3 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> index c647033f3ad2..19ae68a86a0b 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> @@ -238,9 +238,8 @@ struct ionic_queue {
>  	unsigned int index;
>  	unsigned int num_descs;
>  	unsigned int max_sg_elems;
> +
>  	u64 features;
> -	unsigned int type;
> -	unsigned int hw_index;
>  	unsigned int hw_type;
>  	bool xdp_flush;
>  	union {
> @@ -261,7 +260,11 @@ struct ionic_queue {
>  		struct ionic_rxq_sg_desc *rxq_sgl;
>  	};
>  	struct xdp_rxq_info *xdp_rxq_info;
> +	struct bpf_prog *xdp_prog;
>  	struct ionic_queue *partner;
> +
> +	unsigned int type;
> +	unsigned int hw_index;
>  	dma_addr_t base_pa;
>  	dma_addr_t cmb_base_pa;
>  	dma_addr_t sg_base_pa;
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index aa0cc31dfe6e..0fba2df33915 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -2700,24 +2700,24 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
>  
>  static int ionic_xdp_queues_config(struct ionic_lif *lif)
>  {
> +	struct bpf_prog *xdp_prog;
>  	unsigned int i;
>  	int err;
>  
>  	if (!lif->rxqcqs)
>  		return 0;
>  
> -	/* There's no need to rework memory if not going to/from NULL program.
> -	 * If there is no lif->xdp_prog, there should also be no q.xdp_rxq_info
> -	 * This way we don't need to keep an *xdp_prog in every queue struct.
> -	 */
> -	if (!lif->xdp_prog == !lif->rxqcqs[0]->q.xdp_rxq_info)
> +	/* There's no need to rework memory if not going to/from NULL program.  */
> +	xdp_prog = READ_ONCE(lif->xdp_prog);
> +	if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
>  		return 0;
>  
>  	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
>  		struct ionic_queue *q = &lif->rxqcqs[i]->q;
>  
> -		if (q->xdp_rxq_info) {
> +		if (q->xdp_prog) {
>  			ionic_xdp_unregister_rxq_info(q);
> +			q->xdp_prog = NULL;
>  			continue;
>  		}
>  
> @@ -2727,6 +2727,7 @@ static int ionic_xdp_queues_config(struct ionic_lif *lif)
>  				i, err);
>  			goto err_out;
>  		}
> +		q->xdp_prog = xdp_prog;
>  	}
>  
>  	return 0;
> @@ -2878,6 +2879,7 @@ static void ionic_swap_queues(struct ionic_qcq *a, struct ionic_qcq *b)
>  	swap(a->q.base,       b->q.base);
>  	swap(a->q.base_pa,    b->q.base_pa);
>  	swap(a->q.info,       b->q.info);
> +	swap(a->q.xdp_prog,   b->q.xdp_prog);
>  	swap(a->q.xdp_rxq_info, b->q.xdp_rxq_info);
>  	swap(a->q.partner,    b->q.partner);
>  	swap(a->q_base,       b->q_base);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> index d62b2b60b133..858ab4fd9218 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> @@ -190,7 +190,7 @@ static bool ionic_rx_buf_recycle(struct ionic_queue *q,
>  	if (page_to_nid(buf_info->page) != numa_mem_id())
>  		return false;
>  
> -	size = ALIGN(len, q->xdp_rxq_info ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
> +	size = ALIGN(len, q->xdp_prog ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
>  	buf_info->page_offset += size;
>  	if (buf_info->page_offset >= IONIC_PAGE_SIZE)
>  		return false;
> @@ -639,8 +639,7 @@ static void ionic_rx_clean(struct ionic_queue *q,
>  	struct net_device *netdev = q->lif->netdev;
>  	struct ionic_qcq *qcq = q_to_qcq(q);
>  	struct ionic_rx_stats *stats;
> -	struct bpf_prog *xdp_prog;
> -	unsigned int headroom;
> +	unsigned int headroom = 0;
>  	struct sk_buff *skb;
>  	bool synced = false;
>  	bool use_copybreak;
> @@ -664,14 +663,13 @@ static void ionic_rx_clean(struct ionic_queue *q,
>  	stats->pkts++;
>  	stats->bytes += len;
>  
> -	xdp_prog = READ_ONCE(q->lif->xdp_prog);
> -	if (xdp_prog) {
> -		if (ionic_run_xdp(stats, netdev, xdp_prog, q, desc_info->bufs, len))
> +	if (q->xdp_prog) {
> +		if (ionic_run_xdp(stats, netdev, q->xdp_prog, q, desc_info->bufs, len))
>  			return;
>  		synced = true;
> +		headroom = XDP_PACKET_HEADROOM;
>  	}
>  
> -	headroom = q->xdp_rxq_info ? XDP_PACKET_HEADROOM : 0;
>  	use_copybreak = len <= q->lif->rx_copybreak;
>  	if (use_copybreak)
>  		skb = ionic_rx_copybreak(netdev, q, desc_info,
> @@ -814,7 +812,7 @@ void ionic_rx_fill(struct ionic_queue *q)
>  	len = netdev->mtu + VLAN_ETH_HLEN;
>  
>  	for (i = n_fill; i; i--) {
> -		unsigned int headroom;
> +		unsigned int headroom = 0;
>  		unsigned int buf_len;
>  
>  		nfrags = 0;
> @@ -835,11 +833,12 @@ void ionic_rx_fill(struct ionic_queue *q)
>  		 * XDP uses space in the first buffer, so account for
>  		 * head room, tail room, and ip header in the first frag size.
>  		 */
> -		headroom = q->xdp_rxq_info ? XDP_PACKET_HEADROOM : 0;
> -		if (q->xdp_rxq_info)
> +		if (q->xdp_prog) {
>  			buf_len = IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN;
> -		else
> +			headroom = XDP_PACKET_HEADROOM;
> +		} else {
>  			buf_len = ionic_rx_buf_size(buf_info);
> +		}
>  		frag_len = min_t(u16, len, buf_len);
>  
>  		desc->addr = cpu_to_le64(ionic_rx_buf_pa(buf_info) + headroom);
> -- 
> 2.17.1
> 
> 

Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog

[Larysa Zaremba]

On Tue, Aug 27, 2024 at 01:44:10PM +0200, Larysa Zaremba wrote:
> On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
> > From: Shannon Nelson <shannon.nelson@amd.com>
> > 
> > We originally were using a per-interface xdp_prog variable to track
> > a loaded XDP program since we knew there would never be support for a
> > per-queue XDP program.  With that, we only built the per queue rxq_info
> > struct when an XDP program was loaded and removed it on XDP program unload,
> > and used the pointer as an indicator in the Rx hotpath to know to how build
> > the buffers.  However, that's really not the model generally used, and
> > makes a conversion to page_pool Rx buffer cacheing a little problematic.
> > 
> > This patch converts the driver to use the more common approach of using
> > a per-queue xdp_prog pointer to work out buffer allocations and need
> > for bpf_prog_run_xdp().  We jostle a couple of fields in the queue struct
> > in order to keep the new xdp_prog pointer in a warm cacheline.
> > 
> > Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> > Signed-off-by: Brett Creeley <brett.creeley@amd.com>
> 
> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>
>

I would like to rewoke the tag, see below why.

> If you happen to send another version, please include in a commit message a note 
> about READ_ONCE() removal. The removal itself is OK, but an indication that this 
> was intentional would be nice.
> 
> > ---
> >  .../net/ethernet/pensando/ionic/ionic_dev.h   |  7 +++++--
> >  .../net/ethernet/pensando/ionic/ionic_lif.c   | 14 +++++++------
> >  .../net/ethernet/pensando/ionic/ionic_txrx.c  | 21 +++++++++----------
> >  3 files changed, 23 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> > index c647033f3ad2..19ae68a86a0b 100644
> > --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> > +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> > @@ -238,9 +238,8 @@ struct ionic_queue {
> >  	unsigned int index;
> >  	unsigned int num_descs;
> >  	unsigned int max_sg_elems;
> > +
> >  	u64 features;
> > -	unsigned int type;
> > -	unsigned int hw_index;
> >  	unsigned int hw_type;
> >  	bool xdp_flush;
> >  	union {
> > @@ -261,7 +260,11 @@ struct ionic_queue {
> >  		struct ionic_rxq_sg_desc *rxq_sgl;
> >  	};
> >  	struct xdp_rxq_info *xdp_rxq_info;
> > +	struct bpf_prog *xdp_prog;
> >  	struct ionic_queue *partner;
> > +
> > +	unsigned int type;
> > +	unsigned int hw_index;
> >  	dma_addr_t base_pa;
> >  	dma_addr_t cmb_base_pa;
> >  	dma_addr_t sg_base_pa;
> > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > index aa0cc31dfe6e..0fba2df33915 100644
> > --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > @@ -2700,24 +2700,24 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
> >  
> >  static int ionic_xdp_queues_config(struct ionic_lif *lif)
> >  {
> > +	struct bpf_prog *xdp_prog;
> >  	unsigned int i;
> >  	int err;
> >  
> >  	if (!lif->rxqcqs)
> >  		return 0;
> >  
> > -	/* There's no need to rework memory if not going to/from NULL program.
> > -	 * If there is no lif->xdp_prog, there should also be no q.xdp_rxq_info
> > -	 * This way we don't need to keep an *xdp_prog in every queue struct.
> > -	 */
> > -	if (!lif->xdp_prog == !lif->rxqcqs[0]->q.xdp_rxq_info)
> > +	/* There's no need to rework memory if not going to/from NULL program.  */
> > +	xdp_prog = READ_ONCE(lif->xdp_prog);
> > +	if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
> >  		return 0;

In a case when we replace a non-NULL program with another non-NULL program this 
would create a situation where lif and queues have different pointers on them.

> >  
> >  	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
> >  		struct ionic_queue *q = &lif->rxqcqs[i]->q;
> >  
> > -		if (q->xdp_rxq_info) {
> > +		if (q->xdp_prog) {
> >  			ionic_xdp_unregister_rxq_info(q);
> > +			q->xdp_prog = NULL;
> >  			continue;
> >  		}
> >  
> > @@ -2727,6 +2727,7 @@ static int ionic_xdp_queues_config(struct ionic_lif *lif)
> >  				i, err);
> >  			goto err_out;
> >  		}
> > +		q->xdp_prog = xdp_prog;
> >  	}
> >  
> >  	return 0;

[...]

Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog

[Brett Creeley]

On 8/27/2024 4:57 AM, Larysa Zaremba wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Tue, Aug 27, 2024 at 01:44:10PM +0200, Larysa Zaremba wrote:
>> On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
>>> From: Shannon Nelson <shannon.nelson@amd.com>
>>>
>>> We originally were using a per-interface xdp_prog variable to track
>>> a loaded XDP program since we knew there would never be support for a
>>> per-queue XDP program.  With that, we only built the per queue rxq_info
>>> struct when an XDP program was loaded and removed it on XDP program unload,
>>> and used the pointer as an indicator in the Rx hotpath to know to how build
>>> the buffers.  However, that's really not the model generally used, and
>>> makes a conversion to page_pool Rx buffer cacheing a little problematic.
>>>
>>> This patch converts the driver to use the more common approach of using
>>> a per-queue xdp_prog pointer to work out buffer allocations and need
>>> for bpf_prog_run_xdp().  We jostle a couple of fields in the queue struct
>>> in order to keep the new xdp_prog pointer in a warm cacheline.
>>>
>>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>>> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
>>
>> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>
>>
> 
> I would like to rewoke the tag, see below why.
> 
>> If you happen to send another version, please include in a commit message a note
>> about READ_ONCE() removal. The removal itself is OK, but an indication that this
>> was intentional would be nice.
>>
>>> ---
>>>   .../net/ethernet/pensando/ionic/ionic_dev.h   |  7 +++++--
>>>   .../net/ethernet/pensando/ionic/ionic_lif.c   | 14 +++++++------
>>>   .../net/ethernet/pensando/ionic/ionic_txrx.c  | 21 +++++++++----------
>>>   3 files changed, 23 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>>> index c647033f3ad2..19ae68a86a0b 100644
>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
>>> @@ -238,9 +238,8 @@ struct ionic_queue {
>>>      unsigned int index;
>>>      unsigned int num_descs;
>>>      unsigned int max_sg_elems;
>>> +
>>>      u64 features;
>>> -   unsigned int type;
>>> -   unsigned int hw_index;
>>>      unsigned int hw_type;
>>>      bool xdp_flush;
>>>      union {
>>> @@ -261,7 +260,11 @@ struct ionic_queue {
>>>              struct ionic_rxq_sg_desc *rxq_sgl;
>>>      };
>>>      struct xdp_rxq_info *xdp_rxq_info;
>>> +   struct bpf_prog *xdp_prog;
>>>      struct ionic_queue *partner;
>>> +
>>> +   unsigned int type;
>>> +   unsigned int hw_index;
>>>      dma_addr_t base_pa;
>>>      dma_addr_t cmb_base_pa;
>>>      dma_addr_t sg_base_pa;
>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>>> index aa0cc31dfe6e..0fba2df33915 100644
>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>>> @@ -2700,24 +2700,24 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
>>>
>>>   static int ionic_xdp_queues_config(struct ionic_lif *lif)
>>>   {
>>> +   struct bpf_prog *xdp_prog;
>>>      unsigned int i;
>>>      int err;
>>>
>>>      if (!lif->rxqcqs)
>>>              return 0;
>>>
>>> -   /* There's no need to rework memory if not going to/from NULL program.
>>> -    * If there is no lif->xdp_prog, there should also be no q.xdp_rxq_info
>>> -    * This way we don't need to keep an *xdp_prog in every queue struct.
>>> -    */
>>> -   if (!lif->xdp_prog == !lif->rxqcqs[0]->q.xdp_rxq_info)
>>> +   /* There's no need to rework memory if not going to/from NULL program.  */
>>> +   xdp_prog = READ_ONCE(lif->xdp_prog);
>>> +   if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
>>>              return 0;
> 
> In a case when we replace a non-NULL program with another non-NULL program this
> would create a situation where lif and queues have different pointers on them.

Yeah, you are right. Good catch. We will get this fixed up in the next 
version.

Thanks,

Brett

> 
>>>
>>>      for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
>>>              struct ionic_queue *q = &lif->rxqcqs[i]->q;
>>>
>>> -           if (q->xdp_rxq_info) {
>>> +           if (q->xdp_prog) {
>>>                      ionic_xdp_unregister_rxq_info(q);
>>> +                   q->xdp_prog = NULL;
>>>                      continue;
>>>              }
>>>
>>> @@ -2727,6 +2727,7 @@ static int ionic_xdp_queues_config(struct ionic_lif *lif)
>>>                              i, err);
>>>                      goto err_out;
>>>              }
>>> +           q->xdp_prog = xdp_prog;
>>>      }
>>>
>>>      return 0;
> 
> [...]

Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog

[Brett Creeley]

On 8/27/2024 4:44 AM, Larysa Zaremba wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
>> From: Shannon Nelson <shannon.nelson@amd.com>
>>
>> We originally were using a per-interface xdp_prog variable to track
>> a loaded XDP program since we knew there would never be support for a
>> per-queue XDP program.  With that, we only built the per queue rxq_info
>> struct when an XDP program was loaded and removed it on XDP program unload,
>> and used the pointer as an indicator in the Rx hotpath to know to how build
>> the buffers.  However, that's really not the model generally used, and
>> makes a conversion to page_pool Rx buffer cacheing a little problematic.
>>
>> This patch converts the driver to use the more common approach of using
>> a per-queue xdp_prog pointer to work out buffer allocations and need
>> for bpf_prog_run_xdp().  We jostle a couple of fields in the queue struct
>> in order to keep the new xdp_prog pointer in a warm cacheline.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
> 
> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>
> 
> If you happen to send another version, please include in a commit message a note
> about READ_ONCE() removal. The removal itself is OK, but an indication that this
> was intentional would be nice.

Sure, will do. Thanks for the review.

Brett

<snip>

[PATCH v2 net-next 4/5] ionic: always use rxq_info

[Brett Creeley]

From: Shannon Nelson <shannon.nelson@amd.com>

Instead of setting up and tearing down the rxq_info only when the XDP
program is loaded or unloaded, we will build the rxq_info whether or not
XDP is in use.  This is the more common use pattern and better supports
future conversion to page_pool.  Since the rxq_info wants the napi_id
we re-order things slightly to tie this into the queue init and deinit
functions where we do the add and delete of napi.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 55 +++++++------------
 1 file changed, 19 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 0fba2df33915..4a7763ec061f 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -46,8 +46,9 @@ static int ionic_start_queues(struct ionic_lif *lif);
 static void ionic_stop_queues(struct ionic_lif *lif);
 static void ionic_lif_queue_identify(struct ionic_lif *lif);
 
-static int ionic_xdp_queues_config(struct ionic_lif *lif);
-static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q);
+static void ionic_xdp_queues_config(struct ionic_lif *lif);
+static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id);
+static void ionic_unregister_rxq_info(struct ionic_queue *q);
 
 static void ionic_dim_work(struct work_struct *work)
 {
@@ -380,6 +381,7 @@ static void ionic_lif_qcq_deinit(struct ionic_lif *lif, struct ionic_qcq *qcq)
 	if (!(qcq->flags & IONIC_QCQ_F_INITED))
 		return;
 
+	ionic_unregister_rxq_info(&qcq->q);
 	if (qcq->flags & IONIC_QCQ_F_INTR) {
 		ionic_intr_mask(idev->intr_ctrl, qcq->intr.index,
 				IONIC_INTR_MASK_SET);
@@ -437,9 +439,7 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
 		qcq->sg_base_pa = 0;
 	}
 
-	ionic_xdp_unregister_rxq_info(&qcq->q);
 	ionic_qcq_intr_free(lif, qcq);
-
 	vfree(qcq->q.info);
 	qcq->q.info = NULL;
 }
@@ -925,6 +925,11 @@ static int ionic_lif_rxq_init(struct ionic_lif *lif, struct ionic_qcq *qcq)
 		netif_napi_add(lif->netdev, &qcq->napi, ionic_rx_napi);
 	else
 		netif_napi_add(lif->netdev, &qcq->napi, ionic_txrx_napi);
+	err = ionic_register_rxq_info(q, qcq->napi.napi_id);
+	if (err) {
+		netif_napi_del(&qcq->napi);
+		return err;
+	}
 
 	qcq->flags |= IONIC_QCQ_F_INITED;
 
@@ -2143,9 +2148,7 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
 	int derr = 0;
 	int i, err;
 
-	err = ionic_xdp_queues_config(lif);
-	if (err)
-		return err;
+	ionic_xdp_queues_config(lif);
 
 	for (i = 0; i < lif->nxqs; i++) {
 		if (!(lif->rxqcqs[i] && lif->txqcqs[i])) {
@@ -2192,8 +2195,6 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
 		derr = ionic_qcq_disable(lif, lif->rxqcqs[i], derr);
 	}
 
-	ionic_xdp_queues_config(lif);
-
 	return err;
 }
 
@@ -2651,7 +2652,7 @@ static void ionic_vf_attr_replay(struct ionic_lif *lif)
 	ionic_vf_start(ionic);
 }
 
-static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
+static void ionic_unregister_rxq_info(struct ionic_queue *q)
 {
 	struct xdp_rxq_info *xi;
 
@@ -2665,7 +2666,7 @@ static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
 	kfree(xi);
 }
 
-static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
+static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
 {
 	struct xdp_rxq_info *rxq_info;
 	int err;
@@ -2698,45 +2699,27 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
 	return err;
 }
 
-static int ionic_xdp_queues_config(struct ionic_lif *lif)
+static void ionic_xdp_queues_config(struct ionic_lif *lif)
 {
 	struct bpf_prog *xdp_prog;
 	unsigned int i;
-	int err;
 
 	if (!lif->rxqcqs)
-		return 0;
+		return;
 
-	/* There's no need to rework memory if not going to/from NULL program.  */
+	/* Nothing to do if not going to/from NULL program.  */
 	xdp_prog = READ_ONCE(lif->xdp_prog);
 	if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
-		return 0;
+		return;
 
 	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
 		struct ionic_queue *q = &lif->rxqcqs[i]->q;
 
-		if (q->xdp_prog) {
-			ionic_xdp_unregister_rxq_info(q);
+		if (q->xdp_prog)
 			q->xdp_prog = NULL;
-			continue;
-		}
-
-		err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
-		if (err) {
-			dev_err(lif->ionic->dev, "failed to register RX queue %d info for XDP, err %d\n",
-				i, err);
-			goto err_out;
-		}
-		q->xdp_prog = xdp_prog;
+		else
+			q->xdp_prog = xdp_prog;
 	}
-
-	return 0;
-
-err_out:
-	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++)
-		ionic_xdp_unregister_rxq_info(&lif->rxqcqs[i]->q);
-
-	return err;
 }
 
 static int ionic_xdp_config(struct net_device *netdev, struct netdev_bpf *bpf)
-- 
2.17.1

Re: [PATCH v2 net-next 4/5] ionic: always use rxq_info

[Larysa Zaremba]

On Mon, Aug 26, 2024 at 11:44:21AM -0700, Brett Creeley wrote:
> From: Shannon Nelson <shannon.nelson@amd.com>
> 
> Instead of setting up and tearing down the rxq_info only when the XDP
> program is loaded or unloaded, we will build the rxq_info whether or not
> XDP is in use.  This is the more common use pattern and better supports
> future conversion to page_pool.  Since the rxq_info wants the napi_id
> we re-order things slightly to tie this into the queue init and deinit
> functions where we do the add and delete of napi.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
> ---
>  .../net/ethernet/pensando/ionic/ionic_lif.c   | 55 +++++++------------
>  1 file changed, 19 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 0fba2df33915..4a7763ec061f 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -46,8 +46,9 @@ static int ionic_start_queues(struct ionic_lif *lif);
>  static void ionic_stop_queues(struct ionic_lif *lif);
>  static void ionic_lif_queue_identify(struct ionic_lif *lif);
>  
> -static int ionic_xdp_queues_config(struct ionic_lif *lif);
> -static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q);
> +static void ionic_xdp_queues_config(struct ionic_lif *lif);
> +static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id);
> +static void ionic_unregister_rxq_info(struct ionic_queue *q);
>  
>  static void ionic_dim_work(struct work_struct *work)
>  {
> @@ -380,6 +381,7 @@ static void ionic_lif_qcq_deinit(struct ionic_lif *lif, struct ionic_qcq *qcq)
>  	if (!(qcq->flags & IONIC_QCQ_F_INITED))
>  		return;
>  
> +	ionic_unregister_rxq_info(&qcq->q);
>  	if (qcq->flags & IONIC_QCQ_F_INTR) {
>  		ionic_intr_mask(idev->intr_ctrl, qcq->intr.index,
>  				IONIC_INTR_MASK_SET);
> @@ -437,9 +439,7 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
>  		qcq->sg_base_pa = 0;
>  	}
>  
> -	ionic_xdp_unregister_rxq_info(&qcq->q);
>  	ionic_qcq_intr_free(lif, qcq);
> -
>  	vfree(qcq->q.info);
>  	qcq->q.info = NULL;
>  }
> @@ -925,6 +925,11 @@ static int ionic_lif_rxq_init(struct ionic_lif *lif, struct ionic_qcq *qcq)
>  		netif_napi_add(lif->netdev, &qcq->napi, ionic_rx_napi);
>  	else
>  		netif_napi_add(lif->netdev, &qcq->napi, ionic_txrx_napi);
> +	err = ionic_register_rxq_info(q, qcq->napi.napi_id);
> +	if (err) {
> +		netif_napi_del(&qcq->napi);
> +		return err;
> +	}
>  
>  	qcq->flags |= IONIC_QCQ_F_INITED;
>  
> @@ -2143,9 +2148,7 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
>  	int derr = 0;
>  	int i, err;
>  
> -	err = ionic_xdp_queues_config(lif);
> -	if (err)
> -		return err;
> +	ionic_xdp_queues_config(lif);
>  
>  	for (i = 0; i < lif->nxqs; i++) {
>  		if (!(lif->rxqcqs[i] && lif->txqcqs[i])) {
> @@ -2192,8 +2195,6 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
>  		derr = ionic_qcq_disable(lif, lif->rxqcqs[i], derr);
>  	}
>  
> -	ionic_xdp_queues_config(lif);
> -
>  	return err;
>  }
>  
> @@ -2651,7 +2652,7 @@ static void ionic_vf_attr_replay(struct ionic_lif *lif)
>  	ionic_vf_start(ionic);
>  }
>  
> -static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
> +static void ionic_unregister_rxq_info(struct ionic_queue *q)
>  {
>  	struct xdp_rxq_info *xi;
>  
> @@ -2665,7 +2666,7 @@ static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
>  	kfree(xi);
>  }
>  
> -static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
> +static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
>  {
>  	struct xdp_rxq_info *rxq_info;
>  	int err;
> @@ -2698,45 +2699,27 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
>  	return err;
>  }
>  
> -static int ionic_xdp_queues_config(struct ionic_lif *lif)
> +static void ionic_xdp_queues_config(struct ionic_lif *lif)

I think this function should also get a new name that would reflect the fact 
that it is just updating the XDP prog on rings. Also, ionic_xdp_queues_config 
sounds a little bit like configuring XDP_TX/xdp_xmit queues, but here we have rx 
queues only.

>  {
>  	struct bpf_prog *xdp_prog;
>  	unsigned int i;
> -	int err;
>  
>  	if (!lif->rxqcqs)
> -		return 0;
> +		return;
>  
> -	/* There's no need to rework memory if not going to/from NULL program.  */
> +	/* Nothing to do if not going to/from NULL program.  */
>  	xdp_prog = READ_ONCE(lif->xdp_prog);
>  	if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
> -		return 0;
> +		return;
>  
>  	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
>  		struct ionic_queue *q = &lif->rxqcqs[i]->q;
>  
> -		if (q->xdp_prog) {
> -			ionic_xdp_unregister_rxq_info(q);
> +		if (q->xdp_prog)
>  			q->xdp_prog = NULL;
> -			continue;
> -		}
> -
> -		err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
> -		if (err) {
> -			dev_err(lif->ionic->dev, "failed to register RX queue %d info for XDP, err %d\n",
> -				i, err);
> -			goto err_out;
> -		}
> -		q->xdp_prog = xdp_prog;
> +		else
> +			q->xdp_prog = xdp_prog;
>  	}
> -
> -	return 0;
> -
> -err_out:
> -	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++)
> -		ionic_xdp_unregister_rxq_info(&lif->rxqcqs[i]->q);
> -
> -	return err;
>  }
>  
>  static int ionic_xdp_config(struct net_device *netdev, struct netdev_bpf *bpf)
> -- 
> 2.17.1
> 
> 

Re: [PATCH v2 net-next 4/5] ionic: always use rxq_info

[Brett Creeley]

On 8/27/2024 5:08 AM, Larysa Zaremba wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Mon, Aug 26, 2024 at 11:44:21AM -0700, Brett Creeley wrote:
>> From: Shannon Nelson <shannon.nelson@amd.com>
>>
>> Instead of setting up and tearing down the rxq_info only when the XDP
>> program is loaded or unloaded, we will build the rxq_info whether or not
>> XDP is in use.  This is the more common use pattern and better supports
>> future conversion to page_pool.  Since the rxq_info wants the napi_id
>> we re-order things slightly to tie this into the queue init and deinit
>> functions where we do the add and delete of napi.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
>> ---
>>   .../net/ethernet/pensando/ionic/ionic_lif.c   | 55 +++++++------------
>>   1 file changed, 19 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c

<snip>

>> -static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
>> +static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
>>   {
>>        struct xdp_rxq_info *rxq_info;
>>        int err;
>> @@ -2698,45 +2699,27 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
>>        return err;
>>   }
>>
>> -static int ionic_xdp_queues_config(struct ionic_lif *lif)
>> +static void ionic_xdp_queues_config(struct ionic_lif *lif)
> 
> I think this function should also get a new name that would reflect the fact
> that it is just updating the XDP prog on rings. Also, ionic_xdp_queues_config
> sounds a little bit like configuring XDP_TX/xdp_xmit queues, but here we have rx
> queues only.

Yeah that makes sense. We can rename it to something closer to the 
function's contents/purpose.

Thanks,

Brett
> 
>>   {
>>        struct bpf_prog *xdp_prog;
>>        unsigned int i;
>> -     int err;
>>
>>        if (!lif->rxqcqs)
>> -             return 0;
>> +             return;
>>
>> -     /* There's no need to rework memory if not going to/from NULL program.  */
>> +     /* Nothing to do if not going to/from NULL program.  */
>>        xdp_prog = READ_ONCE(lif->xdp_prog);
>>        if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
>> -             return 0;
>> +             return;
>>
>>        for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
>>                struct ionic_queue *q = &lif->rxqcqs[i]->q;
>>
>> -             if (q->xdp_prog) {
>> -                     ionic_xdp_unregister_rxq_info(q);
>> +             if (q->xdp_prog)
>>                        q->xdp_prog = NULL;
>> -                     continue;
>> -             }
>> -
>> -             err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
>> -             if (err) {
>> -                     dev_err(lif->ionic->dev, "failed to register RX queue %d info for XDP, err %d\n",
>> -                             i, err);
>> -                     goto err_out;
>> -             }
>> -             q->xdp_prog = xdp_prog;
>> +             else
>> +                     q->xdp_prog = xdp_prog;
>>        }
>> -
>> -     return 0;
>> -
>> -err_out:
>> -     for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++)
>> -             ionic_xdp_unregister_rxq_info(&lif->rxqcqs[i]->q);
>> -
>> -     return err;
>>   }
>>
>>   static int ionic_xdp_config(struct net_device *netdev, struct netdev_bpf *bpf)
>> --
>> 2.17.1
>>
>>

[PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool

[Brett Creeley]

From: Shannon Nelson <shannon.nelson@amd.com>

Our home-grown buffer management needs to go away and we need
to be playing nicely with the page_pool infrastructure.  This
converts the Rx traffic queues to use page_pool.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/Kconfig         |   1 +
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  14 +-
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  58 ++-
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 344 +++++++++---------
 4 files changed, 221 insertions(+), 196 deletions(-)

diff --git a/drivers/net/ethernet/pensando/Kconfig b/drivers/net/ethernet/pensando/Kconfig
index 3f7519e435b8..01fe76786f77 100644
--- a/drivers/net/ethernet/pensando/Kconfig
+++ b/drivers/net/ethernet/pensando/Kconfig
@@ -23,6 +23,7 @@ config IONIC
 	depends on PTP_1588_CLOCK_OPTIONAL
 	select NET_DEVLINK
 	select DIMLIB
+	select PAGE_POOL
 	help
 	  This enables the support for the Pensando family of Ethernet
 	  adapters.  More specific information on this driver can be
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 19ae68a86a0b..fc93c5c4dbe6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -182,9 +182,6 @@ struct ionic_qcq;
 
 #define IONIC_MAX_BUF_LEN			((u16)-1)
 #define IONIC_PAGE_SIZE				PAGE_SIZE
-#define IONIC_PAGE_SPLIT_SZ			(PAGE_SIZE / 2)
-#define IONIC_PAGE_GFP_MASK			(GFP_ATOMIC | __GFP_NOWARN |\
-						 __GFP_COMP | __GFP_MEMALLOC)
 
 #define IONIC_XDP_MAX_LINEAR_MTU	(IONIC_PAGE_SIZE -	\
 					 (VLAN_ETH_HLEN +	\
@@ -248,11 +245,6 @@ struct ionic_queue {
 		struct ionic_rxq_desc *rxq;
 		struct ionic_admin_cmd *adminq;
 	};
-	union {
-		void __iomem *cmb_base;
-		struct ionic_txq_desc __iomem *cmb_txq;
-		struct ionic_rxq_desc __iomem *cmb_rxq;
-	};
 	union {
 		void *sg_base;
 		struct ionic_txq_sg_desc *txq_sgl;
@@ -261,8 +253,14 @@ struct ionic_queue {
 	};
 	struct xdp_rxq_info *xdp_rxq_info;
 	struct bpf_prog *xdp_prog;
+	struct page_pool *page_pool;
 	struct ionic_queue *partner;
 
+	union {
+		void __iomem *cmb_base;
+		struct ionic_txq_desc __iomem *cmb_txq;
+		struct ionic_rxq_desc __iomem *cmb_rxq;
+	};
 	unsigned int type;
 	unsigned int hw_index;
 	dma_addr_t base_pa;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 4a7763ec061f..879f2e25ec31 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -13,6 +13,7 @@
 #include <linux/cpumask.h>
 #include <linux/crash_dump.h>
 #include <linux/vmalloc.h>
+#include <net/page_pool/helpers.h>
 
 #include "ionic.h"
 #include "ionic_bus.h"
@@ -439,6 +440,9 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
 		qcq->sg_base_pa = 0;
 	}
 
+	page_pool_destroy(qcq->q.page_pool);
+	qcq->q.page_pool = NULL;
+
 	ionic_qcq_intr_free(lif, qcq);
 	vfree(qcq->q.info);
 	qcq->q.info = NULL;
@@ -579,6 +583,33 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
 		goto err_out_free_qcq;
 	}
 
+	if (type == IONIC_QTYPE_RXQ) {
+		struct page_pool_params pp_params = {
+			.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
+			.order = 0,
+			.pool_size = num_descs,
+			.nid = NUMA_NO_NODE,
+			.dev = lif->ionic->dev,
+			.napi = &new->napi,
+			.dma_dir = DMA_FROM_DEVICE,
+			.max_len = PAGE_SIZE,
+			.netdev = lif->netdev,
+		};
+		struct bpf_prog *xdp_prog;
+
+		xdp_prog = READ_ONCE(lif->xdp_prog);
+		if (xdp_prog)
+			pp_params.dma_dir = DMA_BIDIRECTIONAL;
+
+		new->q.page_pool = page_pool_create(&pp_params);
+		if (IS_ERR(new->q.page_pool)) {
+			netdev_err(lif->netdev, "Cannot create page_pool\n");
+			err = PTR_ERR(new->q.page_pool);
+			new->q.page_pool = NULL;
+			goto err_out_free_q_info;
+		}
+	}
+
 	new->q.type = type;
 	new->q.max_sg_elems = lif->qtype_info[type].max_sg_elems;
 
@@ -586,12 +617,12 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
 			   desc_size, sg_desc_size, pid);
 	if (err) {
 		netdev_err(lif->netdev, "Cannot initialize queue\n");
-		goto err_out_free_q_info;
+		goto err_out_free_page_pool;
 	}
 
 	err = ionic_alloc_qcq_interrupt(lif, new);
 	if (err)
-		goto err_out_free_q_info;
+		goto err_out_free_page_pool;
 
 	err = ionic_cq_init(lif, &new->cq, &new->intr, num_descs, cq_desc_size);
 	if (err) {
@@ -712,6 +743,8 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
 		devm_free_irq(dev, new->intr.vector, &new->napi);
 		ionic_intr_free(lif->ionic, new->intr.index);
 	}
+err_out_free_page_pool:
+	page_pool_destroy(new->q.page_pool);
 err_out_free_q_info:
 	vfree(new->q.info);
 err_out_free_qcq:
@@ -2677,15 +2710,15 @@ static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
 
 	err = xdp_rxq_info_reg(rxq_info, q->lif->netdev, q->index, napi_id);
 	if (err) {
-		dev_err(q->dev, "Queue %d xdp_rxq_info_reg failed, err %d\n",
-			q->index, err);
+		netdev_err(q->lif->netdev, "q%d xdp_rxq_info_reg failed, err %d\n",
+			   q->index, err);
 		goto err_out;
 	}
 
-	err = xdp_rxq_info_reg_mem_model(rxq_info, MEM_TYPE_PAGE_ORDER0, NULL);
+	err = xdp_rxq_info_reg_mem_model(rxq_info, MEM_TYPE_PAGE_POOL, q->page_pool);
 	if (err) {
-		dev_err(q->dev, "Queue %d xdp_rxq_info_reg_mem_model failed, err %d\n",
-			q->index, err);
+		netdev_err(q->lif->netdev, "q%d xdp_rxq_info_reg_mem_model failed, err %d\n",
+			   q->index, err);
 		xdp_rxq_info_unreg(rxq_info);
 		goto err_out;
 	}
@@ -2855,7 +2888,16 @@ static int ionic_cmb_reconfig(struct ionic_lif *lif,
 
 static void ionic_swap_queues(struct ionic_qcq *a, struct ionic_qcq *b)
 {
-	/* only swapping the queues, not the napi, flags, or other stuff */
+	/* only swapping the queues and napi, not flags or other stuff */
+	swap(a->napi,         b->napi);
+
+	if (a->q.type == IONIC_QTYPE_RXQ) {
+		swap(a->q.page_pool, b->q.page_pool);
+		a->q.page_pool->p.napi = &a->napi;
+		if (b->q.page_pool)  /* is NULL when increasing queue count */
+			b->q.page_pool->p.napi = &b->napi;
+	}
+
 	swap(a->q.features,   b->q.features);
 	swap(a->q.num_descs,  b->q.num_descs);
 	swap(a->q.desc_size,  b->q.desc_size);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 858ab4fd9218..35e3751dd5a7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -6,6 +6,7 @@
 #include <linux/if_vlan.h>
 #include <net/ip6_checksum.h>
 #include <net/netdev_queues.h>
+#include <net/page_pool/helpers.h>
 
 #include "ionic.h"
 #include "ionic_lif.h"
@@ -118,108 +119,57 @@ static void *ionic_rx_buf_va(struct ionic_buf_info *buf_info)
 
 static dma_addr_t ionic_rx_buf_pa(struct ionic_buf_info *buf_info)
 {
-	return buf_info->dma_addr + buf_info->page_offset;
+	return page_pool_get_dma_addr(buf_info->page) + buf_info->page_offset;
 }
 
-static unsigned int ionic_rx_buf_size(struct ionic_buf_info *buf_info)
+static void __ionic_rx_put_buf(struct ionic_queue *q,
+			       struct ionic_buf_info *buf_info,
+			       bool recycle_direct)
 {
-	return min_t(u32, IONIC_MAX_BUF_LEN, IONIC_PAGE_SIZE - buf_info->page_offset);
-}
-
-static int ionic_rx_page_alloc(struct ionic_queue *q,
-			       struct ionic_buf_info *buf_info)
-{
-	struct device *dev = q->dev;
-	dma_addr_t dma_addr;
-	struct page *page;
-
-	page = alloc_pages(IONIC_PAGE_GFP_MASK, 0);
-	if (unlikely(!page)) {
-		net_err_ratelimited("%s: %s page alloc failed\n",
-				    dev_name(dev), q->name);
-		q_to_rx_stats(q)->alloc_err++;
-		return -ENOMEM;
-	}
-
-	dma_addr = dma_map_page(dev, page, 0,
-				IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
-	if (unlikely(dma_mapping_error(dev, dma_addr))) {
-		__free_pages(page, 0);
-		net_err_ratelimited("%s: %s dma map failed\n",
-				    dev_name(dev), q->name);
-		q_to_rx_stats(q)->dma_map_err++;
-		return -EIO;
-	}
-
-	buf_info->dma_addr = dma_addr;
-	buf_info->page = page;
-	buf_info->page_offset = 0;
-
-	return 0;
-}
-
-static void ionic_rx_page_free(struct ionic_queue *q,
-			       struct ionic_buf_info *buf_info)
-{
-	struct device *dev = q->dev;
-
-	if (unlikely(!buf_info)) {
-		net_err_ratelimited("%s: %s invalid buf_info in free\n",
-				    dev_name(dev), q->name);
-		return;
-	}
-
 	if (!buf_info->page)
 		return;
 
-	dma_unmap_page(dev, buf_info->dma_addr, IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
-	__free_pages(buf_info->page, 0);
+	page_pool_put_full_page(q->page_pool, buf_info->page, recycle_direct);
 	buf_info->page = NULL;
+	buf_info->len = 0;
+	buf_info->page_offset = 0;
 }
 
-static bool ionic_rx_buf_recycle(struct ionic_queue *q,
-				 struct ionic_buf_info *buf_info, u32 len)
-{
-	u32 size;
-
-	/* don't re-use pages allocated in low-mem condition */
-	if (page_is_pfmemalloc(buf_info->page))
-		return false;
-
-	/* don't re-use buffers from non-local numa nodes */
-	if (page_to_nid(buf_info->page) != numa_mem_id())
-		return false;
-
-	size = ALIGN(len, q->xdp_prog ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
-	buf_info->page_offset += size;
-	if (buf_info->page_offset >= IONIC_PAGE_SIZE)
-		return false;
 
-	get_page(buf_info->page);
+static void ionic_rx_put_buf(struct ionic_queue *q,
+			     struct ionic_buf_info *buf_info)
+{
+	__ionic_rx_put_buf(q, buf_info, false);
+}
 
-	return true;
+static void ionic_rx_put_buf_direct(struct ionic_queue *q,
+				    struct ionic_buf_info *buf_info)
+{
+	__ionic_rx_put_buf(q, buf_info, true);
 }
 
 static void ionic_rx_add_skb_frag(struct ionic_queue *q,
 				  struct sk_buff *skb,
 				  struct ionic_buf_info *buf_info,
-				  u32 off, u32 len,
+				  u32 headroom, u32 len,
 				  bool synced)
 {
 	if (!synced)
-		dma_sync_single_range_for_cpu(q->dev, ionic_rx_buf_pa(buf_info),
-					      off, len, DMA_FROM_DEVICE);
+		page_pool_dma_sync_for_cpu(q->page_pool,
+					   buf_info->page,
+					   buf_info->page_offset + headroom,
+					   len);
 
 	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
-			buf_info->page, buf_info->page_offset + off,
-			len,
-			IONIC_PAGE_SIZE);
+			buf_info->page, buf_info->page_offset + headroom,
+			len, buf_info->len);
 
-	if (!ionic_rx_buf_recycle(q, buf_info, len)) {
-		dma_unmap_page(q->dev, buf_info->dma_addr,
-			       IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
-		buf_info->page = NULL;
-	}
+	/* napi_gro_frags() will release/recycle the
+	 * page_pool buffers from the frags list
+	 */
+	buf_info->page = NULL;
+	buf_info->len = 0;
+	buf_info->page_offset = 0;
 }
 
 static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
@@ -244,12 +194,13 @@ static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
 		q_to_rx_stats(q)->alloc_err++;
 		return NULL;
 	}
+	skb_mark_for_recycle(skb);
 
 	if (headroom)
 		frag_len = min_t(u16, len,
 				 IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN);
 	else
-		frag_len = min_t(u16, len, ionic_rx_buf_size(buf_info));
+		frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
 
 	if (unlikely(!buf_info->page))
 		goto err_bad_buf_page;
@@ -260,7 +211,7 @@ static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
 	for (i = 0; i < num_sg_elems; i++, buf_info++) {
 		if (unlikely(!buf_info->page))
 			goto err_bad_buf_page;
-		frag_len = min_t(u16, len, ionic_rx_buf_size(buf_info));
+		frag_len = min_t(u16, len, buf_info->len);
 		ionic_rx_add_skb_frag(q, skb, buf_info, 0, frag_len, synced);
 		len -= frag_len;
 	}
@@ -277,11 +228,13 @@ static struct sk_buff *ionic_rx_copybreak(struct net_device *netdev,
 					  struct ionic_rx_desc_info *desc_info,
 					  unsigned int headroom,
 					  unsigned int len,
+					  unsigned int num_sg_elems,
 					  bool synced)
 {
 	struct ionic_buf_info *buf_info;
 	struct device *dev = q->dev;
 	struct sk_buff *skb;
+	int i;
 
 	buf_info = &desc_info->bufs[0];
 
@@ -292,54 +245,52 @@ static struct sk_buff *ionic_rx_copybreak(struct net_device *netdev,
 		q_to_rx_stats(q)->alloc_err++;
 		return NULL;
 	}
-
-	if (unlikely(!buf_info->page)) {
-		dev_kfree_skb(skb);
-		return NULL;
-	}
+	skb_mark_for_recycle(skb);
 
 	if (!synced)
-		dma_sync_single_range_for_cpu(dev, ionic_rx_buf_pa(buf_info),
-					      headroom, len, DMA_FROM_DEVICE);
+		page_pool_dma_sync_for_cpu(q->page_pool,
+					   buf_info->page,
+					   buf_info->page_offset + headroom,
+					   len);
+
 	skb_copy_to_linear_data(skb, ionic_rx_buf_va(buf_info) + headroom, len);
-	dma_sync_single_range_for_device(dev, ionic_rx_buf_pa(buf_info),
-					 headroom, len, DMA_FROM_DEVICE);
 
 	skb_put(skb, len);
 	skb->protocol = eth_type_trans(skb, netdev);
 
+	/* recycle the Rx buffer now that we're done with it */
+	ionic_rx_put_buf_direct(q, buf_info);
+	buf_info++;
+	for (i = 0; i < num_sg_elems; i++, buf_info++)
+		ionic_rx_put_buf_direct(q, buf_info);
+
 	return skb;
 }
 
 static void ionic_xdp_tx_desc_clean(struct ionic_queue *q,
-				    struct ionic_tx_desc_info *desc_info)
+				    struct ionic_tx_desc_info *desc_info,
+				    bool in_napi)
 {
-	unsigned int nbufs = desc_info->nbufs;
-	struct ionic_buf_info *buf_info;
-	struct device *dev = q->dev;
-	int i;
+	struct xdp_frame_bulk bq;
 
-	if (!nbufs)
+	if (!desc_info->nbufs)
 		return;
 
-	buf_info = desc_info->bufs;
-	dma_unmap_single(dev, buf_info->dma_addr,
-			 buf_info->len, DMA_TO_DEVICE);
-	if (desc_info->act == XDP_TX)
-		__free_pages(buf_info->page, 0);
-	buf_info->page = NULL;
+	xdp_frame_bulk_init(&bq);
+	rcu_read_lock(); /* need for xdp_return_frame_bulk */
 
-	buf_info++;
-	for (i = 1; i < nbufs + 1 && buf_info->page; i++, buf_info++) {
-		dma_unmap_page(dev, buf_info->dma_addr,
-			       buf_info->len, DMA_TO_DEVICE);
-		if (desc_info->act == XDP_TX)
-			__free_pages(buf_info->page, 0);
-		buf_info->page = NULL;
+	if (desc_info->act == XDP_TX) {
+		if (likely(in_napi))
+			xdp_return_frame_rx_napi(desc_info->xdpf);
+		else
+			xdp_return_frame(desc_info->xdpf);
+	} else if (desc_info->act == XDP_REDIRECT) {
+		ionic_tx_desc_unmap_bufs(q, desc_info);
+		xdp_return_frame_bulk(desc_info->xdpf, &bq);
 	}
 
-	if (desc_info->act == XDP_REDIRECT)
-		xdp_return_frame(desc_info->xdpf);
+	xdp_flush_frame_bulk(&bq);
+	rcu_read_unlock();
 
 	desc_info->nbufs = 0;
 	desc_info->xdpf = NULL;
@@ -363,9 +314,17 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame,
 	buf_info = desc_info->bufs;
 	stats = q_to_tx_stats(q);
 
-	dma_addr = ionic_tx_map_single(q, frame->data, len);
-	if (!dma_addr)
-		return -EIO;
+	if (act == XDP_TX) {
+		dma_addr = page_pool_get_dma_addr(page) +
+			   off + XDP_PACKET_HEADROOM;
+		dma_sync_single_for_device(q->dev, dma_addr,
+					   len, DMA_TO_DEVICE);
+	} else /* XDP_REDIRECT */ {
+		dma_addr = ionic_tx_map_single(q, frame->data, len);
+		if (!dma_addr)
+			return -EIO;
+	}
+
 	buf_info->dma_addr = dma_addr;
 	buf_info->len = len;
 	buf_info->page = page;
@@ -387,10 +346,21 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame,
 		frag = sinfo->frags;
 		elem = ionic_tx_sg_elems(q);
 		for (i = 0; i < sinfo->nr_frags; i++, frag++, bi++) {
-			dma_addr = ionic_tx_map_frag(q, frag, 0, skb_frag_size(frag));
-			if (!dma_addr) {
-				ionic_tx_desc_unmap_bufs(q, desc_info);
-				return -EIO;
+			if (act == XDP_TX) {
+				struct page *pg = skb_frag_page(frag);
+
+				dma_addr = page_pool_get_dma_addr(pg) +
+					   skb_frag_off(frag);
+				dma_sync_single_for_device(q->dev, dma_addr,
+							   skb_frag_size(frag),
+							   DMA_TO_DEVICE);
+			} else {
+				dma_addr = ionic_tx_map_frag(q, frag, 0,
+							     skb_frag_size(frag));
+				if (dma_mapping_error(q->dev, dma_addr)) {
+					ionic_tx_desc_unmap_bufs(q, desc_info);
+					return -EIO;
+				}
 			}
 			bi->dma_addr = dma_addr;
 			bi->len = skb_frag_size(frag);
@@ -488,8 +458,6 @@ static void ionic_xdp_rx_unlink_bufs(struct ionic_queue *q,
 	int i;
 
 	for (i = 0; i < nbufs; i++) {
-		dma_unmap_page(q->dev, buf_info->dma_addr,
-			       IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
 		buf_info->page = NULL;
 		buf_info++;
 	}
@@ -516,11 +484,9 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 	frag_len = min_t(u16, len, IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN);
 	xdp_prepare_buff(&xdp_buf, ionic_rx_buf_va(buf_info),
 			 XDP_PACKET_HEADROOM, frag_len, false);
-
-	dma_sync_single_range_for_cpu(rxq->dev, ionic_rx_buf_pa(buf_info),
-				      XDP_PACKET_HEADROOM, frag_len,
-				      DMA_FROM_DEVICE);
-
+	page_pool_dma_sync_for_cpu(rxq->page_pool, buf_info->page,
+				   buf_info->page_offset + XDP_PACKET_HEADROOM,
+				   frag_len);
 	prefetchw(&xdp_buf.data_hard_start);
 
 	/*  We limit MTU size to one buffer if !xdp_has_frags, so
@@ -542,15 +508,16 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 		do {
 			if (unlikely(sinfo->nr_frags >= MAX_SKB_FRAGS)) {
 				err = -ENOSPC;
-				goto out_xdp_abort;
+				break;
 			}
 
 			frag = &sinfo->frags[sinfo->nr_frags];
 			sinfo->nr_frags++;
 			bi++;
-			frag_len = min_t(u16, remain_len, ionic_rx_buf_size(bi));
-			dma_sync_single_range_for_cpu(rxq->dev, ionic_rx_buf_pa(bi),
-						      0, frag_len, DMA_FROM_DEVICE);
+			frag_len = min_t(u16, remain_len, bi->len);
+			page_pool_dma_sync_for_cpu(rxq->page_pool, bi->page,
+						   buf_info->page_offset,
+						   frag_len);
 			skb_frag_fill_page_desc(frag, bi->page, 0, frag_len);
 			sinfo->xdp_frags_size += frag_len;
 			remain_len -= frag_len;
@@ -569,14 +536,16 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 		return false;  /* false = we didn't consume the packet */
 
 	case XDP_DROP:
-		ionic_rx_page_free(rxq, buf_info);
+		ionic_rx_put_buf_direct(rxq, buf_info);
 		stats->xdp_drop++;
 		break;
 
 	case XDP_TX:
 		xdpf = xdp_convert_buff_to_frame(&xdp_buf);
-		if (!xdpf)
-			goto out_xdp_abort;
+		if (!xdpf) {
+			err = -ENOSPC;
+			break;
+		}
 
 		txq = rxq->partner;
 		nq = netdev_get_tx_queue(netdev, txq->index);
@@ -588,7 +557,8 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 					  ionic_q_space_avail(txq),
 					  1, 1)) {
 			__netif_tx_unlock(nq);
-			goto out_xdp_abort;
+			err = -EIO;
+			break;
 		}
 
 		err = ionic_xdp_post_frame(txq, xdpf, XDP_TX,
@@ -598,19 +568,17 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 		__netif_tx_unlock(nq);
 		if (unlikely(err)) {
 			netdev_dbg(netdev, "tx ionic_xdp_post_frame err %d\n", err);
-			goto out_xdp_abort;
+			break;
 		}
 		ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
 		stats->xdp_tx++;
-
-		/* the Tx completion will free the buffers */
 		break;
 
 	case XDP_REDIRECT:
 		err = xdp_do_redirect(netdev, &xdp_buf, xdp_prog);
 		if (unlikely(err)) {
 			netdev_dbg(netdev, "xdp_do_redirect err %d\n", err);
-			goto out_xdp_abort;
+			break;
 		}
 		ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
 		rxq->xdp_flush = true;
@@ -619,15 +587,15 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
 
 	case XDP_ABORTED:
 	default:
-		goto out_xdp_abort;
+		err = -EIO;
+		break;
 	}
 
-	return true;
-
-out_xdp_abort:
-	trace_xdp_exception(netdev, xdp_prog, xdp_action);
-	ionic_rx_page_free(rxq, buf_info);
-	stats->xdp_aborted++;
+	if (err) {
+		ionic_rx_put_buf_direct(rxq, buf_info);
+		trace_xdp_exception(netdev, xdp_prog, xdp_action);
+		stats->xdp_aborted++;
+	}
 
 	return true;
 }
@@ -673,7 +641,8 @@ static void ionic_rx_clean(struct ionic_queue *q,
 	use_copybreak = len <= q->lif->rx_copybreak;
 	if (use_copybreak)
 		skb = ionic_rx_copybreak(netdev, q, desc_info,
-					 headroom, len, synced);
+					 headroom, len,
+					 comp->num_sg_elems, synced);
 	else
 		skb = ionic_rx_build_skb(q, desc_info, headroom, len,
 					 comp->num_sg_elems, synced);
@@ -794,6 +763,9 @@ void ionic_rx_fill(struct ionic_queue *q)
 	struct ionic_buf_info *buf_info;
 	unsigned int fill_threshold;
 	struct ionic_rxq_desc *desc;
+	unsigned int first_frag_len;
+	unsigned int first_buf_len;
+	unsigned int headroom = 0;
 	unsigned int remain_len;
 	unsigned int frag_len;
 	unsigned int nfrags;
@@ -811,35 +783,42 @@ void ionic_rx_fill(struct ionic_queue *q)
 
 	len = netdev->mtu + VLAN_ETH_HLEN;
 
-	for (i = n_fill; i; i--) {
-		unsigned int headroom = 0;
-		unsigned int buf_len;
+	if (q->xdp_prog) {
+		/* Always alloc the full size buffer, but only need
+		 * the actual frag_len in the descriptor
+		 * XDP uses space in the first buffer, so account for
+		 * head room, tail room, and ip header in the first frag size.
+		 */
+		headroom = XDP_PACKET_HEADROOM;
+		first_buf_len = IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN + headroom;
+		first_frag_len = min_t(u16, len + headroom, first_buf_len);
+	} else {
+		/* Use MTU size if smaller than max buffer size */
+		first_frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
+		first_buf_len = first_frag_len;
+	}
 
+	for (i = n_fill; i; i--) {
+		/* fill main descriptor - buf[0] */
 		nfrags = 0;
 		remain_len = len;
 		desc = &q->rxq[q->head_idx];
 		desc_info = &q->rx_info[q->head_idx];
 		buf_info = &desc_info->bufs[0];
 
-		if (!buf_info->page) { /* alloc a new buffer? */
-			if (unlikely(ionic_rx_page_alloc(q, buf_info))) {
-				desc->addr = 0;
-				desc->len = 0;
-				return;
-			}
-		}
-
-		/* fill main descriptor - buf[0]
-		 * XDP uses space in the first buffer, so account for
-		 * head room, tail room, and ip header in the first frag size.
-		 */
-		if (q->xdp_prog) {
-			buf_len = IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN;
-			headroom = XDP_PACKET_HEADROOM;
-		} else {
-			buf_len = ionic_rx_buf_size(buf_info);
+		buf_info->len = first_buf_len;
+		frag_len = first_frag_len - headroom;
+
+		/* get a new buffer if we can't reuse one */
+		if (!buf_info->page)
+			buf_info->page = page_pool_alloc(q->page_pool,
+							 &buf_info->page_offset,
+							 &buf_info->len,
+							 GFP_ATOMIC);
+		if (unlikely(!buf_info->page)) {
+			buf_info->len = 0;
+			return;
 		}
-		frag_len = min_t(u16, len, buf_len);
 
 		desc->addr = cpu_to_le64(ionic_rx_buf_pa(buf_info) + headroom);
 		desc->len = cpu_to_le16(frag_len);
@@ -850,16 +829,26 @@ void ionic_rx_fill(struct ionic_queue *q)
 		/* fill sg descriptors - buf[1..n] */
 		sg_elem = q->rxq_sgl[q->head_idx].elems;
 		for (j = 0; remain_len > 0 && j < q->max_sg_elems; j++, sg_elem++) {
-			if (!buf_info->page) { /* alloc a new sg buffer? */
-				if (unlikely(ionic_rx_page_alloc(q, buf_info))) {
-					sg_elem->addr = 0;
-					sg_elem->len = 0;
+			frag_len = min_t(u16, remain_len, IONIC_PAGE_SIZE);
+
+			/* Recycle any leftover buffers that are too small to reuse */
+			if (unlikely(buf_info->page && buf_info->len < frag_len))
+				ionic_rx_put_buf_direct(q, buf_info);
+
+			/* Get new buffer if needed */
+			if (!buf_info->page) {
+				buf_info->len = frag_len;
+				buf_info->page = page_pool_alloc(q->page_pool,
+								 &buf_info->page_offset,
+								 &buf_info->len,
+								 GFP_ATOMIC);
+				if (unlikely(!buf_info->page)) {
+					buf_info->len = 0;
 					return;
 				}
 			}
 
 			sg_elem->addr = cpu_to_le64(ionic_rx_buf_pa(buf_info));
-			frag_len = min_t(u16, remain_len, ionic_rx_buf_size(buf_info));
 			sg_elem->len = cpu_to_le16(frag_len);
 			remain_len -= frag_len;
 			buf_info++;
@@ -889,17 +878,12 @@ void ionic_rx_fill(struct ionic_queue *q)
 void ionic_rx_empty(struct ionic_queue *q)
 {
 	struct ionic_rx_desc_info *desc_info;
-	struct ionic_buf_info *buf_info;
 	unsigned int i, j;
 
 	for (i = 0; i < q->num_descs; i++) {
 		desc_info = &q->rx_info[i];
-		for (j = 0; j < ARRAY_SIZE(desc_info->bufs); j++) {
-			buf_info = &desc_info->bufs[j];
-			if (buf_info->page)
-				ionic_rx_page_free(q, buf_info);
-		}
-
+		for (j = 0; j < ARRAY_SIZE(desc_info->bufs); j++)
+			ionic_rx_put_buf(q, &desc_info->bufs[j]);
 		desc_info->nbufs = 0;
 	}
 
@@ -1172,7 +1156,7 @@ static void ionic_tx_clean(struct ionic_queue *q,
 	struct sk_buff *skb;
 
 	if (desc_info->xdpf) {
-		ionic_xdp_tx_desc_clean(q->partner, desc_info);
+		ionic_xdp_tx_desc_clean(q->partner, desc_info, in_napi);
 		stats->clean++;
 
 		if (unlikely(__netif_subqueue_stopped(q->lif->netdev, q->index)))
-- 
2.17.1

Re: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool

[kernel test robot]

Hi Brett,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Brett-Creeley/ionic-debug-line-for-Tx-completion-errors/20240827-024626
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240826184422.21895-6-brett.creeley%40amd.com
patch subject: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20240828/202408280318.gE2lxcpO-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408280318.gE2lxcpO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408280318.gE2lxcpO-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:28,
                    from include/linux/skbuff.h:13,
                    from include/linux/ip.h:16,
                    from drivers/net/ethernet/pensando/ionic/ionic_txrx.c:4:
   drivers/net/ethernet/pensando/ionic/ionic_txrx.c: In function 'ionic_rx_build_skb':
>> include/linux/minmax.h:93:37: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '65536' to '0' [-Woverflow]
      93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
         |                                     ^
   include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
      96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
     213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
         |                           ^~~~~~~~~~
   drivers/net/ethernet/pensando/ionic/ionic_txrx.c:203:28: note: in expansion of macro 'min_t'
     203 |                 frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
         |                            ^~~~~
   drivers/net/ethernet/pensando/ionic/ionic_txrx.c: In function 'ionic_rx_fill':
>> include/linux/minmax.h:93:37: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '65536' to '0' [-Woverflow]
      93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
         |                                     ^
   include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
      96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
     213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
         |                           ^~~~~~~~~~
   drivers/net/ethernet/pensando/ionic/ionic_txrx.c:797:34: note: in expansion of macro 'min_t'
     797 |                 first_frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
         |                                  ^~~~~
>> include/linux/minmax.h:93:37: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '65536' to '0' [-Woverflow]
      93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
         |                                     ^
   include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
      96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
     213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
         |                           ^~~~~~~~~~
   drivers/net/ethernet/pensando/ionic/ionic_txrx.c:832:36: note: in expansion of macro 'min_t'
     832 |                         frag_len = min_t(u16, remain_len, IONIC_PAGE_SIZE);
         |                                    ^~~~~


vim +93 include/linux/minmax.h

d03eba99f5bf7c David Laight   2023-09-18  91  
017fa3e8918784 Linus Torvalds 2024-07-28  92  #define __cmp_once_unique(op, type, x, y, ux, uy) \
017fa3e8918784 Linus Torvalds 2024-07-28 @93  	({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
017fa3e8918784 Linus Torvalds 2024-07-28  94  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool

[kernel test robot]

Hi Brett,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Brett-Creeley/ionic-debug-line-for-Tx-completion-errors/20240827-024626
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240826184422.21895-6-brett.creeley%40amd.com
patch subject: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20240828/202408280648.f6Hb9cX3-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 08e5a1de8227512d4774a534b91cb2353cef6284)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408280648.f6Hb9cX3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408280648.f6Hb9cX3-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/net/ethernet/pensando/ionic/ionic_txrx.c:4:
   In file included from include/linux/ip.h:16:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/powerpc/include/asm/cacheflush.h:7:
   In file included from include/linux/mm.h:2228:
   include/linux/vmstat.h:503:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     503 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     504 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:510:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     510 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     511 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:523:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     523 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     524 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/pensando/ionic/ionic_txrx.c:203:30: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 65536 to 0 [-Wconstant-conversion]
     203 |                 frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
         |                            ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
   drivers/net/ethernet/pensando/ionic/ionic_dev.h:184:28: note: expanded from macro 'IONIC_PAGE_SIZE'
     184 | #define IONIC_PAGE_SIZE                         PAGE_SIZE
         |                                                 ^
   arch/powerpc/include/asm/page.h:25:34: note: expanded from macro 'PAGE_SIZE'
      25 | #define PAGE_SIZE               (ASM_CONST(1) << PAGE_SHIFT)
         |                                               ^
   include/linux/minmax.h:213:52: note: expanded from macro 'min_t'
     213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
         |                           ~~~~~~~~~~~~~~~~~~~~~~~~~^~
   include/linux/minmax.h:96:33: note: expanded from macro '__cmp_once'
      96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:93:31: note: expanded from macro '__cmp_once_unique'
      93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
         |                                ~~    ^
   drivers/net/ethernet/pensando/ionic/ionic_txrx.c:797:36: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 65536 to 0 [-Wconstant-conversion]
     797 |                 first_frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
         |                                  ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
   drivers/net/ethernet/pensando/ionic/ionic_dev.h:184:28: note: expanded from macro 'IONIC_PAGE_SIZE'
     184 | #define IONIC_PAGE_SIZE                         PAGE_SIZE
         |                                                 ^
   arch/powerpc/include/asm/page.h:25:34: note: expanded from macro 'PAGE_SIZE'
      25 | #define PAGE_SIZE               (ASM_CONST(1) << PAGE_SHIFT)
         |                                               ^
   include/linux/minmax.h:213:52: note: expanded from macro 'min_t'
     213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
         |                           ~~~~~~~~~~~~~~~~~~~~~~~~~^~
   include/linux/minmax.h:96:33: note: expanded from macro '__cmp_once'
      96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:93:31: note: expanded from macro '__cmp_once_unique'
      93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
         |                                ~~    ^
   drivers/net/ethernet/pensando/ionic/ionic_txrx.c:832:38: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 65536 to 0 [-Wconstant-conversion]
     832 |                         frag_len = min_t(u16, remain_len, IONIC_PAGE_SIZE);
         |                                    ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
   drivers/net/ethernet/pensando/ionic/ionic_dev.h:184:28: note: expanded from macro 'IONIC_PAGE_SIZE'
     184 | #define IONIC_PAGE_SIZE                         PAGE_SIZE
         |                                                 ^
   arch/powerpc/include/asm/page.h:25:34: note: expanded from macro 'PAGE_SIZE'
      25 | #define PAGE_SIZE               (ASM_CONST(1) << PAGE_SHIFT)
         |                                               ^
   include/linux/minmax.h:213:52: note: expanded from macro 'min_t'
     213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
         |                           ~~~~~~~~~~~~~~~~~~~~~~~~~^~
   include/linux/minmax.h:96:33: note: expanded from macro '__cmp_once'
      96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:93:31: note: expanded from macro '__cmp_once_unique'
      93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
         |                                ~~    ^
   7 warnings generated.


vim +203 drivers/net/ethernet/pensando/ionic/ionic_txrx.c

   174	
   175	static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
   176						  struct ionic_rx_desc_info *desc_info,
   177						  unsigned int headroom,
   178						  unsigned int len,
   179						  unsigned int num_sg_elems,
   180						  bool synced)
   181	{
   182		struct ionic_buf_info *buf_info;
   183		struct sk_buff *skb;
   184		unsigned int i;
   185		u16 frag_len;
   186	
   187		buf_info = &desc_info->bufs[0];
   188		prefetchw(buf_info->page);
   189	
   190		skb = napi_get_frags(&q_to_qcq(q)->napi);
   191		if (unlikely(!skb)) {
   192			net_warn_ratelimited("%s: SKB alloc failed on %s!\n",
   193					     dev_name(q->dev), q->name);
   194			q_to_rx_stats(q)->alloc_err++;
   195			return NULL;
   196		}
   197		skb_mark_for_recycle(skb);
   198	
   199		if (headroom)
   200			frag_len = min_t(u16, len,
   201					 IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN);
   202		else
 > 203			frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
   204	
   205		if (unlikely(!buf_info->page))
   206			goto err_bad_buf_page;
   207		ionic_rx_add_skb_frag(q, skb, buf_info, headroom, frag_len, synced);
   208		len -= frag_len;
   209		buf_info++;
   210	
   211		for (i = 0; i < num_sg_elems; i++, buf_info++) {
   212			if (unlikely(!buf_info->page))
   213				goto err_bad_buf_page;
   214			frag_len = min_t(u16, len, buf_info->len);
   215			ionic_rx_add_skb_frag(q, skb, buf_info, 0, frag_len, synced);
   216			len -= frag_len;
   217		}
   218	
   219		return skb;
   220	
   221	err_bad_buf_page:
   222		dev_kfree_skb(skb);
   223		return NULL;
   224	}
   225	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki