[PATCH net-next 0/2][pull request] Intel Wired LAN Driver Updates 2026-09-29 (idpf, libie)

[PATCH net-next 0/2][pull request] Intel Wired LAN Driver Updates 2026-09-29 (idpf, libie)

[Tony Nguyen]

Alexander Lobakin adds flow-based XDP fallback on idpf for firmware
without Tx FIFO support by tracking out-of-order descriptor completions;
the faster queue-based path is used when available.

Li Li improves virtchnl failure diagnostics in libie by logging
transaction details to aid in debugging.

The following are changes since commit 6e23f90f2b8c3ea3bf904f86f3ed06bf2844b4bc:
  Merge branch 'net-consolidate-devmem-net_iov-freelist-and-dma-handling'
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 200GbE

Alexander Lobakin (1):
  idpf: add flow-based XDP fallback for FWs without Tx FIFO support

Li Li (1):
  libie: log more info when virtchnl fails

 drivers/net/ethernet/intel/idpf/idpf.h      |   2 +
 drivers/net/ethernet/intel/idpf/idpf_txrx.c |  12 +-
 drivers/net/ethernet/intel/idpf/idpf_txrx.h |  18 +--
 drivers/net/ethernet/intel/idpf/xdp.c       | 131 +++++++++++++++++++-
 drivers/net/ethernet/intel/idpf/xdp.h       |  73 ++++++++++-
 drivers/net/ethernet/intel/libie/controlq.c |  11 ++
 include/net/libeth/xdp.h                    |  13 ++
 7 files changed, 241 insertions(+), 19 deletions(-)

-- 
2.47.1

[PATCH net-next 1/2] idpf: add flow-based XDP fallback for FWs without Tx FIFO support

[Tony Nguyen]

From: Alexander Lobakin <aleksander.lobakin@intel.com>

From the first days of XDP implementation in idpf, it relied and
worked solely on top of the queue-based scheduling Tx mode, which
basically means simple FIFO. However, turned out not every firmware
supports this mode and XDP doesn't work there at all.

Since the flow-based scheduling Tx mode is mandatory and supported
by every FW, introduce a simple fallback guarded by a static key
to not hurt the more performant mode. The FB mode generates a
completion for each Tx descriptor and never guarantees that there
won't be any out-of-order completions. Serialize that using a
bitmap of completed descriptors and report contiguous blocks of
free bits to match XDP and XSk expectations and avoid further
code complication.

The usage of a bitmap on hotpath might sound scary, but this
fallback is able to reach around 70% of the QB mode's performance,
which is comparable to what ice gives us. The main bottlenecks are
unlikely()s and one completion per each descriptor, while in the QB
mode we have one completion per batch (which might contain 64 or
even 128 frames), plus the size of the completion descriptor is
8 bytes in this mode (4 bytes in the QB mode), which means a lot
of additional PCI traffic.

