Re: [PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[Simon Horman <horms@kernel.org>]

On Thu, Jul 17, 2025 at 11:50:06AM +0530, G Thomas, Rohan wrote:
> Hi Simon,
> 
> On 7/16/2025 1:52 PM, Simon Horman wrote:
> > On Tue, Jul 15, 2025 at 07:14:21PM +0530, G Thomas, Rohan wrote:
> > > Hi Simon,
> > > 
> > > Thanks for reviewing the patch.
> > > 
> > > On 7/14/2025 7:10 PM, Simon Horman wrote:
> > > > On Mon, Jul 14, 2025 at 03:59:19PM +0800, Rohan G Thomas via B4 Relay wrote:
> > > > > From: Rohan G Thomas <rohan.g.thomas@altera.com>
> > > > > 
> > > > > Currently, in the AF_XDP transmit paths, the CIC bit of
> > > > > TX Desc3 is set for all packets. Setting this bit for
> > > > > packets transmitting through queues that don't support
> > > > > checksum offloading causes the TX DMA to get stuck after
> > > > > transmitting some packets. This patch ensures the CIC bit
> > > > > of TX Desc3 is set only if the TX queue supports checksum
> > > > > offloading.
> > > > > 
> > > > > Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
> > > > > Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> > > > 
> > > > Hi Rohan,
> > > > 
> > > > I notice that stmmac_xmit() handles a few other cases where
> > > > checksum offload should not be requested via stmmac_prepare_tx_desc:
> > > > 
> > > >           csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL);
> > > >           /* DWMAC IPs can be synthesized to support tx coe only for a few tx
> > > >            * queues. In that case, checksum offloading for those queues that don't
> > > >            * support tx coe needs to fallback to software checksum calculation.
> > > >            *
> > > >            * Packets that won't trigger the COE e.g. most DSA-tagged packets will
> > > >            * also have to be checksummed in software.
> > > >            */
> > > >           if (csum_insertion &&
> > > >               (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
> > > >                !stmmac_has_ip_ethertype(skb))) {
> > > >                   if (unlikely(skb_checksum_help(skb)))
> > > >                           goto dma_map_err;
> > > >                   csum_insertion = !csum_insertion;
> > > >           }
> > > > 
> > > > Do we need to care about them in stmmac_xdp_xmit_zc()
> > > > and stmmac_xdp_xmit_xdpf() too?
> > > 
> > > This patch only addresses avoiding the TX DMA hang by ensuring the CIC
> > > bit is only set when the queue supports checksum offload. For DSA tagged
> > > packets checksum offloading is not supported by the DWMAC IPs but no TX
> > > DMA hang. AFAIK, currently AF_XDP paths don't have equivalent handling
> > > like skb_checksum_help(), since they operate on xdp buffers. So this
> > > patch doesn't attempt to implement a sw fallback but just avoids DMA
> > > stall.
> > 
> > Ok, fair enough.
> > 
> > As per Andrew's advice elsewhere in this thread.
> > This patch also looks like it should be a fix for net,
> > and should have a Fixes tag.
> 
> Thanks for your comments.
> 
> You're right—this patch is a fix for the TX DMA hang issue caused by
> setting the CIC bit on queues that don't support checksum offload. But
> I couldn’t pinpoint a specific commit that introduced this behavior in
> the AF_XDP path. Initially, there was no support for DWMAC IPs with COE
> enabled only on specific queues, even though there can be IPs with such
> configuration. Commit 8452a05b2c63 ("net: stmmac: Tx coe sw fallback")
> added software fallback support for the AF_PACKET path. But the AF_XDP
> path has always enabled COE unconditionally even before that. So, do you
> think referencing the commit 8452a05b2c63 in the Fixes tag is
> appropriate and sufficient?

Hi Rohan,

Perhaps I'm missing the point, but my thinking is as follows:

As this patch only addresses the AF_XDP path I think we can take the
approach of asking "in which patch would a user of AF_XDP with this stmmac
observe this bug". (Or some variant thereof.) And I think the answer to
that question is the patch that added AF_XDP support to stmmac driver.

So I think we can use:

Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")