| Message ID | 20180920123306.14772-18-yuehaibing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: fix return type of ndo_start_xmit function | expand |
> -----Original Message----- > From: YueHaibing <yuehaibing@huawei.com> > Sent: Thursday, September 20, 2018 8:33 AM > To: davem@davemloft.net; dmitry.tarnyagin@lockless.no; > wg@grandegger.com; mkl@pengutronix.de; michal.simek@xilinx.com; > hsweeten@visionengravers.com; madalin.bucur@nxp.com; > pantelis.antoniou@gmail.com; claudiu.manoil@nxp.com; leoyang.li@nxp.com; > linux@armlinux.org.uk; sammy@sammy.net; ralf@linux-mips.org; > nico@fluxnic.net; steve.glendinning@shawell.net; f.fainelli@gmail.com; > grygorii.strashko@ti.com; w-kwok2@ti.com; m-karicheri2@ti.com; > t.sailer@alumni.ethz.ch; jreuter@yaina.de; KY Srinivasan <kys@microsoft.com>; > Haiyang Zhang <haiyangz@microsoft.com>; wei.liu2@citrix.com; > paul.durrant@citrix.com; arvid.brodin@alten.se; pshelar@ovn.org > Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; linux- > can@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linuxppc- > dev@lists.ozlabs.org; linux-mips@linux-mips.org; linux-omap@vger.kernel.org; > linux-hams@vger.kernel.org; devel@linuxdriverproject.org; linux- > usb@vger.kernel.org; xen-devel@lists.xenproject.org; dev@openvswitch.org; > YueHaibing <yuehaibing@huawei.com> > Subject: [PATCH net-next 17/22] hv_netvsc: fix return type of ndo_start_xmit > function > > 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/hyperv/netvsc_drv.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index 3af6d8d..056c472 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -511,7 +511,8 @@ static int netvsc_vf_xmit(struct net_device *net, struct > net_device *vf_netdev, > return rc; > } > > -static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) > +static netdev_tx_t > +netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) > { > struct net_device_context *net_device_ctx = netdev_priv(net); > struct hv_netvsc_packet *packet = NULL; @@ -528,8 +529,11 @@ > static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) > */ > vf_netdev = rcu_dereference_bh(net_device_ctx->vf_netdev); > if (vf_netdev && netif_running(vf_netdev) && > - !netpoll_tx_running(net)) > - return netvsc_vf_xmit(net, vf_netdev, skb); > + !netpoll_tx_running(net)) { > + ret = netvsc_vf_xmit(net, vf_netdev, skb); > + if (ret) > + return NETDEV_TX_BUSY; For error case, please just return NETDEV_TX_OK. We are not sure if the error can go away after retrying, returning NETDEV_TX_BUSY may cause infinite retry from the upper layer. Thanks, - Haiyang
On Thu, 20 Sep 2018 20:33:01 +0800 YueHaibing <yuehaibing@huawei.com> wrote: > 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/hyperv/netvsc_drv.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index 3af6d8d..056c472 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -511,7 +511,8 @@ static int netvsc_vf_xmit(struct net_device *net, struct net_device *vf_netdev, > return rc; > } > > -static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) > +static netdev_tx_t > +netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) > { > struct net_device_context *net_device_ctx = netdev_priv(net); > struct hv_netvsc_packet *packet = NULL; > @@ -528,8 +529,11 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) > */ > vf_netdev = rcu_dereference_bh(net_device_ctx->vf_netdev); > if (vf_netdev && netif_running(vf_netdev) && > - !netpoll_tx_running(net)) > - return netvsc_vf_xmit(net, vf_netdev, skb); > + !netpoll_tx_running(net)) { > + ret = netvsc_vf_xmit(net, vf_netdev, skb); > + if (ret) > + return NETDEV_TX_BUSY; > + } Sorry, the new code is wrong. It will fall through if ret == 0 (NETDEV_TX_OK) Please review and test your patches.
On 2018/9/20 22:43, Stephen Hemminger wrote: > On Thu, 20 Sep 2018 20:33:01 +0800 > YueHaibing <yuehaibing@huawei.com> wrote: > >> 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/hyperv/netvsc_drv.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c >> index 3af6d8d..056c472 100644 >> --- a/drivers/net/hyperv/netvsc_drv.c >> +++ b/drivers/net/hyperv/netvsc_drv.c >> @@ -511,7 +511,8 @@ static int netvsc_vf_xmit(struct net_device *net, struct net_device *vf_netdev, >> return rc; >> } >> >> -static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) >> +static netdev_tx_t >> +netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) >> { >> struct net_device_context *net_device_ctx = netdev_priv(net); >> struct hv_netvsc_packet *packet = NULL; >> @@ -528,8 +529,11 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) >> */ >> vf_netdev = rcu_dereference_bh(net_device_ctx->vf_netdev); >> if (vf_netdev && netif_running(vf_netdev) && >> - !netpoll_tx_running(net)) >> - return netvsc_vf_xmit(net, vf_netdev, skb); >> + !netpoll_tx_running(net)) { >> + ret = netvsc_vf_xmit(net, vf_netdev, skb); >> + if (ret) >> + return NETDEV_TX_BUSY; >> + } > > Sorry, the new code is wrong. It will fall through if ret == 0 (NETDEV_TX_OK) > Please review and test your patches. I'm sorry for this, will correct it as Haiyang's suggestion. > > . >
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 3af6d8d..056c472 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -511,7 +511,8 @@ static int netvsc_vf_xmit(struct net_device *net, struct net_device *vf_netdev, return rc; } -static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) +static netdev_tx_t +netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) { struct net_device_context *net_device_ctx = netdev_priv(net); struct hv_netvsc_packet *packet = NULL; @@ -528,8 +529,11 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) */ vf_netdev = rcu_dereference_bh(net_device_ctx->vf_netdev); if (vf_netdev && netif_running(vf_netdev) && - !netpoll_tx_running(net)) - return netvsc_vf_xmit(net, vf_netdev, skb); + !netpoll_tx_running(net)) { + ret = netvsc_vf_xmit(net, vf_netdev, skb); + if (ret) + return NETDEV_TX_BUSY; + } /* We will atmost need two pages to describe the rndis * header. We can only transmit MAX_PAGE_BUFFER_COUNT number
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/hyperv/netvsc_drv.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)