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>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f350a6662880a230a32ad6785c475cce4e950322..d9f7435a44fac695899e0b4ffd0dc7851c4e759f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2584,6 +2584,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
+ bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
struct xsk_buff_pool *pool = tx_q->xsk_pool;
unsigned int entry = tx_q->cur_tx;
struct dma_desc *tx_desc = NULL;
@@ -2671,7 +2672,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
}
stmmac_prepare_tx_desc(priv, tx_desc, 1, xdp_desc.len,
- true, priv->mode, true, true,
+ csum, priv->mode, true, true,
xdp_desc.len);
stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);
@@ -4983,6 +4984,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
{
struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
+ bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
unsigned int entry = tx_q->cur_tx;
struct dma_desc *tx_desc;
dma_addr_t dma_addr;
@@ -5034,7 +5036,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
stmmac_set_desc_addr(priv, tx_desc, dma_addr);
stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
- true, priv->mode, true, true,
+ csum, priv->mode, true, true,
xdpf->len);
tx_q->tx_count_frames++;
--
2.25.1On 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?
...
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.
>
> ...
Best Regards,
Rohan
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.
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?
Best Regards,
Rohan
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")
On 7/19/2025 1:36 AM, Simon Horman wrote:
>
> 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")
Hi Simon,
Thanks for the clarification. Will include the Fixes tag in the next
version.
Best Regards,
Rohan
© 2016 - 2025 Red Hat, Inc.