Re: [PATCH net-next v2 4/8] eth: mpnic: implement Tx queue allocation and cleanup

["Daniel Zahka" <daniel.zahka@gmail.com>]

On Sun Sep 27, 2026 at 8:01 PM EDT, netdev-bot+sashiko wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 2 · Medium: 0 · Low: 1
>
> 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).
>
> - [High] net: mpnic: missing bounds check on hardware completion head
>   causes NULL deref
> - [High] net: mpnic: integer underflow in desc_cnt causes massive out-of-
>   bounds unmaps
> - [Low] struct mpnic_ring::deferred_meta is documented as '-1 if the
>   doorbell is in sync with the tail', yet at this commit the rings come…
>
> --
>
> eth: mpnic: implement Tx queue allocation and cleanup
>
> This adds the Tx ring structures and the per-NAPI-vector allocation of
> queue triads, descriptor memory and interrupts. It also adds completion
> processing, where a single completion reports a work queue head and can
> release many packets. The queues are not yet enabled in hardware.
>
>> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> new file mode 100644
>> index 0000000000000..853976e11b032
>> --- /dev/null
>> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> @@ -0,0 +1,495 @@
>
> [ ... ]
>
>> +static void mpnic_ring_init(struct mpnic_ring *ring, u32 __iomem *doorbell,
>> +			    int q_idx)
>> +{
>> +	ring->doorbell = doorbell;
>> +	ring->q_idx = q_idx;
>> +}
>
> [Severity: Low]
> This isn't a bug at this point in the series, but the struct mpnic_ring
> comment in mpnic_txrx.h describes deferred_meta like this:
>
> 	/* TWQ only, index of the metadata descriptor of the last packet
> 	 * placed in the ring without ringing the doorbell, -1 if the
> 	 * doorbell is in sync with the tail.
> 	 */
> 	s32 deferred_meta;
>
> mpnic_alloc_napi_vector() gets the rings from kzalloc_flex(), and
> mpnic_ring_init() never sets deferred_meta. So the field starts at 0, not
> -1.
>
> Would it make sense to set deferred_meta to -1 in mpnic_ring_init(), so the
> field matches its comment from the start?
>
> Nothing reads deferred_meta in this commit. The later patch "eth: mpnic:
> start and stop the Tx HW queues" adds mpnic_enable_twq(), which sets tail
> and head to 0 and deferred_meta to -1 on every enable. In mpnic_open()
> that runs before mpnic_napi_enable() and netif_tx_wake_all_queues(), so
> this only affects the intermediate state of the series.
>

Hmm, I guess the comment is slightly ahead of the implementation (next
patch)... I suppose introducing deferred_meta later would probably be
the solution, but to me that would be a bit out of place, given the
other ring fields are really initialized in that next patch.