bloat-o-meter shows .text increase in about 2 Kb without adding new
functions or uninlining any of the existing ones. I played a bunch
with inlining and uninlining certain pieces or the whole fallback,
but the compiler collapses and optimizes libeth templates so hardly
so that each additional external call only makes things worse.

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: YiFei Zhu <zhuyifei@google.com>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
Tested-by: Patryk Holda <patryk.holda@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf.h      |   2 +
 drivers/net/ethernet/intel/idpf/idpf_txrx.c |  12 +-
 drivers/net/ethernet/intel/idpf/idpf_txrx.h |  18 +--
 drivers/net/ethernet/intel/idpf/xdp.c       | 131 +++++++++++++++++++-
 drivers/net/ethernet/intel/idpf/xdp.h       |  73 ++++++++++-
 include/net/libeth/xdp.h                    |  13 ++
 6 files changed, 230 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index f214023095ee..e76bee11756e 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -354,6 +354,7 @@ struct idpf_q_vec_rsrc {
  * @txqs: Used only in hotpath to get to the right queue very fast
  * @num_txq: Number of allocated TX queues
  * @num_xdp_txq: number of XDPSQs
+ * @xdpsq_fb: whether flow-based fallback is enabled for XDPSQs
  * @xdpsq_share: whether XDPSQ sharing is enabled
  * @xdp_prog: installed XDP program
  * @vdev_info: IDC vport device info pointer
@@ -383,6 +384,7 @@ struct idpf_vport {
 	struct idpf_tx_queue **txqs;
 	u16 num_txq;
 	u16 num_xdp_txq;
+	bool xdpsq_fb;
 	bool xdpsq_share;
 	struct bpf_prog *xdp_prog;
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 885df6294ad9..1f989fe476b1 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -227,7 +227,7 @@ static int idpf_tx_desc_alloc(const struct idpf_vport *vport,
 
 	idpf_xsk_setup_queue(vport, tx_q, VIRTCHNL2_QUEUE_TYPE_TX);
 
-	if (!idpf_queue_has(FLOW_SCH_EN, tx_q))
+	if (!idpf_queue_has(FLOW_SCH_EN, tx_q) || idpf_queue_has(XDP, tx_q))
 		return 0;
 
 	refillq = tx_q->refillq;
@@ -1054,6 +1054,13 @@ static void idpf_clean_queue_set(const struct idpf_queue_set *qs)
 			if (idpf_queue_has(XDP, q->txq)) {
 				q->txq->pending = 0;
 				q->txq->xdp_tx = 0;
+
+				if (static_branch_unlikely(&idpf_xdp_fb) &&
+				    idpf_queue_has(FLOW_SCH_EN, q->txq)) {
+					bitmap_zero(q->txq->pending_mask,
+						    q->txq->desc_count);
+					q->txq->last_ntu = 0;
+				}
 			} else {
 				q->txq->txq_grp->num_completions_pending = 0;
 			}
@@ -1316,7 +1323,8 @@ static void idpf_txq_group_rel(struct idpf_q_vec_rsrc *rsrc)
 			if (!txq_grp->txqs[j])
 				continue;
 
-			if (idpf_queue_has(FLOW_SCH_EN, txq_grp->txqs[j])) {
+			if (idpf_queue_has(FLOW_SCH_EN, txq_grp->txqs[j]) &&
+			    !idpf_queue_has(XDP, txq_grp->txqs[j])) {
 				kfree(txq_grp->txqs[j]->refillq);
 				txq_grp->txqs[j]->refillq = NULL;
 			}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index fe0c913f9bb9..37b9beccc018 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -628,12 +628,14 @@ libeth_cacheline_set_assert(struct idpf_rx_queue,
  * @clean_budget: singleq only, queue cleaning budget
  * @cleaned_pkts: Number of packets cleaned for the above said case
  * @refillq: Pointer to refill queue
+ * @cached_tstamp_caps: Tx timestamp capabilities negotiated with the CP
+ * @tstamp_task: Work that handles Tx timestamp read
  * @pending: number of pending descriptors to send in QB
  * @xdp_tx: number of pending &xdp_buff or &xdp_frame buffers
  * @timer: timer for XDP Tx queue cleanup
  * @xdp_lock: lock for XDP Tx queues sharing
- * @cached_tstamp_caps: Tx timestamp capabilities negotiated with the CP
- * @tstamp_task: Work that handles Tx timestamp read
+ * @pending_mask: mask of buffers waiting for completion in the FB XDP mode
+ * @last_ntu: @next_to_use from the previous batch in the FB XDP mode
  * @stats_sync: See struct u64_stats_sync
  * @q_stats: See union idpf_tx_queue_stats
  * @q_id: Queue id
@@ -695,6 +697,9 @@ struct idpf_tx_queue {
 			u16 cleaned_pkts;
 
 			struct idpf_sw_queue *refillq;
+
+			struct idpf_ptp_vport_tx_tstamp_caps *cached_tstamp_caps;
+			struct work_struct *tstamp_task;
 		};
 		struct {
 			u32 pending;
@@ -702,12 +707,12 @@ struct idpf_tx_queue {
 
 			struct libeth_xdpsq_timer *timer;
 			struct libeth_xdpsq_lock xdp_lock;
+
+			unsigned long *pending_mask;
+			u32 last_ntu;
 		};
 	};
 
-	struct idpf_ptp_vport_tx_tstamp_caps *cached_tstamp_caps;
-	struct work_struct *tstamp_task;
-
 	struct u64_stats_sync stats_sync;
 	struct idpf_tx_queue_stats q_stats;
 	__cacheline_group_end_aligned(read_write);
@@ -724,8 +729,7 @@ struct idpf_tx_queue {
 	__cacheline_group_end_aligned(cold);
 };
 libeth_cacheline_set_assert(struct idpf_tx_queue, 64,
-			    104 +
-			    offsetof(struct idpf_tx_queue, cached_tstamp_caps) -
+			    96 + offsetof(struct idpf_tx_queue, tstamp_task) -
 			    offsetofend(struct idpf_tx_queue, timer) +
 			    offsetof(struct idpf_tx_queue, q_stats) -
 			    offsetofend(struct idpf_tx_queue, tstamp_task),
diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
index cbccd4546768..c771663677a5 100644
--- a/drivers/net/ethernet/intel/idpf/xdp.c
+++ b/drivers/net/ethernet/intel/idpf/xdp.c
@@ -7,6 +7,8 @@
 #include "xdp.h"
 #include "xsk.h"
 
+DEFINE_STATIC_KEY_FALSE(idpf_xdp_fb);
+
 static int idpf_rxq_for_each(const struct idpf_q_vec_rsrc *rsrc,
 			     int (*fn)(struct idpf_rx_queue *rxq, void *arg),
 			     void *arg)
@@ -149,10 +151,12 @@ void idpf_xdp_copy_prog_to_rqs(const struct idpf_q_vec_rsrc *rsrc,
 
 static void idpf_xdp_tx_timer(struct work_struct *work);
 
-int idpf_xdpsqs_get(const struct idpf_vport *vport)
+int idpf_xdpsqs_get(struct idpf_vport *vport)
 {
 	struct libeth_xdpsq_timer **timers __free(kvfree) = NULL;
-	struct net_device *dev;
+	unsigned long **masks __free(kvfree) = NULL;
+	const struct net_device *dev;
+	bool warn;
 	u32 sqs;
 
 	if (!idpf_xdp_enabled(vport))
@@ -176,6 +180,34 @@ int idpf_xdpsqs_get(const struct idpf_vport *vport)
 	dev = vport->netdev;
 	sqs = vport->dflt_qv_rsrc.xdp_txq_offset;
 
+	vport->xdpsq_fb = !idpf_is_cap_ena(vport->adapter, IDPF_OTHER_CAPS,
+					   VIRTCHNL2_CAP_SPLITQ_QSCHED);
+	if (!vport->xdpsq_fb)
+		goto setup;
+
+	masks = kvzalloc_objs(*masks, vport->num_xdp_txq);
+	if (!masks)
+		goto err_masks;
+
+	for (u32 i = 0; i < vport->num_xdp_txq; i++) {
+		masks[i] = bitmap_zalloc_node(vport->txqs[sqs + i]->desc_count,
+					      GFP_KERNEL, cpu_to_mem(i));
+		if (!masks[i]) {
+			for (int j = i - 1; j >= 0; j--)
+				bitmap_free(masks[j]);
+
+			goto err_masks;
+		}
+	}
+
+	warn = !static_key_enabled(&idpf_xdp_fb);
+	static_branch_inc(&idpf_xdp_fb);
+
+	if (warn && net_ratelimit())
+		netdev_warn(dev,
+			    "The FW doesn't support Tx in FIFO mode, XDP Tx performance might be suboptimal\n");
+
+setup:
 	for (u32 i = sqs; i < vport->num_txq; i++) {
 		struct idpf_tx_queue *xdpsq = vport->txqs[i];
 
@@ -183,8 +215,8 @@ int idpf_xdpsqs_get(const struct idpf_vport *vport)
 		kfree(xdpsq->refillq);
 		xdpsq->refillq = NULL;
 
-		idpf_queue_clear(FLOW_SCH_EN, xdpsq);
-		idpf_queue_clear(FLOW_SCH_EN, xdpsq->complq);
+		idpf_queue_assign(FLOW_SCH_EN, xdpsq, vport->xdpsq_fb);
+		idpf_queue_assign(FLOW_SCH_EN, xdpsq->complq, vport->xdpsq_fb);
 		idpf_queue_set(NOIRQ, xdpsq);
 		idpf_queue_set(XDP, xdpsq);
 		idpf_queue_set(XDP, xdpsq->complq);
@@ -197,12 +229,25 @@ int idpf_xdpsqs_get(const struct idpf_vport *vport)
 		xdpsq->pending = 0;
 		xdpsq->xdp_tx = 0;
 		xdpsq->thresh = libeth_xdp_queue_threshold(xdpsq->desc_count);
+
+		if (static_branch_unlikely(&idpf_xdp_fb) && vport->xdpsq_fb) {
+			xdpsq->pending_mask = masks[i - sqs];
+			xdpsq->last_ntu = 0;
+		}
 	}
 
 	return 0;
+
+err_masks:
+	vport->xdpsq_fb = false;
+
+	for (u32 i = 0; i < vport->num_xdp_txq; i++)
+		kfree(timers[i]);
+
+	return -ENOMEM;
 }
 
-void idpf_xdpsqs_put(const struct idpf_vport *vport)
+void idpf_xdpsqs_put(struct idpf_vport *vport)
 {
 	struct net_device *dev;
 	u32 sqs;
@@ -222,10 +267,23 @@ void idpf_xdpsqs_put(const struct idpf_vport *vport)
 		libeth_xdpsq_deinit_timer(xdpsq->timer);
 		libeth_xdpsq_put(&xdpsq->xdp_lock, dev);
 
+		if (static_branch_unlikely(&idpf_xdp_fb) &&
+		    idpf_queue_has(FLOW_SCH_EN, xdpsq)) {
+			bitmap_free(xdpsq->pending_mask);
+			xdpsq->pending_mask = NULL;
+			xdpsq->last_ntu = 0;
+		}
+
 		kfree(xdpsq->timer);
 		xdpsq->refillq = NULL;
 		idpf_queue_clear(NOIRQ, xdpsq);
 	}
+
+	if (!vport->xdpsq_fb)
+		return;
+
+	static_branch_dec(&idpf_xdp_fb);
+	vport->xdpsq_fb = false;
 }
 
 static int idpf_xdp_parse_cqe(const struct idpf_splitq_4b_tx_compl_desc *desc,
@@ -250,6 +308,65 @@ static int idpf_xdp_parse_cqe(const struct idpf_splitq_4b_tx_compl_desc *desc,
 	return upper_16_bits(val);
 }
 
+static u32 idpf_xdpsq_poll_fb(struct idpf_tx_queue *xdpsq, u32 budget)
+{
+	struct idpf_compl_queue *cq = xdpsq->complq;
+	unsigned long *mask = xdpsq->pending_mask;
+	u32 done_frames, tx_cnt, new_ntc;
+	u32 ntc = cq->next_to_clean;
+	u32 cnt = cq->desc_count;
+	bool gen;
+
+	gen = idpf_queue_has(GEN_CHK, cq);
+
+	for (done_frames = 0; done_frames < budget; ) {
+		int ret;
+
+		ret = idpf_xdp_parse_cqe(&cq->comp[ntc].common, gen);
+		if (ret >= 0) {
+			__clear_bit(ret, mask);
+			done_frames++;
+
+			goto next;
+		}
+
+		switch (ret) {
+		case -ENODATA:
+			goto out;
+		case -EINVAL:
+			break;
+		}
+
+next:
+		if (unlikely(++ntc == cnt)) {
+			ntc = 0;
+			gen = !gen;
+			idpf_queue_change(GEN_CHK, cq);
+		}
+	}
+
+out:
+	cq->next_to_clean = ntc;
+
+	if (unlikely(!done_frames))
+		return 0;
+
+	tx_cnt = xdpsq->desc_count;
+
+	/* Don't go past next_to_use */
+	__set_bit(xdpsq->next_to_use, mask);
+
+	new_ntc = find_next_bit(mask, tx_cnt, xdpsq->next_to_clean);
+	done_frames = new_ntc - xdpsq->next_to_clean;
+
+	if (new_ntc == tx_cnt)
+		done_frames += find_first_bit(mask, tx_cnt);
+
+	__clear_bit(xdpsq->next_to_use, mask);
+
+	return done_frames;
+}
+
 u32 idpf_xdpsq_poll(struct idpf_tx_queue *xdpsq, u32 budget)
 {
 	struct idpf_compl_queue *cq = xdpsq->complq;
@@ -260,6 +377,10 @@ u32 idpf_xdpsq_poll(struct idpf_tx_queue *xdpsq, u32 budget)
 	u32 done_frames;
 	bool gen;
 
+	if (static_branch_unlikely(&idpf_xdp_fb) &&
+	    idpf_queue_has(FLOW_SCH_EN, xdpsq))
+		return idpf_xdpsq_poll_fb(xdpsq, budget);
+
 	gen = idpf_queue_has(GEN_CHK, cq);
 
 	for (done_frames = 0; done_frames < budget; ) {
diff --git a/drivers/net/ethernet/intel/idpf/xdp.h b/drivers/net/ethernet/intel/idpf/xdp.h
index 63e56f7d43e0..95e47ea2ccba 100644
--- a/drivers/net/ethernet/intel/idpf/xdp.h
+++ b/drivers/net/ethernet/intel/idpf/xdp.h
@@ -8,6 +8,10 @@
 
 #include "idpf_txrx.h"
 
+struct idpf_q_vec_rsrc;
+
+DECLARE_STATIC_KEY_FALSE(idpf_xdp_fb);
+
 int idpf_xdp_rxq_info_init(struct idpf_rx_queue *rxq);
 int idpf_xdp_rxq_info_init_all(const struct idpf_q_vec_rsrc *rsrc);
 void idpf_xdp_rxq_info_deinit(struct idpf_rx_queue *rxq, u32 model);
@@ -15,12 +19,40 @@ void idpf_xdp_rxq_info_deinit_all(const struct idpf_q_vec_rsrc *rsrc);
 void idpf_xdp_copy_prog_to_rqs(const struct idpf_q_vec_rsrc *rsrc,
 			       struct bpf_prog *xdp_prog);
 
-int idpf_xdpsqs_get(const struct idpf_vport *vport);
-void idpf_xdpsqs_put(const struct idpf_vport *vport);
+int idpf_xdpsqs_get(struct idpf_vport *vport);
+void idpf_xdpsqs_put(struct idpf_vport *vport);
 
 u32 idpf_xdpsq_poll(struct idpf_tx_queue *xdpsq, u32 budget);
 bool idpf_xdp_tx_flush_bulk(struct libeth_xdp_tx_bulk *bq, u32 flags);
 
+static inline void idpf_xdp_tx_xmit_fb(struct libeth_xdp_tx_desc desc, u32 i,
+				       const struct libeth_xdpsq *sq, u64 priv)
+{
+	struct idpf_flex_tx_sched_desc *tx_desc = sq->descs;
+	u32 cmd;
+
+	cmd = FIELD_PREP(IDPF_TXD_FLEX_FLOW_DTYPE_M,
+			 IDPF_TX_DESC_DTYPE_FLEX_FLOW_SCHE);
+	if (desc.flags & LIBETH_XDP_TX_LAST)
+		cmd |= IDPF_TXD_FLEX_FLOW_CMD_EOP;
+	if (priv && (desc.flags & LIBETH_XDP_TX_CSUM))
+		cmd |= IDPF_TXD_FLEX_FLOW_CMD_CS_EN;
+
+	tx_desc = &tx_desc[i];
+	tx_desc->buf_addr = cpu_to_le64(desc.addr);
+
+#ifdef __LIBETH_WORD_ACCESS
+	*(u64 *)&tx_desc->qw1 = ((u64)desc.len << 48) | ((u64)i << 32) | cmd;
+#else
+	tx_desc->qw1.rxr_bufsize = cpu_to_le16(desc.len);
+	tx_desc->qw1.compl_tag = cpu_to_le16(i);
+	tx_desc->qw1.ts[0] = 0;
+	tx_desc->qw1.ts[1] = 0;
+	tx_desc->qw1.ts[2] = 0;
+	tx_desc->qw1.cmd_dtype = cmd;
+#endif
+}
+
 /**
  * idpf_xdp_tx_xmit - produce a single HW Tx descriptor out of XDP desc
  * @desc: XDP descriptor to pull the DMA address and length from
@@ -34,6 +66,14 @@ static inline void idpf_xdp_tx_xmit(struct libeth_xdp_tx_desc desc, u32 i,
 	struct idpf_flex_tx_desc *tx_desc = sq->descs;
 	u32 cmd;
 
+	if (static_branch_unlikely(&idpf_xdp_fb) &&
+	    idpf_queue_has(FLOW_SCH_EN,
+			   libeth_xdpsq_to_sq(sq, struct idpf_tx_queue,
+					      next_to_use))) {
+		idpf_xdp_tx_xmit_fb(desc, i, sq, priv);
+		return;
+	}
+
 	cmd = FIELD_PREP(IDPF_FLEX_TXD_QW1_DTYPE_M,
 			 IDPF_TX_DESC_DTYPE_FLEX_L2TAG1_L2TAG2);
 	if (desc.flags & LIBETH_XDP_TX_LAST)
@@ -53,12 +93,34 @@ static inline void idpf_xdp_tx_xmit(struct libeth_xdp_tx_desc desc, u32 i,
 #endif
 }
 
-static inline void idpf_xdpsq_set_rs(const struct idpf_tx_queue *xdpsq)
+static inline void idpf_xdpsq_set_rs_fb(struct idpf_tx_queue *xdpsq)
+{
+	u32 ntu = xdpsq->next_to_use;
+	u32 old = xdpsq->last_ntu;
+
+	if (old > ntu) {
+		bitmap_set(xdpsq->pending_mask, old, xdpsq->desc_count - old);
+		if (ntu)
+			bitmap_set(xdpsq->pending_mask, 0, ntu);
+	} else {
+		bitmap_set(xdpsq->pending_mask, old, ntu - old);
+	}
+
+	xdpsq->last_ntu = ntu;
+}
+
+static __always_inline void idpf_xdpsq_set_rs(struct idpf_tx_queue *xdpsq)
 {
 	u32 ntu, cmd;
 
+	if (static_branch_unlikely(&idpf_xdp_fb) &&
+	    idpf_queue_has(FLOW_SCH_EN, xdpsq)) {
+		idpf_xdpsq_set_rs_fb(xdpsq);
+		return;
+	}
+
 	ntu = xdpsq->next_to_use;
-	if (unlikely(!ntu))
+	if (!ntu)
 		ntu = xdpsq->desc_count;
 
 	cmd = FIELD_PREP(IDPF_FLEX_TXD_QW1_CMD_M, IDPF_TX_DESC_CMD_RS);
@@ -84,7 +146,8 @@ static inline void idpf_xdpsq_update_tail(const struct idpf_tx_queue *xdpsq)
  * Set the RS bit ("end of batch"), bump the tail, and queue the cleanup timer.
  * To be called after a NAPI polling loop, at the end of .ndo_xdp_xmit() etc.
  */
-static inline void idpf_xdp_tx_finalize(void *_xdpsq, bool sent, bool flush)
+static __always_inline void idpf_xdp_tx_finalize(void *_xdpsq, bool sent,
+						 bool flush)
 {
 	struct idpf_tx_queue *xdpsq = _xdpsq;
 
diff --git a/include/net/libeth/xdp.h b/include/net/libeth/xdp.h
index 898723ab62e8..ed61d83bda2d 100644
--- a/include/net/libeth/xdp.h
+++ b/include/net/libeth/xdp.h
@@ -430,6 +430,19 @@ struct libeth_xdpsq {
 	struct libeth_xdpsq_lock	*lock;
 };
 
+/**
+ * libeth_xdpsq_to_sq - get SQ pointer from an XDPSQ pointer
+ * @xdpsq: &libeth_xdpsq corresponding to the queue
+ * @type: typeof() of the driver Tx queue structure
+ * @member: name of the NTU field inside @type
+ *
+ * Some of the sending callbacks take only &libeth_xdpsq pointer and no pointer
+ * to the actual driver Tx queue structure. Use this helper to quickly jump to
+ * the latter when needed.
+ */
+#define libeth_xdpsq_to_sq(xdpsq, type, member)				      \
+	container_of_const((xdpsq)->ntu, type, member)
+
 /**
  * struct libeth_xdp_tx_desc - abstraction for an XDP Tx descriptor
  * @addr: DMA address of the frame
-- 
2.47.1

Re: [PATCH net-next 1/2] idpf: add flow-based XDP fallback for FWs without Tx FIFO support

[Alexander Lobakin]

From: Sashiko-Bot <sashiko-bot@kernel.org>
Date: Wed, 30 Sep 2026 23:13:30 +0000

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] idpf: data race on pending_mask and last_ntu in XDP Tx finalize
> - [High] idpf: out-of-bounds write in idpf_xdpsq_poll_fb due to unvalidated completion tag
> - [Medium] idpf: mixing goto-based cleanup and __free() in idpf_xdpsqs_get

I think this already appeared earlier and I replied to these a while ago?

> --
> 
> commit 512d4db75f1d8f6ab37619b2712b334a77a2df35
> Author: Alexander Lobakin <aleksander.lobakin@intel.com>
> 
> idpf: add flow-based XDP fallback for FWs without Tx FIFO support
> 
> This commit introduces a static-key guarded flow-based XDP fallback for
> firmware that lacks queue-based scheduling support. It generates a
> completion for each Tx descriptor and serializes them using a dynamically
> allocated bitmap.
> 
>> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
>> index cbccd45467681..c771663677a52 100644
>> --- a/drivers/net/ethernet/intel/idpf/xdp.c
>> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
>> @@ -149,10 +151,12 @@ void idpf_xdp_copy_prog_to_rqs(const struct idpf_q_vec_rsrc *rsrc,
>>  
>>  static void idpf_xdp_tx_timer(struct work_struct *work);
>>  
>> -int idpf_xdpsqs_get(const struct idpf_vport *vport)
>> +int idpf_xdpsqs_get(struct idpf_vport *vport)
>>  {
>>  	struct libeth_xdpsq_timer **timers __free(kvfree) = NULL;
>> -	struct net_device *dev;
>> +	unsigned long **masks __free(kvfree) = NULL;
> 
> [ ... ]
> 
>> +	masks = kvzalloc_objs(*masks, vport->num_xdp_txq);
>> +	if (!masks)
>> +		goto err_masks;
> 
> [ ... ]
> 
>> +err_masks:
> 
> [Severity: Medium]
> Is it safe to mix goto statements with __free() scope-based cleanups in
> idpf_xdpsqs_get?

Safe. __free() can be used only to automatically cleanup pointers that
are on the stack. Here we have 2 dynamically-allocated arrays of
pointers, then can't be cleaned up that way.

> 
> The cleanup subsystem guidelines recommend that functions should either use
> scope-based cleanups entirely or rely on goto-based cleanups, but never
> mix both. Mixing them can lead to confusing ownership semantics and
> double-free or resource leak bugs.
> 
>> +	vport->xdpsq_fb = false;
>> +
>> +	for (u32 i = 0; i < vport->num_xdp_txq; i++)
>> +		kfree(timers[i]);
>> +
>> +	return -ENOMEM;
>>  }
> 
> [ ... ]
> 
>> @@ -250,6 +308,65 @@ static int idpf_xdp_parse_cqe(const struct idpf_splitq_4b_tx_compl_desc *desc,
>>  	return upper_16_bits(val);
>>  }
>>  
>> +static u32 idpf_xdpsq_poll_fb(struct idpf_tx_queue *xdpsq, u32 budget)
>> +{
> 
> [ ... ]
> 
>> +	for (done_frames = 0; done_frames < budget; ) {
>> +		int ret;
>> +
>> +		ret = idpf_xdp_parse_cqe(&cq->comp[ntc].common, gen);
>> +		if (ret >= 0) {
>> +			__clear_bit(ret, mask);
> 
> [Severity: High]
> Could this result in an out-of-bounds write?
> 
> The 16-bit completion tag parsed from hardware by idpf_xdp_parse_cqe() is
> used directly as an index for __clear_bit(). Since the mask is sized based
> on desc_count, a completion tag larger than desc_count returned from the
> hardware or firmware might cause an out-of-bounds write in the kernel
> heap.

HW never returns tags larger than desc_count because we actually set
these tags when filling Tx descriptors -- a completion descriptor only
carries the tag which was written by us to the corresponding Tx descriptor.

> 
> Should the tag value be validated against desc_count before using it as a
> bitmap index?
> 
>> +			done_frames++;
> 
> 
>> diff --git a/drivers/net/ethernet/intel/idpf/xdp.h b/drivers/net/ethernet/intel/idpf/xdp.h
>> index 63e56f7d43e00..95e47ea2ccba8 100644
>> --- a/drivers/net/ethernet/intel/idpf/xdp.h
>> +++ b/drivers/net/ethernet/intel/idpf/xdp.h
>> @@ -93,12 +93,34 @@ static inline void idpf_xdp_tx_xmit(struct libeth_xdp_tx_desc desc, u32 i,
>>  #endif
>>  }
>>  
>> -static inline void idpf_xdpsq_set_rs(const struct idpf_tx_queue *xdpsq)
>> +static inline void idpf_xdpsq_set_rs_fb(struct idpf_tx_queue *xdpsq)
>> +{
>> +	u32 ntu = xdpsq->next_to_use;
>> +	u32 old = xdpsq->last_ntu;
>> +
>> +	if (old > ntu) {
>> +		bitmap_set(xdpsq->pending_mask, old, xdpsq->desc_count - old);
> 
> [Severity: High]
> Could there be a data race on pending_mask and last_ntu here?
> 
> When idpf_xdp_tx_finalize() calls idpf_xdpsq_set_rs_fb(), the xdp_lock may
> have already been released (for example, by the bulk flush callback).

idpf_xdp_tx_finalize() takes the lock.

> 
> If multiple CPUs submit XDP frames concurrently when xdpsq_share is true,
> it appears we could have unprotected, concurrent, non-atomic bitmap_set()
> operations and updates to last_ntu. This might also race with the
> __clear_bit() operations from the consumer context in
> idpf_xdpsq_poll_fb().

idpf_xdpsq_poll_fb() is called by prep callbacks, all of them take the lock.

> 
>> +		if (ntu)
>> +			bitmap_set(xdpsq->pending_mask, 0, ntu);
>> +	} else {
>> +		bitmap_set(xdpsq->pending_mask, old, ntu - old);
>> +	}
>> +
>> +	xdpsq->last_ntu = ntu;
>> +}

Thanks,
Olek

Re: [PATCH net-next 1/2] idpf: add flow-based XDP fallback for FWs without Tx FIFO support

[Jakub Kicinski]

On Thu, 1 Oct 2026 17:43:50 +0200 Alexander Lobakin wrote:
> > Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> > - [High] idpf: data race on pending_mask and last_ntu in XDP Tx finalize
> > - [High] idpf: out-of-bounds write in idpf_xdpsq_poll_fb due to unvalidated completion tag
> > - [Medium] idpf: mixing goto-based cleanup and __free() in idpf_xdpsqs_get  
> 
> I think this already appeared earlier and I replied to these a while ago?

Please ignore the sashiko.dev reviews at the moment.
netdev isn't even CCed on those :/

Reply to the netdev-ai one, until the systems converge and netdev-ai 
is deprecated. 

Re: [PATCH net-next 1/2] idpf: add flow-based XDP fallback for FWs without Tx FIFO support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 11 potential
issue(s) to consider.

Critical: 1 · High: 2 · Medium: 3 · Low: 5

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] Flow-based (FB) XDP completion can release descriptors that
  were never sent to the hardware.
- [High] idpf_xdpsq_poll_fb() cannot handle a completely full ring.
- [High] The FB XDP path gives each descriptor its own compl_tag (the
  ring index i) and sets EOP only on the LIBETH_XDP_TX_LAST descriptor.
- [Medium] In FB mode, XDP SQs now keep FLOW_SCH_EN set:
  idpf_xdpsqs_get() calls idpf_queue_assign(FLOW_SCH_EN, xdpsq,…
- [Medium] The patch moves cached_tstamp_caps and tstamp_task into the
  skb-only arm of the union in struct idpf_tx_queue.
- [Medium] idpf_xdpsqs_put() decides whether to call
  static_branch_dec(&idpf_xdp_fb) only from vport->xdpsq_fb, not from…
- [Low] The commit message says XDP "doesn't work there at all" on
  firmware without Tx FIFO (QB) support.
- [Low] idpf_xdpsqs_get() mixes scope-based cleanup with goto-based
  cleanup.
- [Low] idpf_xdp_tx_xmit_fb() writes desc.len (a u32) into the flow
  descriptor's rxr_bufsize with no masking and no length check:…
- [Low] The kernel-doc for idpf_xdp_tx_finalize() in
  drivers/net/ethernet/intel/idpf/xdp.h still says "Set the RS bit…
- [Low] idpf_xdpsq_poll_fb() uses the 16-bit completion tag from a
  device-written CQE (idpf_xdp_parse_cqe() returns upper_16_bits(val))…

--

idpf: add flow-based XDP fallback for FWs without Tx FIFO support

This adds a fallback for XDP Tx queues that uses flow-based scheduling. It
applies to firmware that does not advertise VIRTCHNL2_CAP_SPLITQ_QSCHED,
and it is guarded by a static key. Completions can arrive out of order, so
each queue tracks them in a bitmap and reports contiguous blocks of
completed descriptors back to libeth XDP/XSk.

> basically means simple FIFO. However, turned out not every firmware
> supports this mode and XDP doesn't work there at all.

[Severity: Low]
The previous idpf_xdpsqs_get() always did
idpf_queue_clear(FLOW_SCH_EN, ...) on XDP SQs, whatever
VIRTCHNL2_CAP_SPLITQ_QSCHED said. That makes this read like a fix for XDP
being broken on such firmware.

Should this carry a Fixes: tag for the commit that added the QB-only XDP
SQ setup? Or is it meant only as a net-next feature?

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index 885df6294ad94..1f989fe476b1a 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -227,7 +227,7 @@ static int idpf_tx_desc_alloc(const struct idpf_vport *vport,
>  
>  	idpf_xsk_setup_queue(vport, tx_q, VIRTCHNL2_QUEUE_TYPE_TX);
>  
> -	if (!idpf_queue_has(FLOW_SCH_EN, tx_q))
> +	if (!idpf_queue_has(FLOW_SCH_EN, tx_q) || idpf_queue_has(XDP, tx_q))
>  		return 0;

[Severity: Medium]
The refillq setup is now skipped for XDP SQs. What about the tx_buf
allocation earlier in this same function?

In FB mode, idpf_xdpsqs_get() now leaves FLOW_SCH_EN set on XDP SQs.
idpf_vport_queues_alloc() calls idpf_xdpsqs_get() before
idpf_tx_desc_alloc_all(). So idpf_tx_desc_alloc()->idpf_tx_buf_alloc_all()
takes the flow scheduling branch:

idpf_tx_buf_alloc_all() {
	if (idpf_queue_has(FLOW_SCH_EN, tx_q))
		tx_q->buf_pool_size = U16_MAX;
	else
		tx_q->buf_pool_size = tx_q->desc_count;
	tx_q->tx_buf = kzalloc_objs(*tx_q->tx_buf, tx_q->buf_pool_size);
	...
}

XDP only uses tx_buf[0] to tx_buf[desc_count - 1], because compl_tag is
the ring index. Does this allocate about 3 MB of physically contiguous
memory (65535 * sizeof(struct libeth_sqe)) for each XDP SQ? There is
usually one XDP SQ per CPU.

If that high-order allocation fails, vport open or soft reset fails with
-ENOMEM. idpf_tx_buf_clean() also walks all 65535 entries at teardown.

[ ... ]

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> index fe0c913f9bb95..37b9beccc0185 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h

[ ... ]

> @@ -695,6 +697,9 @@ struct idpf_tx_queue {
>  			u16 cleaned_pkts;
>  
>  			struct idpf_sw_queue *refillq;
> +
> +			struct idpf_ptp_vport_tx_tstamp_caps *cached_tstamp_caps;
> +			struct work_struct *tstamp_task;
>  		};
>  		struct {
>  			u32 pending;
> @@ -702,12 +707,12 @@ struct idpf_tx_queue {
>  
>  			struct libeth_xdpsq_timer *timer;
>  			struct libeth_xdpsq_lock xdp_lock;
> +
> +			unsigned long *pending_mask;
> +			u32 last_ntu;
>  		};
>  	};
>  
> -	struct idpf_ptp_vport_tx_tstamp_caps *cached_tstamp_caps;
> -	struct work_struct *tstamp_task;
> -
>  	struct u64_stats_sync stats_sync;
>  	struct idpf_tx_queue_stats q_stats;
>  	__cacheline_group_end_aligned(read_write);

[Severity: Medium]
cached_tstamp_caps and tstamp_task are now in the skb-only arm of the
union. Is it still safe for idpf_vport_init_fast_path_txqs() to write them
on every Tx queue, XDP SQs included?

idpf_vport_init_fast_path_txqs() {
	...
		for (unsigned int j = 0; j < tx_grp->num_txq; j++, k++) {
			vport->txqs[k] = tx_grp->txqs[j];
			vport->txqs[k]->idx = k;

			if (!caps)
				continue;

			vport->txqs[k]->cached_tstamp_caps = caps;
			vport->txqs[k]->tstamp_task = tstamp_task;
		}
	...
}

This runs before idpf_xdpsqs_get(). On 32-bit SMP builds without spinlock
debugging, cached_tstamp_caps seems to sit at the same offset as
xdp_lock.share.

idpf_xdpsqs_get() only clears refillq, which overlaps the spinlock.
libeth_xdpsq_get(..., false) never writes share. So share keeps the low
byte of the caps pointer.

Suppose another user has enabled the global libeth_xdpsq_share key. Would
libeth_xdpsq_put() then call __libeth_xdpsq_put() and do a
static_branch_dec() for a reference that was never taken? That could turn
off locking for another device's shared XDPSQs.

> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
> index cbccd45467681..c771663677a52 100644
> --- a/drivers/net/ethernet/intel/idpf/xdp.c
> +++ b/drivers/net/ethernet/intel/idpf/xdp.c

[ ... ]

> @@ -149,10 +151,12 @@ void idpf_xdp_copy_prog_to_rqs(const struct idpf_q_vec_rsrc *rsrc,
>  
>  static void idpf_xdp_tx_timer(struct work_struct *work);
>  
> -int idpf_xdpsqs_get(const struct idpf_vport *vport)
> +int idpf_xdpsqs_get(struct idpf_vport *vport)
>  {
>  	struct libeth_xdpsq_timer **timers __free(kvfree) = NULL;
> -	struct net_device *dev;
> +	unsigned long **masks __free(kvfree) = NULL;
> +	const struct net_device *dev;
> +	bool warn;
>  	u32 sqs;
>  
>  	if (!idpf_xdp_enabled(vport))
> @@ -176,6 +180,34 @@ int idpf_xdpsqs_get(const struct idpf_vport *vport)
>  	dev = vport->netdev;
>  	sqs = vport->dflt_qv_rsrc.xdp_txq_offset;
>  
> +	vport->xdpsq_fb = !idpf_is_cap_ena(vport->adapter, IDPF_OTHER_CAPS,
> +					   VIRTCHNL2_CAP_SPLITQ_QSCHED);
> +	if (!vport->xdpsq_fb)
> +		goto setup;
> +
> +	masks = kvzalloc_objs(*masks, vport->num_xdp_txq);
> +	if (!masks)
> +		goto err_masks;

[ ... ]

> @@ -197,12 +229,25 @@ int idpf_xdpsqs_get(const struct idpf_vport *vport)
>  		xdpsq->pending = 0;
>  		xdpsq->xdp_tx = 0;
>  		xdpsq->thresh = libeth_xdp_queue_threshold(xdpsq->desc_count);
> +
> +		if (static_branch_unlikely(&idpf_xdp_fb) && vport->xdpsq_fb) {
> +			xdpsq->pending_mask = masks[i - sqs];
> +			xdpsq->last_ntu = 0;
> +		}
>  	}
>  
>  	return 0;
> +
> +err_masks:
> +	vport->xdpsq_fb = false;
> +
> +	for (u32 i = 0; i < vport->num_xdp_txq; i++)
> +		kfree(timers[i]);
> +
> +	return -ENOMEM;
>  }

[Severity: Low]
This isn't a bug on the current paths, but idpf_xdpsqs_get() now mixes
__free(kvfree) on the timers and masks arrays with goto setup, goto
err_masks and a manual cleanup label for timers[i]. The cleanup.h guidance
is not to mix goto-based and scope-based cleanup in one function.

Could this be restructured so the array elements are freed in one place?

[ ... ]

> @@ -222,10 +267,23 @@ void idpf_xdpsqs_put(const struct idpf_vport *vport)

[ ... ]

>  		kfree(xdpsq->timer);
>  		xdpsq->refillq = NULL;
>  		idpf_queue_clear(NOIRQ, xdpsq);
>  	}
> +
> +	if (!vport->xdpsq_fb)
> +		return;
> +
> +	static_branch_dec(&idpf_xdp_fb);
> +	vport->xdpsq_fb = false;
>  }

[Severity: Medium]
Can this static_branch_dec() run without a matching static_branch_inc()?

The decision depends only on vport->xdpsq_fb. idpf_initiate_soft_reset()
copies the vport into new_vport while FB XDP SQs are live, so the copy has
xdpsq_fb == true. The sequence is then:

idpf_initiate_soft_reset()
  idpf_vport_stop()
    idpf_vport_queues_rel()
      idpf_xdpsqs_put()        <- dec, xdpsq_fb = false
  memcpy(vport, new_vport, offsetof(struct idpf_vport, link_up))
                               <- xdpsq_fb is true again
  idpf_vport_open()
    idpf_vport_queues_alloc()
      <fails before xdpsq_fb is recomputed, for example the
       timers allocation in idpf_xdpsqs_get() returns -ENOMEM>
      idpf_vport_queues_rel()
        idpf_xdpsqs_put()      <- dec again with no inc

The fresh queues don't have the XDP flag, so the per-queue loop is
skipped, but the key is still decremented.

idpf_xdp_fb is module-global. Could this underflow the key? Could it also
drop the key to zero while another vport still has FB XDP SQs? Those
queues would then take the QB branches in idpf_xdp_tx_xmit(),
idpf_xdpsq_set_rs() and idpf_xdpsq_poll().

> @@ -250,6 +308,65 @@ static int idpf_xdp_parse_cqe(const struct idpf_splitq_4b_tx_compl_desc *desc,
>  	return upper_16_bits(val);
>  }
>  
> +static u32 idpf_xdpsq_poll_fb(struct idpf_tx_queue *xdpsq, u32 budget)
> +{
> +	struct idpf_compl_queue *cq = xdpsq->complq;
> +	unsigned long *mask = xdpsq->pending_mask;
> +	u32 done_frames, tx_cnt, new_ntc;
> +	u32 ntc = cq->next_to_clean;
> +	u32 cnt = cq->desc_count;
> +	bool gen;
> +
> +	gen = idpf_queue_has(GEN_CHK, cq);
> +
> +	for (done_frames = 0; done_frames < budget; ) {
> +		int ret;
> +
> +		ret = idpf_xdp_parse_cqe(&cq->comp[ntc].common, gen);
> +		if (ret >= 0) {
> +			__clear_bit(ret, mask);

[Severity: Low]
Should ret be checked against xdpsq->desc_count before it is used as a bit
index here?

idpf_xdpsqs_get() allocates pending_mask with only desc_count bits, but
the completion tag from the CQE can be any 16-bit value. An unexpected tag
from the device would clear a bit outside the bitmap.

> +			done_frames++;
> +
> +			goto next;
> +		}

[ ... ]

> +out:
> +	cq->next_to_clean = ntc;
> +
> +	if (unlikely(!done_frames))
> +		return 0;
> +
> +	tx_cnt = xdpsq->desc_count;
> +
> +	/* Don't go past next_to_use */
> +	__set_bit(xdpsq->next_to_use, mask);
> +
> +	new_ntc = find_next_bit(mask, tx_cnt, xdpsq->next_to_clean);
> +	done_frames = new_ntc - xdpsq->next_to_clean;

[Severity: Critical]
Can this report descriptors as completed when they were never posted to
the hardware?

Only idpf_xdpsq_set_rs_fb(), called from idpf_xdp_tx_finalize(), sets
pending_mask bits. libeth_xdp_tx_xmit_bulk() advances next_to_use and
pending, then drops the lock without finalizing.

On the Rx NAPI path, finalize runs once at the end of the poll, possibly
after several bulk flushes. So in the middle of a poll, the range
[last_ntu, next_to_use) has been written but is not marked in the bitmap
and has not been posted to the tail.

A later flush in the same poll goes through idpf_xdp_tx_prep():

	free = xdpsq->desc_count - xdpsq->pending;
	if (free < xdpsq->thresh)
		free += idpf_xdpsq_complete(xdpsq, xdpsq->thresh);

If everything in [next_to_clean, last_ntu) has completed, find_next_bit()
skips over the zero bits of the unposted window. It stops at the stop bit
at next_to_use and returns next_to_use - next_to_clean.

idpf_xdpsq_complete() then runs libeth_xdp_complete_tx() on those SQEs.
That recycles XDP_TX pages, returns xdp_frames, unmaps ndo_xdp_xmit DMA
and completes XSk frames. It also moves next_to_clean and pending past
them.

The finalize that follows marks the range pending and bumps the tail. The
NIC then DMAs from buffers that were already released, and the slots can
be reused while the hardware still owns them.

Should the stop bit be at last_ntu, the boundary actually handed to the
hardware, instead of next_to_use?

> +
> +	if (new_ntc == tx_cnt)
> +		done_frames += find_first_bit(mask, tx_cnt);
> +
> +	__clear_bit(xdpsq->next_to_use, mask);

[Severity: High]
What happens here when the ring is completely full and next_to_use equals
next_to_clean?

idpf_xdp_tx_prep() returns free = desc_count - pending and keeps no slot
in reserve, and libeth_xdp_tx_xmit_bulk() uses all of it:

	n = min(n, prep(xdpsq, &sq));

So if completion returns 0 while 0 < free < bulk size, pending can reach
desc_count. From then on, the stop bit lands on next_to_clean,
find_next_bit() returns next_to_clean, and done_frames is 0.

The __clear_bit(xdpsq->next_to_use, mask) above also clears the pending
bit of the descriptor at next_to_clean, which may still be in flight.
Every later poll returns 0, next_to_clean never moves, and free stays at 0.

Does this stall XDP_TX, redirect and XSk Tx on the queue until the queue
is reset?

The QB idpf_xdpsq_poll() handles this case explicitly (ret == tx_ntc gives
tx_cnt), but the FB variant does not seem to.

> +
> +	return done_frames;
> +}

[ ... ]

> diff --git a/drivers/net/ethernet/intel/idpf/xdp.h b/drivers/net/ethernet/intel/idpf/xdp.h
> index 63e56f7d43e00..95e47ea2ccba8 100644
> --- a/drivers/net/ethernet/intel/idpf/xdp.h
> +++ b/drivers/net/ethernet/intel/idpf/xdp.h

[ ... ]

> +static inline void idpf_xdp_tx_xmit_fb(struct libeth_xdp_tx_desc desc, u32 i,
> +				       const struct libeth_xdpsq *sq, u64 priv)
> +{
> +	struct idpf_flex_tx_sched_desc *tx_desc = sq->descs;
> +	u32 cmd;
> +
> +	cmd = FIELD_PREP(IDPF_TXD_FLEX_FLOW_DTYPE_M,
> +			 IDPF_TX_DESC_DTYPE_FLEX_FLOW_SCHE);
> +	if (desc.flags & LIBETH_XDP_TX_LAST)
> +		cmd |= IDPF_TXD_FLEX_FLOW_CMD_EOP;
> +	if (priv && (desc.flags & LIBETH_XDP_TX_CSUM))
> +		cmd |= IDPF_TXD_FLEX_FLOW_CMD_CS_EN;
> +
> +	tx_desc = &tx_desc[i];
> +	tx_desc->buf_addr = cpu_to_le64(desc.addr);
> +
> +#ifdef __LIBETH_WORD_ACCESS
> +	*(u64 *)&tx_desc->qw1 = ((u64)desc.len << 48) | ((u64)i << 32) | cmd;
> +#else
> +	tx_desc->qw1.rxr_bufsize = cpu_to_le16(desc.len);

[Severity: Low]
Should desc.len be masked with IDPF_TXD_FLEX_FLOW_BUFSIZE_M here and in
the __LIBETH_WORD_ACCESS variant above? rxr_bufsize holds only 14 bits of
size, and bit 14 is IDPF_TXD_FLEX_FLOW_RXR:

	__le16 rxr_bufsize;
#define IDPF_TXD_FLEX_FLOW_RXR		BIT(14)
#define IDPF_TXD_FLEX_FLOW_BUFSIZE_M	GENMASK(13, 0)

A buffer of 16K or more would set RXR or bit 15. It is not clear that XDP
or XSk can pass such a buffer to this path, so this may not be reachable
today.

> +	tx_desc->qw1.compl_tag = cpu_to_le16(i);

[Severity: High]
How are multi-buffer frames completed in this mode?

Each descriptor gets its own compl_tag, the ring index i, and only the
LIBETH_XDP_TX_LAST descriptor has EOP set. idpf_xdpsq_poll_fb() clears
exactly one pending_mask bit per RS completion.

The existing flow scheduling skb path seems to assume one RS completion
per packet. idpf_tx_splitq_frame() sets tx_params.compl_tag = buf_id once
per packet, and that tag goes into every descriptor of the packet.
idpf_tx_handle_rs_completion()->idpf_tx_clean_bufs() then cleans the whole
buffer chain from a single completion.

idpf_xdp_set_features() still advertises multi-buffer Tx through
libeth_xdp_set_features_noredir(..., idpf_get_max_tx_bufs(), ...) in
every scheduling mode.

For a frame of N descriptors, would N-1 bits stay set forever? If so,
find_next_bit() stops at the first stuck bit and XDP/XSk Tx on that queue
hangs permanently.

The commit message says "The FB mode generates a completion for each Tx
descriptor". Does that also hold for descriptors without EOP?

> +	tx_desc->qw1.ts[0] = 0;

[ ... ]

> @@ -84,7 +146,8 @@ static inline void idpf_xdpsq_update_tail(const struct idpf_tx_queue *xdpsq)
>   * Set the RS bit ("end of batch"), bump the tail, and queue the cleanup timer.
>   * To be called after a NAPI polling loop, at the end of .ndo_xdp_xmit() etc.
>   */
> -static inline void idpf_xdp_tx_finalize(void *_xdpsq, bool sent, bool flush)
> +static __always_inline void idpf_xdp_tx_finalize(void *_xdpsq, bool sent,
> +						 bool flush)
>  {
>  	struct idpf_tx_queue *xdpsq = _xdpsq;

[Severity: Low]
This isn't a bug, but the kernel-doc still says this sets the RS bit. In
FB mode, idpf_xdpsq_set_rs() returns early through idpf_xdpsq_set_rs_fb(),
which only updates pending_mask and last_ntu.

Could the comment be updated to cover the FB case?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929231305.1515873-1-anthony.l.nguyen%40intel.com

Re: [PATCH net-next 1/2] idpf: add flow-based XDP fallback for FWs without Tx FIFO support

[Alexander Lobakin]

From: Netdev-Bot+sashiko <netdev-bot+sashiko@kernel.org>
Date: Sat, 03 Oct 2026 23:35:14 +0000

> Thank you for your contribution! Sashiko AI review found 11 potential
> issue(s) to consider.
> 
> Critical: 1 · High: 2 · Medium: 3 · Low: 5
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).

[...]

Most makes sense, thanks. Dunno about the second patch in the series as
I'm not the author, but in this patch, I actually need to address stuff.

I was skeptic about AI reviews until I wired up CodeRabbit and Greptile
to my home project :D

Also, the review from sashiko.dev is the same as our internal Sashiko
gave. But this one is a lot more complete and complex. We might need
to take a look at our internal configuration.

pw-bot: cr

Thanks,
Olek

[PATCH net-next 2/2] libie: log more info when virtchnl fails

[Tony Nguyen]

From: Li Li <boolli@google.com>

Virtchnl failures can be hard to debug without logs. Logging the details
of virtchnl transactions can be useful for debugging virtchnl-related
issues.

Tested: Built & booted on a test machine and synthetically produced a
virtual failure to produce the following log:

idpf 0000:01:00.0: Non-zero virtchnl ret val 6 (msg op: 1, data_len: 8);
xn id: 0, cookie: 0
idpf 0000:01:00.0: Transaction failed (op 1, xn state:
3, id: 0, cookie: 0, size: 8)

Signed-off-by: Li Li <boolli@google.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Samuel Salin <Samuel.salin@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/libie/controlq.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index 6214fc036ce5..5fce6d9db1e2 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -835,6 +835,13 @@ libie_ctlq_xn_process_recv(struct libie_ctlq_xn_recv_params *params,
 		return false;
 	}
 
+	if (ctlq_msg->chnl_retval) {
+		dev_err_ratelimited(params->ctlq->dev,
+				    "Non-zero virtchnl ret val %u (msg op: %u, data_len: %u); xn id: %u, cookie: %u\n",
+				    ctlq_msg->chnl_retval, ctlq_msg->chnl_opcode,
+				    ctlq_msg->data_len, xn->index, xn->cookie);
+	}
+
 	if (xn->state != LIBIE_CTLQ_XN_ASYNC &&
 	    xn->state != LIBIE_CTLQ_XN_WAITING) {
 		spin_unlock(&xn->xn_lock);
@@ -1088,6 +1095,10 @@ int libie_ctlq_xn_send(struct libie_ctlq_xn_send_params *params)
 		params->recv_mem = xn->recv_mem;
 		break;
 	default:
+		dev_err_ratelimited(params->ctlq->dev,
+				    "Transaction failed (op %u, xn state: %u, id: %u, cookie: %u, size: %zu)\n",
+				    params->chnl_opcode, xn->state, xn->index,
+				    xn->cookie, xn->recv_mem.iov_len);
 		ret = -EBADMSG;
 		break;
 	}
-- 
2.47.1

Re: [PATCH net-next 2/2] libie: log more info when virtchnl fails

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new "Non-zero virtchnl ret val" log in
  libie_ctlq_xn_process_recv()…

--

libie: log more info when virtchnl fails

This commit adds rate-limited error messages to the libie control queue
code. libie_ctlq_xn_process_recv() now logs when a received message has a
non-zero virtchnl return value. libie_ctlq_xn_send() now logs when a
synchronous transaction ends in a failed state.

> diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
> index 6214fc036ce5f..5fce6d9db1e28 100644
> --- a/drivers/net/ethernet/intel/libie/controlq.c
> +++ b/drivers/net/ethernet/intel/libie/controlq.c
> @@ -835,6 +835,13 @@ libie_ctlq_xn_process_recv(struct libie_ctlq_xn_recv_params *params,
>  		return false;
>  	}
>  
> +	if (ctlq_msg->chnl_retval) {
> +		dev_err_ratelimited(params->ctlq->dev,
> +				    "Non-zero virtchnl ret val %u (msg op: %u, data_len: %u); xn id: %u, cookie: %u\n",
> +				    ctlq_msg->chnl_retval, ctlq_msg->chnl_opcode,
> +				    ctlq_msg->data_len, xn->index, xn->cookie);
> +	}

[Severity: Low]
Does this message always show values the device actually reported? When
ctlq_msg reaches libie_ctlq_xn_process_recv(), libie_ctlq_fill_rx_msg()
may already have overwritten chnl_retval and data_len:

drivers/net/ethernet/intel/libie/controlq.c:libie_ctlq_fill_rx_msg() {
    ...
	if (likely(msg->data_len)) {
		if (unlikely(msg->data_len > LIBIE_CTLQ_MAX_BUF_LEN)) {
			msg->data_len = LIBIE_CTLQ_MAX_BUF_LEN;
			msg->chnl_retval = U32_MAX;
		}
    ...
}

Suppose a reply's descriptor has a DATA_LEN field larger than
LIBIE_CTLQ_MAX_BUF_LEN. This would print "Non-zero virtchnl ret val
4294967295" with data_len equal to LIBIE_CTLQ_MAX_BUF_LEN.

By then the firmware's real virtchnl retval (possibly 0) and the real
DATA_LEN are lost. The log would then make the driver's own length check
look like a firmware error.

The new "Transaction failed" message in libie_ctlq_xn_send() prints
xn->recv_mem.iov_len, which is the same capped length.

Also, neither message includes ctlq_msg->hw_retval. That is the
mailbox-level status that libie_ctlq_fill_rx_msg() decodes from the
descriptor.

Could the oversized-length case be logged separately in
libie_ctlq_fill_rx_msg() with the raw DATA_LEN? Could hw_retval also be
added to the new messages?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929231305.1515873-1-anthony.l.nguyen%40intel.com