RE: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores tx_buffer call response

[Ooi, Joyce]

> -----Original Message-----
> From: David Miller <davem@xxxxxxxxxxxxx>
> Sent: Tuesday, May 5, 2020 1:40 AM
> To: Ooi, Joyce <joyce.ooi@xxxxxxxxx>
> Cc: thor.thayer@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Westergreen, Dalon <dalon.westergreen@xxxxxxxxx>;
> Tan, Ley Foon <ley.foon.tan@xxxxxxxxx>; See, Chin Liang
> <chin.liang.see@xxxxxxxxx>; Nguyen, Dinh <dinh.nguyen@xxxxxxxxx>
> Subject: Re: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores tx_buffer
> call response
> 
> From: Joyce Ooi <joyce.ooi@xxxxxxxxx>
> 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().