Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features
@ 2019-10-30 15:32 Haiyang Zhang
  2019-10-30 15:32 ` [PATCH net, 1/2] hv_netvsc: Fix error handling in netvsc_set_features() Haiyang Zhang
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Haiyang Zhang @ 2019-10-30 15:32 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	davem, linux-kernel

The error handling code path in these functions are not correct.
This patch set fixes them.

Haiyang Zhang (2):
  hv_netvsc: Fix error handling in netvsc_set_features()
  hv_netvsc: Fix error handling in netvsc_attach()

 drivers/net/hyperv/netvsc_drv.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net, 1/2] hv_netvsc: Fix error handling in netvsc_set_features()
  2019-10-30 15:32 [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features Haiyang Zhang
@ 2019-10-30 15:32 ` Haiyang Zhang
  2019-10-30 15:32 ` [PATCH net, 2/2] hv_netvsc: Fix error handling in netvsc_attach() Haiyang Zhang
  2019-10-31  1:17 ` [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: Haiyang Zhang @ 2019-10-30 15:32 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	davem, linux-kernel

When an error is returned by rndis_filter_set_offload_params(), we should
still assign the unaffected features to ndev->features. Otherwise, these
features will be missing.

Fixes: d6792a5a0747 ("hv_netvsc: Add handler for LRO setting change")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 39dddcd..734e411 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1807,8 +1807,10 @@ static int netvsc_set_features(struct net_device *ndev,
 
 	ret = rndis_filter_set_offload_params(ndev, nvdev, &offloads);
 
-	if (ret)
+	if (ret) {
 		features ^= NETIF_F_LRO;
+		ndev->features = features;
+	}
 
 syncvf:
 	if (!vf_netdev)
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net, 2/2] hv_netvsc: Fix error handling in netvsc_attach()
  2019-10-30 15:32 [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features Haiyang Zhang
  2019-10-30 15:32 ` [PATCH net, 1/2] hv_netvsc: Fix error handling in netvsc_set_features() Haiyang Zhang
@ 2019-10-30 15:32 ` Haiyang Zhang
  2019-11-01 20:14   ` Markus Elfring
  2019-10-31  1:17 ` [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features David Miller
  2 siblings, 1 reply; 5+ messages in thread
From: Haiyang Zhang @ 2019-10-30 15:32 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	davem, linux-kernel

If rndis_filter_open() fails, we need to remove the rndis device created
in earlier steps, before returning an error code. Otherwise, the retry of
netvsc_attach() from its callers will fail and hang.

Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 734e411..a14fc8e 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -982,7 +982,7 @@ static int netvsc_attach(struct net_device *ndev,
 	if (netif_running(ndev)) {
 		ret = rndis_filter_open(nvdev);
 		if (ret)
-			return ret;
+			goto err;
 
 		rdev = nvdev->extension;
 		if (!rdev->link_state)
@@ -990,6 +990,13 @@ static int netvsc_attach(struct net_device *ndev,
 	}
 
 	return 0;
+
+err:
+	netif_device_detach(ndev);
+
+	rndis_filter_device_remove(hdev, nvdev);
+
+	return ret;
 }
 
 static int netvsc_set_channels(struct net_device *net,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features
  2019-10-30 15:32 [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features Haiyang Zhang
  2019-10-30 15:32 ` [PATCH net, 1/2] hv_netvsc: Fix error handling in netvsc_set_features() Haiyang Zhang
  2019-10-30 15:32 ` [PATCH net, 2/2] hv_netvsc: Fix error handling in netvsc_attach() Haiyang Zhang
@ 2019-10-31  1:17 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2019-10-31  1:17 UTC (permalink / raw)
  To: haiyangz
  Cc: sashal, linux-hyperv, netdev, kys, sthemmin, olaf, vkuznets,
	linux-kernel

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Wed, 30 Oct 2019 15:32:05 +0000

> The error handling code path in these functions are not correct.
> This patch set fixes them.

Series applied, thank you.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net, 2/2] hv_netvsc: Fix error handling in netvsc_attach()
  2019-10-30 15:32 ` [PATCH net, 2/2] hv_netvsc: Fix error handling in netvsc_attach() Haiyang Zhang
@ 2019-11-01 20:14   ` Markus Elfring
  0 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2019-11-01 20:14 UTC (permalink / raw)
  To: Haiyang Zhang, linux-hyperv, netdev
  Cc: kernel-janitors, linux-kernel, David S. Miller, K. Y. Srinivasan,
	Olaf Hering, Sasha Levin, Stephen Hemminger, Vitaly Kuznetsov

…
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -982,7 +982,7 @@ static int netvsc_attach(struct net_device *ndev,
>  	if (netif_running(ndev)) {
>  		ret = rndis_filter_open(nvdev);
>  		if (ret)
> -			return ret;
> +			goto err;
>
>  		rdev = nvdev->extension;
>  		if (!rdev->link_state)
…

I would prefer to specify the completed exception handling
(addition of two function calls) by a compound statement in
the shown if branch directly.

If you would insist to use a goto statement, I find an other label
more appropriate according to the Linux coding style.

Regards,
Markus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 15:32 [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features Haiyang Zhang
2019-10-30 15:32 ` [PATCH net, 1/2] hv_netvsc: Fix error handling in netvsc_set_features() Haiyang Zhang
2019-10-30 15:32 ` [PATCH net, 2/2] hv_netvsc: Fix error handling in netvsc_attach() Haiyang Zhang
2019-11-01 20:14   ` Markus Elfring
2019-10-31  1:17 ` [PATCH net, 0/2] hv_netvsc: fix error handling in netvsc_attach/set_features David Miller

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org
	public-inbox-index linux-hyperv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git