Re: [PATCH net] octeontx2-pf: fix XDP frames freed as skbs on interface down

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] octeontx2-pf: missing xsk_tx_completed() call in otx2_cleanup_tx_cqes()
--

commit 40946781f760bef2ef894a136c752dbc204d2650
Author: Nicolai Buchwitz <nb@tipi-net.de>

octeontx2-pf: fix XDP frames freed as skbs on interface down

This commit updates otx2_cleanup_tx_cqes() to handle CQ_XDP completions via
otx2_xdp_snd_pkt_handler() instead of freeing pending xdp_frame pointers with
dev_kfree_skb_any() during interface teardown or XDP program replacement.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> index 90af9b9044aa0..e3b07e5335fb8 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> @@ -1332,6 +1332,7 @@ void otx2_cleanup_tx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue *cq)
>  	struct nix_cqe_tx_s *cqe;
>  	struct netdev_queue *txq;
>  	int processed_cqe = 0;
> +	int xsk_frames = 0;
>  	struct sg_list *sg;
>  	int qidx;
>  
> @@ -1348,6 +1349,10 @@ void otx2_cleanup_tx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue *cq)
>  
>  		if (!cqe)
>  			continue;
> +		if (cq->cq_type == CQ_XDP) {
> +			otx2_xdp_snd_pkt_handler(pfvf, sq, cqe, &xsk_frames);
> +			continue;
> +		}

[Severity: Medium]
Does otx2_cleanup_tx_cqes() also need to pass xsk_frames to
xsk_tx_completed() after draining the loop?

When AF_XDP zero-copy TX descriptors are reaped in
drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c:

otx2_xdp_snd_pkt_handler() {
	...
	sg = &sq->sg[snd_comp->sqe_id];
	if (sg->flags & OTX2_AF_XDP_FRAME) {
		(*xsk_frames)++;
		return;
	}
	...
}

the handler only increments xsk_frames and relies on the caller to publish
the completions to sq->xsk_pool, as done in the normal NAPI completion path via
otx2_tx_napi_handler() -> otx2_zc_submit_pkts():

	if (*xsk_frames)
		xsk_tx_completed(xsk_pool, *xsk_frames);

In otx2_cleanup_tx_cqes(), xsk_frames is accumulated across the while
(cq->pend_cqe) loop, but it is never read afterwards. If an interface with an
active AF_XDP zero-copy TX socket is brought down or has its XDP program
replaced while AF_XDP TX completions are pending in a CQ_XDP queue, wouldn't
those completion ring entries be leaked without calling
xsk_tx_completed(sq->xsk_pool, xsk_frames)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-nb-otx2-xdp-cqe-cleanup-net-v1-1-710635c0f504@tipi-net.de?part=1