> [ ... ]
>
>> @@ -0,0 +1,495 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) Meta Platforms, Inc. and affiliates. */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/pci.h>
>> +#include <linux/slab.h>
>> +
>> +#include "mpnic.h"
>> +#include "mpnic_netdev.h"
>> +#include "mpnic_txrx.h"
>> +
>> +struct mpnic_xmit_cb {
>> +	u32 bytecount;
>> +	u8 desc_count;
>> +};
>> +
>> +#define MPNIC_XMIT_CB(__skb) ((struct mpnic_xmit_cb *)((__skb)->cb))
>> +
>> +/* Leave the interrupt moderation counters alone when arming or masking */
>> +#define MPNIC_TIM_PARAM_CFG_PRESERVE_MASK \
>> +	(MPNIC_TIM_CTL1_UPD_IGN_LONG_EVENT_CNT | \
>> +	 MPNIC_TIM_CTL1_UPD_IGN_LONG_TIME_CNT | \
>> +	 MPNIC_TIM_CTL1_UPD_IGN_SHORT_TIME_CNT)
>> +
>> +static void mpnic_nv_irq_disable(struct mpnic_napi_vector *nv)
>> +{
>> +	mpnic_wr64(nv->mpd, MPNIC_TIM_CTL1(nv->qt[0].cmpl.q_idx),
>> +		   MPNIC_TIM_PARAM_CFG_PRESERVE_MASK |
>> +		   MPNIC_TIM_CTL1_MASK_EN | MPNIC_TIM_CTL1_MASK);
>> +}
>> +
>> +static void mpnic_nv_irq_rearm(struct mpnic_napi_vector *nv)
>> +{
>> +	/* Rearming a single queue on a given IRQ rearms all the other
>> +	 * queues mapped to the same IRQ.
>> +	 */
>> +	mpnic_wr64(nv->mpd, MPNIC_TIM_CTL1(nv->qt[0].cmpl.q_idx),
>> +		   MPNIC_TIM_PARAM_CFG_PRESERVE_MASK | MPNIC_TIM_CTL1_MASK_EN);
>> +}
>> +
>> +static void mpnic_nv_irq_trigger(struct mpnic_napi_vector *nv)
>> +{
>> +	mpnic_wr64(nv->mpd, MPNIC_TIM_CTL1(nv->qt[0].cmpl.q_idx),
>> +		   MPNIC_TIM_PARAM_CFG_PRESERVE_MASK |
>> +		   MPNIC_TIM_CTL1_MASK_EN | MPNIC_TIM_CTL1_TRIGGER);
>> +}
>> +
>> +static unsigned int mpnic_desc_unused(struct mpnic_ring *ring)
>> +{
>> +	return (ring->head - ring->tail - 1) & ring->size_mask;
>> +}
>> +
>> +static struct netdev_queue *mpnic_txring_txq(const struct net_device *dev,
>> +					     const struct mpnic_ring *ring)
>> +{
>> +	return netdev_get_tx_queue(dev, ring->q_idx);
>> +}
>> +
>> +static void mpnic_unmap_single_twd(struct device *dev, __le64 *twd)
>> +{
>> +	u64 raw_twd = le64_to_cpu(*twd);
>> +
>> +	dma_unmap_single(dev, FIELD_GET(MPNIC_TWD_ADDR, raw_twd),
>> +			 FIELD_GET(MPNIC_TWD_LEN, raw_twd), DMA_TO_DEVICE);
>> +}
>> +
>> +static void mpnic_unmap_page_twd(struct device *dev, __le64 *twd)
>> +{
>> +	u64 raw_twd = le64_to_cpu(*twd);
>> +
>> +	dma_unmap_page(dev, FIELD_GET(MPNIC_TWD_ADDR, raw_twd),
>> +		       FIELD_GET(MPNIC_TWD_LEN, raw_twd), DMA_TO_DEVICE);
>> +}
>> +
>> +static void mpnic_clean_twq0(struct mpnic_napi_vector *nv, int napi_budget,
>> +			     struct mpnic_ring *ring, bool discard,
>> +			     unsigned int hw_head)
>> +{
>> +	u64 total_bytes = 0, total_packets = 0;
>> +	unsigned int head = ring->head;
>> +	struct netdev_queue *txq;
>> +	unsigned int clean_desc;
>> +
>> +	clean_desc = (hw_head - head) & ring->size_mask;
>> +
>> +	while (clean_desc) {
>> +		struct sk_buff *skb = ring->tx_buf[head];
>> +		unsigned int desc_cnt;
>> +
>> +		desc_cnt = MPNIC_XMIT_CB(skb)->desc_count;
>> +		if (desc_cnt > clean_desc)
>> +			break;
>> +
>> +		ring->tx_buf[head] = NULL;
>> +
>> +		clean_desc -= desc_cnt;
>> +
>> +		/* Step over the metadata descriptor */
>> +		head++;
>> +		head &= ring->size_mask;
>> +		desc_cnt--;
>> +
>> +		mpnic_unmap_single_twd(nv->dev, &ring->desc[head]);
>> +		head++;
>> +		head &= ring->size_mask;
>> +		desc_cnt--;
>> +
>> +		while (desc_cnt--) {
>
> [Severity: High]
> What happens here if desc_count is 0 or 1? desc_cnt is an unsigned
> int, and it is decremented twice unconditionally before the frag loop:
> once for the metadata descriptor and once for the head descriptor.
>
> With desc_count == 0 the desc_cnt > clean_desc check passes and
> clean_desc is left unchanged. The second decrement then wraps desc_cnt
> around to UINT_MAX. The frag loop calls mpnic_unmap_page_twd() about
> four billion times, walking round and round the ring. Each call hands
> whatever address and length it finds to dma_unmap_page() from softirq
> context.
>
> With desc_count == 1, mpnic_unmap_single_twd() is already applied to
> the following slot, which does not belong to this packet. The same
> wrap happens after that.
>
> desc_count lives in skb->cb and is only written by the xmit path added
> later in the series. This relies on every skb in tx_buf carrying a
> sane count. It also relies on hw_head never covering a packet whose
> descriptors are still being written. A device reporting a head that
> runs ahead of what was posted is enough to get here.
>
> Would it make sense to reject desc_cnt < 2 with a WARN_ON_ONCE()
> before stepping over the metadata descriptor? Possibly
> desc_cnt > MPNIC_MAX_SKB_DESC should be rejected as well.
>

Not reachable. We control the desc_count that is put into the control
block on xmit. We know that we allocate a primary metadata desc and at
least one dma mapped region for the skb head.

>> +			mpnic_unmap_page_twd(nv->dev, &ring->desc[head]);
>> +			head++;
>> +			head &= ring->size_mask;
>> +		}
>> +
>> +		total_bytes += MPNIC_XMIT_CB(skb)->bytecount;
>> +		total_packets++;
>> +
>> +		napi_consume_skb(skb, napi_budget);
>> +	}
>> +
>> +	if (!total_bytes)
>> +		return;
>> +
>> +	ring->head = head;
>> +
>> +	if (discard)
>> +		return;
>> +
>> +	txq = mpnic_txring_txq(nv->napi.dev, ring);
>> +	netif_txq_completed_wake(txq, total_packets, total_bytes,
>> +				 mpnic_desc_unused(ring),
>> +				 MPNIC_TX_DESC_WAKEUP);
>> +}
>> +
>> +static void mpnic_commit_cq_head(struct mpnic_ring *cmpl)
>> +{
>> +	u32 head = cmpl->head;
>> +
>> +	/* The tail shadows the last value written to the doorbell, so a
>> +	 * completion queue which has not moved costs no MMIO write.
>> +	 */
>> +	if (cmpl->tail != head) {
>> +		cmpl->tail = head;
>> +		writeq(head & cmpl->size_mask, cmpl->doorbell);
>> +	}
>> +}
>> +
>> +static void mpnic_clean_tcq(struct mpnic_napi_vector *nv,
>> +			    struct mpnic_q_triad *qt, int napi_budget)
>> +{
>> +	struct mpnic_ring *cmpl = &qt->cmpl;
>> +	__le64 *raw_tcd, done;
>> +	u32 head = cmpl->head;
>> +	s32 head0 = -1;
>> +
>> +	done = (head & (cmpl->size_mask + 1)) ? 0 : cpu_to_le64(MPNIC_TCD_DONE);
>> +	raw_tcd = &cmpl->desc[head & cmpl->size_mask];
>> +
>> +	/* Walk the completion queue collecting the heads reported by NIC.
>> +	 * Only the first work queue is enabled and no packet asks for a
>> +	 * timestamp, so every completion is a plain head update and the
>> +	 * descriptor type does not have to be decoded.
>> +	 */
>> +	while ((*raw_tcd & cpu_to_le64(MPNIC_TCD_DONE)) == done) {
>> +		u64 tcd;
>> +
>> +		dma_rmb();
>> +
>> +		tcd = le64_to_cpu(*raw_tcd);
>> +		head0 = FIELD_GET(MPNIC_TCD_TYPE0_HEAD0, tcd);
>> +
>> +		raw_tcd++;
>> +		head++;
>> +
>> +		if (unlikely(!(head & cmpl->size_mask))) {
>> +			done ^= cpu_to_le64(MPNIC_TCD_DONE);
>> +			raw_tcd = &cmpl->desc[0];
>> +		}
>> +	}
>> +
>> +	cmpl->head = head;
>> +
>> +	if (head0 >= 0)
>> +		mpnic_clean_twq0(nv, napi_budget, &qt->sub0, false, head0);
>> +}
>> +
>> +static int mpnic_poll(struct napi_struct *napi, int budget)
>> +{
>> +	struct mpnic_napi_vector *nv = container_of(napi,
>> +						    struct mpnic_napi_vector,
>> +						    napi);
>> +	int i;
>> +
>> +	for (i = 0; i < nv->txt_count; i++)
>> +		mpnic_clean_tcq(nv, &nv->qt[i], budget);
>> +
>> +	for (i = 0; i < nv->txt_count; i++)
>> +		mpnic_commit_cq_head(&nv->qt[i].cmpl);
>> +
>> +	if (likely(napi_complete_done(napi, 0)))
>> +		mpnic_nv_irq_rearm(nv);
>> +
>> +	return 0;
>> +}
>> +
>> +static irqreturn_t mpnic_msix_clean_rings(int __always_unused irq, void *data)
>> +{
>> +	struct mpnic_napi_vector *nv = data;
>> +
>> +	napi_schedule_irqoff(&nv->napi);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static void mpnic_free_napi_vector(struct mpnic_net *mpn,
>> +				   struct mpnic_napi_vector *nv)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < nv->txt_count; i++)
>> +		mpn->tx[nv->qt[i].sub0.q_idx] = NULL;
>> +
>> +	mpnic_free_irq(nv->mpd, nv->v_idx, nv);
>> +	netif_napi_del_locked(&nv->napi);
>> +	mpn->napi[nv->v_idx - MPNIC_NON_NAPI_VECTORS] = NULL;
>> +	kfree(nv);
>> +}
>> +
>> +void mpnic_free_napi_vectors(struct mpnic_net *mpn)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++)
>> +		if (mpn->napi[i])
>> +			mpnic_free_napi_vector(mpn, mpn->napi[i]);
>> +}
>> +
>> +static void mpnic_ring_init(struct mpnic_ring *ring, u32 __iomem *doorbell,
>> +			    int q_idx)
>> +{
>> +	ring->doorbell = doorbell;
>> +	ring->q_idx = q_idx;
>> +}
>> +
>> +static int mpnic_alloc_napi_vector(struct mpnic_dev *mpd,
>> +				   struct mpnic_net *mpn, unsigned int idx)
>> +{
>> +	u32 __iomem *uc_addr = READ_ONCE(mpd->uc_addr0);
>> +	struct mpnic_napi_vector *nv;
>> +	int err;
>> +
>> +	/* Doorbells are plain pointers into the register window, they have
>> +	 * no way of noticing that it went away.
>> +	 */
>> +	if (!uc_addr)
>> +		return -EIO;
>> +
>> +	nv = kzalloc_flex(*nv, qt, 1);
>> +	if (!nv)
>> +		return -ENOMEM;
>> +
>> +	nv->txt_count = 1;
>> +	nv->mpd = mpd;
>> +	nv->dev = mpd->dev;
>> +	nv->v_idx = idx + MPNIC_NON_NAPI_VECTORS;
>> +
>> +	mpn->napi[idx] = nv;
>> +	netif_napi_add_config_locked(mpn->netdev, &nv->napi, mpnic_poll, idx);
>> +	netif_napi_set_irq_locked(&nv->napi,
>> +				  pci_irq_vector(to_pci_dev(mpd->dev),
>> +						 nv->v_idx));
>> +
>> +	snprintf(nv->name, sizeof(nv->name), "%s-TxRx-%u",
>> +		 mpn->netdev->name, idx);
>> +
>> +	err = mpnic_request_irq(mpd, nv->v_idx, mpnic_msix_clean_rings, 0,
>> +				nv->name, nv);
>> +	if (err)
>> +		goto err_napi_del;
>> +
>> +	mpnic_ring_init(&nv->qt[0].sub0, &uc_addr[MPNIC_TWQ_TAIL(idx, 0)], idx);
>> +	mpnic_ring_init(&nv->qt[0].cmpl, &uc_addr[MPNIC_TCQ_HEAD(idx)], idx);
>> +	mpn->tx[idx] = &nv->qt[0].sub0;
>> +
>> +	return 0;
>> +
>> +err_napi_del:
>> +	netif_napi_del_locked(&nv->napi);
>> +	mpn->napi[idx] = NULL;
>> +	kfree(nv);
>> +	return err;
>> +}
>> +
>> +int mpnic_alloc_napi_vectors(struct mpnic_net *mpn)
>> +{
>> +	unsigned int i;
>> +	int err;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++) {
>> +		err = mpnic_alloc_napi_vector(mpn->mpd, mpn, i);
>> +		if (err)
>> +			goto err_free_vectors;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_free_vectors:
>> +	mpnic_free_napi_vectors(mpn);
>> +
>> +	return err;
>> +}
>> +
>> +static void mpnic_free_ring_resources(struct device *dev,
>> +				      struct mpnic_ring *ring)
>> +{
>> +	kvfree(ring->tx_buf);
>> +	ring->tx_buf = NULL;
>> +
>> +	/* If size is not set there are no descriptors present */
>> +	if (!ring->size)
>> +		return;
>> +
>> +	dma_free_coherent(dev, ring->size, ring->desc, ring->dma);
>> +	ring->size_mask = 0;
>> +	ring->size = 0;
>> +}
>> +
>> +static int mpnic_alloc_ring_desc(struct mpnic_net *mpn,
>> +				 struct mpnic_ring *ring, u32 count)
>> +{
>> +	struct device *dev = mpn->netdev->dev.parent;
>> +	size_t size;
>> +
>> +	size = ALIGN(array_size(sizeof(*ring->desc), count), 4096);
>> +
>> +	ring->desc = dma_alloc_coherent(dev, size, &ring->dma,
>> +					GFP_KERNEL | __GFP_NOWARN);
>> +	if (!ring->desc)
>> +		return -ENOMEM;
>> +
>> +	ring->size_mask = count - 1;
>> +	ring->size = size;
>> +
>> +	return 0;
>> +}
>> +
>> +static void mpnic_free_tx_qt_resources(struct mpnic_net *mpn,
>> +				       struct mpnic_q_triad *qt)
>> +{
>> +	struct device *dev = mpn->netdev->dev.parent;
>> +
>> +	mpnic_free_ring_resources(dev, &qt->cmpl);
>> +	mpnic_free_ring_resources(dev, &qt->sub0);
>> +}
>> +
>> +static int mpnic_alloc_tx_qt_resources(struct mpnic_net *mpn,
>> +				       struct mpnic_q_triad *qt)
>> +{
>> +	int err;
>> +
>> +	err = mpnic_alloc_ring_desc(mpn, &qt->sub0, mpn->txq_size);
>> +	if (err)
>> +		return err;
>> +
>> +	qt->sub0.tx_buf = kvzalloc_objs(*qt->sub0.tx_buf, mpn->txq_size,
>> +					GFP_KERNEL | __GFP_NOWARN);
>> +	if (!qt->sub0.tx_buf) {
>> +		err = -ENOMEM;
>> +		goto err_free_qt;
>> +	}
>> +
>> +	err = mpnic_alloc_ring_desc(mpn, &qt->cmpl, mpn->txq_size);
>> +	if (err)
>> +		goto err_free_qt;
>> +
>> +	return 0;
>> +
>> +err_free_qt:
>> +	mpnic_free_tx_qt_resources(mpn, qt);
>> +	return err;
>> +}
>> +
>> +static void mpnic_free_nv_resources(struct mpnic_net *mpn,
>> +				    struct mpnic_napi_vector *nv)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < nv->txt_count; i++)
>> +		mpnic_free_tx_qt_resources(mpn, &nv->qt[i]);
>> +}
>> +
>> +static int mpnic_alloc_nv_resources(struct mpnic_net *mpn,
>> +				    struct mpnic_napi_vector *nv)
>> +{
>> +	int i, err;
>> +
>> +	for (i = 0; i < nv->txt_count; i++) {
>> +		err = mpnic_alloc_tx_qt_resources(mpn, &nv->qt[i]);
>> +		if (err)
>> +			goto err_free_qt_resources;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_free_qt_resources:
>> +	while (i--)
>> +		mpnic_free_tx_qt_resources(mpn, &nv->qt[i]);
>> +	return err;
>> +}
>> +
>> +void mpnic_free_resources(struct mpnic_net *mpn)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++)
>> +		mpnic_free_nv_resources(mpn, mpn->napi[i]);
>> +}
>> +
>> +int mpnic_alloc_resources(struct mpnic_net *mpn)
>> +{
>> +	int i, err;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++) {
>> +		err = mpnic_alloc_nv_resources(mpn, mpn->napi[i]);
>> +		if (err)
>> +			goto err_free_resources;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_free_resources:
>> +	while (i--)
>> +		mpnic_free_nv_resources(mpn, mpn->napi[i]);
>> +
>> +	return err;
>> +}
>> +
>> +int mpnic_set_netif_queues(struct mpnic_net *mpn)
>> +{
>> +	int i, j, err;
>> +
>> +	err = netif_set_real_num_tx_queues(mpn->netdev, mpn->num_tx_queues);
>> +	if (err)
>> +		return err;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++) {
>> +		struct mpnic_napi_vector *nv = mpn->napi[i];
>> +
>> +		for (j = 0; j < nv->txt_count; j++)
>> +			netif_queue_set_napi(mpn->netdev, nv->qt[j].sub0.q_idx,
>> +					     NETDEV_QUEUE_TYPE_TX, &nv->napi);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +void mpnic_reset_netif_queues(struct mpnic_net *mpn)
>> +{
>> +	int i, j;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++) {
>> +		struct mpnic_napi_vector *nv = mpn->napi[i];
>> +
>> +		for (j = 0; j < nv->txt_count; j++)
>> +			netif_queue_set_napi(mpn->netdev, nv->qt[j].sub0.q_idx,
>> +					     NETDEV_QUEUE_TYPE_TX, NULL);
>> +	}
>> +}
>> +
>> +void mpnic_napi_disable(struct mpnic_net *mpn)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++) {
>> +		napi_disable_locked(&mpn->napi[i]->napi);
>> +
>> +		mpnic_nv_irq_disable(mpn->napi[i]);
>> +	}
>> +}
>> +
>> +void mpnic_napi_enable(struct mpnic_net *mpn)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < mpn->num_napi; i++)
>> +		napi_enable_locked(&mpn->napi[i]->napi);
>> +
>> +	/* Force the first interrupt on each vector to guarantee that any
>> +	 * completions posted during bringup are processed.
>> +	 */
>> +	for (i = 0; i < mpn->num_napi; i++)
>> +		mpnic_nv_irq_trigger(mpn->napi[i]);
>> +
>> +	mpnic_wrfl(mpn->mpd);
>> +}
>
> [Severity: High]
> Is hw_head used here without any check against what was actually
> posted? It comes straight from the TCD read in mpnic_clean_tcq(), and
> clean_desc is derived only from its distance to ring->head. It is never
> compared with ring->tail.
>
> The device could report a head beyond the last descriptor the driver
> posted, for example through a firmware bug or a completion type that
> mpnic_clean_tcq() does not decode. clean_desc then covers slots that
> were never filled. tx_buf comes from kvzalloc_objs(), and only the
> metadata slot of each packet holds an skb. So ring->tx_buf[head] is
> NULL there, and MPNIC_XMIT_CB(skb)->desc_count dereferences a NULL
> pointer from NAPI context.
>
> Could clean_desc be clamped to the number of in-flight descriptors,
> i.e. (ring->tail - head) & ring->size_mask? Alternatively, could the
> loop bail out with a WARN_ON_ONCE() when it finds an empty tx_buf
> slot? Ignoring a bogus completion seems preferable to taking the host
> down.

We trust the device to fill out this completion field correctly.