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;
Date:   Mon, 04 May 2020 10:39:35 -0700 (PDT)
Message-Id: <20200504.103935.1584665284135386530.davem@davemloft.net>
To:     joyce.ooi@intel.com
Cc:     thor.thayer@linux.intel.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, dalon.westergreen@intel.com,
        ley.foon.tan@intel.com, chin.liang.see@intel.com,
        dinh.nguyen@intel.com
Subject: Re: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores
 tx_buffer call response
From:   David Miller <davem@davemloft.net>
In-Reply-To: <20200504082558.112627-2-joyce.ooi@intel.com>
References: <20200504082558.112627-1-joyce.ooi@intel.com>
        <20200504082558.112627-2-joyce.ooi@intel.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.