lkml.org 
[lkml]   [2020]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores tx_buffer call response
Date
> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Tuesday, May 5, 2020 1:40 AM
> To: Ooi, Joyce <joyce.ooi@intel.com>
> Cc: thor.thayer@linux.intel.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Westergreen, Dalon <dalon.westergreen@intel.com>;
> Tan, Ley Foon <ley.foon.tan@intel.com>; See, Chin Liang
> <chin.liang.see@intel.com>; Nguyen, Dinh <dinh.nguyen@intel.com>
> Subject: Re: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores tx_buffer
> call response
>
> From: Joyce Ooi <joyce.ooi@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().

\
 
 \ /
  Last update: 2020-05-05 11:28    [W:0.140 / U:0.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site