Netdev List
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: David Miller <davem@davemloft.net>, <yuehaibing@huawei.com>
Cc: <f.fainelli@gmail.com>, <w-kwok2@ti.com>, <m-karicheri2@ti.com>,
	<lukas@wunner.de>, <bgolaszewski@baylibre.com>,
	<ivan.khoronzhuk@linaro.org>, <dan.carpenter@oracle.com>,
	<linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
	<linux-omap@vger.kernel.org>
Subject: Re: [PATCH net-next] net: ti: fix return type of ndo_start_xmit function
Date: Wed, 26 Sep 2018 15:36:46 -0500	[thread overview]
Message-ID: <b24fa149-40b3-57fd-ccb0-78a10c97b825@ti.com> (raw)
In-Reply-To: <20180926.101733.2070254569725987062.davem@davemloft.net>

Hi All,

On 09/26/2018 12:17 PM, David Miller wrote:
> From: YueHaibing <yuehaibing@huawei.com>
> Date: Wed, 26 Sep 2018 17:09:51 +0800
> 
>> @@ -1290,7 +1291,7 @@ static int netcp_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>>   			dev_warn(netcp->ndev_dev, "padding failed (%d), packet dropped\n",
>>   				 ret);
>>   			tx_stats->tx_dropped++;
>> -			return ret;
>> +			return NETDEV_TX_BUSY;
>>   		}
>>   		skb->len = NETCP_MIN_PACKET_SIZE;
>>   	}
>> @@ -1298,7 +1299,6 @@ static int netcp_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>>   	desc = netcp_tx_map_skb(skb, netcp);
>>   	if (unlikely(!desc)) {
>>   		netif_stop_subqueue(ndev, subqueue);
>> -		ret = -ENOBUFS;
>>   		goto drop;
>>   	}
>>   
>> @@ -1319,7 +1319,7 @@ static int netcp_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>>   	if (desc)
>>   		netcp_free_tx_desc_chain(netcp, desc, sizeof(*desc));
>>   	dev_kfree_skb(skb);
>> -	return ret;
>> +	return NETDEV_TX_BUSY;
>>   }
> 
> These conversions are not correct.
> 
> If the driver frees the SKB you must not return NETDEV_TX_BUSY.
> 
> NETDEV_TX_BUSY tells the caller that the driver could not process the
> packet and that it should reqeueu up the SKB and try again later when
> there is more TX queue room.
> 

Sry, but I still do not understand the reason for these changes (as I asked already [1])
May be there are some patches on the fly or long term decisions were made in this area.

According to the code include/linux/netdevice.h the .ndo_start_xmit() can return 3 type of values
1) enum netdev_tx, expected to be used by regular netdev drivers, but
2) "error while transmitting (rc < 0)" is also acceptable values
3) NET_XMIT_SUCCESS/DROP/CN (qdisc ->enqueue() return codes) for Virtual network devices 

So, are there plans to move NET_XMIT_XXX in enum? 
or any patches to change include/linux/netdevice.h "Transmit return codes:" section?

Not sure, that blindly following coccinelle recommendations is a good
choice in this particular case.

[1] https://lkml.org/lkml/2018/9/20/881

-- 
regards,
-grygorii

      reply	other threads:[~2018-09-26 20:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-26  9:09 [PATCH net-next] net: ti: fix return type of ndo_start_xmit function YueHaibing
2018-09-26 17:17 ` David Miller
2018-09-26 20:36   ` Grygorii Strashko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b24fa149-40b3-57fd-ccb0-78a10c97b825@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=dan.carpenter@oracle.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=ivan.khoronzhuk@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=m-karicheri2@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=w-kwok2@ti.com \
    --cc=yuehaibing@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox