Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: HrjuwcHloNRbAd3zNftqDzMMT8saE4++o85RMGWhTDJOnPhrY1buuCEaL2gBd7faZZfSOUPCXY
 28mJP4GxNamQ==
IronPort-SDR: s6FRpRj89M396XsE4yZR/hrr04ao7pJ1lEGjCGn70zUoLNSqv1iOpU7i3IoAG4eosa8UfnpeLq
 WMJNPT6LRm7Q==
From:   "Ooi, Joyce" <joyce.ooi@intel.com>
To:     David Miller <davem@davemloft.net>
CC:     "thor.thayer@linux.intel.com" <thor.thayer@linux.intel.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@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
Thread-Topic: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores
 tx_buffer call response
Thread-Index: AQHWIe38IzYWwFsQS0ypU6jURd+N3KiYMheAgAECGmA=
Date:   Tue, 5 May 2020 09:27:13 +0000
Message-ID: <BL0PR11MB305783B621110C7A38675659F2A70@BL0PR11MB3057.namprd11.prod.outlook.com>
References: <20200504082558.112627-1-joyce.ooi@intel.com>
        <20200504082558.112627-2-joyce.ooi@intel.com>
 <20200504.103935.1584665284135386530.davem@davemloft.net>
In-Reply-To: <20200504.103935.1584665284135386530.davem@davemloft.net>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.2.0.6
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: d8075056-b820-4b0c-efae-08d7f0d67ee2
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 May 2020 09:27:13.7134
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 9ISbxG5cpBpGLi5LNb1L5ZgtctpbloJlPhBDPoAC/pNsL4QdGDYiesvkjdROonw+sY3U/OIEP0ucWwQ+Dgjd8g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR11MB3345
X-OriginatorOrg: intel.com
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----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
>=20
> From: Joyce Ooi <joyce.ooi@intel.com>
> Date: Mon,  4 May 2020 16:25:49 +0800
>=20
> > 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.
>=20
> From driver.txt:
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> 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.
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> The problem is that when you return this error code, something has to tri=
gger
> restarting the transmit queue to start sending packets to your device aga=
in.  The
> usual mechanism is waking the transmit queue, but it's obviously already =
awake
> since your transmit routine is being called.  Therefore nothing will reli=
ably restart
> the queue when you return this error code.
>=20
> 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) <=3D (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().