Re: [PATCH v3 net 2/7] net: enetc: ensure enetc_xdp_xmit() calls enetc_update_tx_ring_tail()

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Wed, Sep 16, 2026 at 10:28:06PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] net: enetc: missing XDP_XMIT_FLUSH doorbell when k=0
> --
> 
> commit d8271163a46118cc9a3212492775857b3ece7ba0
> Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> net: enetc: ensure enetc_xdp_xmit() calls enetc_update_tx_ring_tail()
> 
> This commit modifies enetc_xdp_xmit() to fix the condition that triggers
> the TX doorbell. It removes the redundant xdp_tx_frm_cnt variable and
> updates the check to verify if at least one frame was sent along with
> flush requests.
> 
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> [ ... ]
> > @@ -1845,19 +1844,17 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames,
> >  						    &xdp_redirect_arr[i]);
> >  			break;
> >  		}
> > -
> > -		xdp_tx_frm_cnt++;
> >  	}
> >  
> > -	if (unlikely((flags & XDP_XMIT_FLUSH) || k != xdp_tx_frm_cnt))
> > +	if (unlikely(k && ((flags & XDP_XMIT_FLUSH) || k < num_frames)))
> 
> [Severity: High]
> Does the addition of the k && condition block the mandatory XDP_XMIT_FLUSH
> doorbell if the current transmission call fails to enqueue any frames?
> 
> If the devmap subsystem enqueues a full batch of frames with flags=0
> (no flush), enetc_xdp_xmit() successfully enqueues them but skips the
> doorbell because k == num_frames and flags=0.
> 
> Subsequently, devmap calls __dev_flush() which invokes enetc_xdp_xmit()
> with flags=XDP_XMIT_FLUSH to ring the doorbell. If the hardware TX ring
> is now full, this call fails to enqueue its pending frames, resulting in
> k=0.
> 
> Since the modified condition evaluates to false when k=0, wouldn't this
> skip the doorbell entirely and leave the previously enqueued XDP frames
> permanently stuck in the hardware TX ring?

This is fun. The LLM has no idea what it's saying, but it's saying the
words that are gravitating in the right direction anyway (kind of).

Specifically, it's unable to articulate "the original code was
functionally correct, it just needs some cleanup".

All other drivers only ring the doorbell if the XDP_XMIT_FLUSH bit is
set in 'flags', and I wasn't able to understand why that is correct/
sufficient, either when I wrote the original commit, or this 'fix'.

My concern was: "if a full batch with flags=0 fails to be completely
enqueued, don't I need to ring the doorbell anyway? what if another
ndo_xdp_xmit() call never comes?" and as a result, I remained under the
impression that this is condition that needs handling.

But the answer is "it _will_ come; the only reason why you got an
ndo_xdp_xmit() call with flags=0 is because there's even more data to
send, it's just that there's no more room to store it in bq->count,
so let's first send what we gathered thus far". The remainder _will_
result in another ndo_xdp_xmit() call, which (if terminal) _will_ have
the XDP_XMIT_FLUSH flag.

What is even more interesting is that, after dead code elimination,
enetc _does_ behave like every other driver, and there is no bug, just
misunderstanding. Because as identified, "k != xdp_tx_frm_cnt" is always
false, we have "if (A || false)" which simplifies to "if (A)". So enetc
flushes the doorbell based on the same 'if (flags & XDP_XMIT_FLUSH)'
condition as everybody else.

I will remove this patch from this series, and resubmit the cleanup part
only to net-next.