[PATCH net] net: wan: fsl_ucc_hdlc: return NETDEV_TX_OK if skb was freed

[PATCH net] net: wan: fsl_ucc_hdlc: return NETDEV_TX_OK if skb was freed

[Holger Brunck]

If the skb was freed in the ucc_hdlc_tx function and the packet marked
as dropped we need to return NETDEV_TX_OK. Otherwise the above layer
will try to requeue an already freed skb.

Fixes: c19b6d246a35 ("drivers/net: support hdlc function for QE-UCC")
Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 7af558780bdc..6ce539151618 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -360,7 +360,7 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 			dev->stats.tx_dropped++;
 			dev_kfree_skb(skb);
 			netdev_err(dev, "No enough space for hdlc head\n");
-			return -ENOMEM;
+			return NETDEV_TX_OK;
 		}
 
 		skb_push(skb, HDLC_HEAD_LEN);
@@ -377,7 +377,7 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 			dev->stats.tx_dropped++;
 			dev_kfree_skb(skb);
 			netdev_err(dev, "Wrong ppp header\n");
-			return -ENOMEM;
+			return NETDEV_TX_OK;
 		}
 
 		dev->stats.tx_bytes += skb->len;
@@ -390,7 +390,7 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 	default:
 		dev->stats.tx_dropped++;
 		dev_kfree_skb(skb);
-		return -ENOMEM;
+		return NETDEV_TX_OK;
 	}
 	netdev_sent_queue(dev, skb->len);
 	spin_lock_irqsave(&priv->lock, flags);
-- 
2.47.3

Re: [PATCH net] net: wan: fsl_ucc_hdlc: return NETDEV_TX_OK if skb was freed

[Jakub Kicinski]

On Mon,  4 May 2026 19:44:06 +0200 Holger Brunck wrote:
> If the skb was freed in the ucc_hdlc_tx function and the packet marked
> as dropped we need to return NETDEV_TX_OK. Otherwise the above layer
> will try to requeue an already freed skb.

Is this really true? I thought negative returns mean drop.

RE: [PATCH net] net: wan: fsl_ucc_hdlc: return NETDEV_TX_OK if skb was freed

[Holger Brunck]

> 
> On Mon,  4 May 2026 19:44:06 +0200 Holger Brunck wrote:
> > If the skb was freed in the ucc_hdlc_tx function and the packet marked
> > as dropped we need to return NETDEV_TX_OK. Otherwise the above layer
> > will try to requeue an already freed skb.
> 
> Is this really true? I thought negative returns mean drop.

the API suggest to only use NETDEV_TX_OK or NETDEV_TX_BUSY as return value.
I checked several drivers and they are usually returning NETDEV_TX_OK if an
error occurred and the driver consumed the packet. But you are right
dev_xmit_complete will also return true if the return code is smaller than zero
and the packet is not requeued. Should I update the commit message or should
the patch be dropped?

Best regards
Holger

Re: [PATCH net] net: wan: fsl_ucc_hdlc: return NETDEV_TX_OK if skb was freed

[Jakub Kicinski]

On Wed, 6 May 2026 09:35:00 +0000 Holger Brunck wrote:
> > On Mon,  4 May 2026 19:44:06 +0200 Holger Brunck wrote:  
> > > If the skb was freed in the ucc_hdlc_tx function and the packet marked
> > > as dropped we need to return NETDEV_TX_OK. Otherwise the above layer
> > > will try to requeue an already freed skb.  
> > 
> > Is this really true? I thought negative returns mean drop.  
> 
> the API suggest to only use NETDEV_TX_OK or NETDEV_TX_BUSY as return value.
> I checked several drivers and they are usually returning NETDEV_TX_OK if an
> error occurred and the driver consumed the packet. But you are right
> dev_xmit_complete will also return true if the return code is smaller than zero
> and the packet is not requeued. Should I update the commit message or should
> the patch be dropped?

Drop the patch.