> -----Original Message-----
> From: David Miller <da...@davemloft.net>
> Sent: Tuesday, May 5, 2020 1:40 AM
> To: Ooi, Joyce <joyce....@intel.com>
> Cc: thor.tha...@linux.intel.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Westergreen, Dalon <dalon.westergr...@intel.com>;
> Tan, Ley Foon <ley.foon....@intel.com>; See, Chin Liang
> <chin.liang....@intel.com>; Nguyen, Dinh <dinh.ngu...@intel.com>
> Subject: Re: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores
> tx_buffer
> call response
>
> From: Joyce Ooi <joyce....@intel.com>
> Date: Mon, 4 May 2020 16:25:49 +0800
>
> > The return from tx_buffer call in tse_start_xmit is inapropriately
> > ignored. tse_buffer calls should return
> > 0 for success or NETDEV_TX_BUSY. tse_start_xmit should return not
> > report a successful transmit when the tse_buffer call returns an error
> > condition.
>
> From driver.txt:
>
> ====================
> 1) The ndo_start_xmit method must not return NETDEV_TX_BUSY under
> any normal circumstances. It is considered a hard error unless
> there is no way your device can tell ahead of time when it's
> transmit function will become busy.
> ====================
>
> The problem is that when you return this error code, something has to trigger
> restarting the transmit queue to start sending packets to your device again.
> The
> usual mechanism is waking the transmit queue, but it's obviously already awake
> since your transmit routine is being called. Therefore nothing will reliably
> restart
> the queue when you return this error code.
>
> The best thing to do honestly is to drop the packet and return NETDEV_TX_OK,
> meanwhile bumping a statistic counter to record this event.
My change is similar to this hard error mentioned in drvier.txt:
/* This is a hard error log it. */
if (TX_BUFFS_AVAIL(dp) <= (skb_shinfo(skb)->nr_frags + 1)) {
netif_stop_queue(dev);
unlock_tx(dp);
printk(KERN_ERR PFX "%s: BUG! Tx Ring full when queue awake!\n",
dev->name);
return NETDEV_TX_BUSY;
}
So, before returning NETDEV_TX_BUSY, I can stop the queue first by calling
netif_stop_queue().
