| Message ID | 20180926090951.25544-1-yuehaibing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [net-next] net: ti: fix return type of ndo_start_xmit function | expand |
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.
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
diff --git a/drivers/net/ethernet/ti/cpmac.c b/drivers/net/ethernet/ti/cpmac.c index 9b8a30b..64c45eb 100644 --- a/drivers/net/ethernet/ti/cpmac.c +++ b/drivers/net/ethernet/ti/cpmac.c @@ -544,7 +544,7 @@ static int cpmac_poll(struct napi_struct *napi, int budget) } -static int cpmac_start_xmit(struct sk_buff *skb, struct net_device *dev) +static netdev_tx_t cpmac_start_xmit(struct sk_buff *skb, struct net_device *dev) { int queue; unsigned int len; diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c index f270bee..b83f32d 100644 --- a/drivers/net/ethernet/ti/davinci_emac.c +++ b/drivers/net/ethernet/ti/davinci_emac.c @@ -943,7 +943,7 @@ static void emac_tx_handler(void *token, int len, int status) * * Returns success(NETDEV_TX_OK) or error code (typically out of desc's) */ -static int emac_dev_xmit(struct sk_buff *skb, struct net_device *ndev) +static netdev_tx_t emac_dev_xmit(struct sk_buff *skb, struct net_device *ndev) { struct device *emac_dev = &ndev->dev; int ret_code; diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c index 1f61226..2d8cfe8 100644 --- a/drivers/net/ethernet/ti/netcp_core.c +++ b/drivers/net/ethernet/ti/netcp_core.c @@ -1270,7 +1270,8 @@ static int netcp_tx_submit_skb(struct netcp_intf *netcp, } /* Submit the packet */ -static int netcp_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev) +static netdev_tx_t +netcp_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev) { struct netcp_intf *netcp = netdev_priv(ndev); struct netcp_stats *tx_stats = &netcp->stats; @@ -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; } int netcp_txpipe_close(struct netcp_tx_pipe *tx_pipe)
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t', which is a typedef for an enum type, so make sure the implementation in this driver has returns 'netdev_tx_t' value, and change the function return type to netdev_tx_t. Found by coccinelle. Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/net/ethernet/ti/cpmac.c | 2 +- drivers/net/ethernet/ti/davinci_emac.c | 2 +- drivers/net/ethernet/ti/netcp_core.c | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-)