> -----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; netdev@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().