* [PATCH net 0/3] netvsc bug fixes
@ 2017-06-05 21:10 Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 1/3] netvsc: fix rcu dereference warning from ethtool Stephen Hemminger
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Stephen Hemminger @ 2017-06-05 21:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Stephen Hemminger
These are non-critical bug fixes to the 4.12 version of netvsc
network driver. Two fix RCU splat warnings, and the other fixes
long standing issue with net poll. I don't think anyone has run
into the net poll issue before, so not worth incorporating into
stable.
Stephen Hemminger (3):
netvsc: fix rcu dereference warning from ethtool
netvsc: fix net poll mode
netvsc: fix RCU warning from set_multicast
drivers/net/hyperv/netvsc_drv.c | 55 +++++++++++++++++++++++++----------------
1 file changed, 34 insertions(+), 21 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 1/3] netvsc: fix rcu dereference warning from ethtool
2017-06-05 21:10 [PATCH net 0/3] netvsc bug fixes Stephen Hemminger
@ 2017-06-05 21:10 ` Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 2/3] netvsc: fix net poll mode Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 3/3] netvsc: fix RCU warning from set_multicast Stephen Hemminger
2 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2017-06-05 21:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Stephen Hemminger
The ethtool info command calls the netvsc get_sset_count with RTNL
but not with RCU. Which causes warning:
drivers/net/hyperv/netvsc_drv.c:1010 suspicious rcu_dereference_check() usage!
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 4421a6d00375..d93e4da25fd2 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1028,7 +1028,7 @@ static const struct {
static int netvsc_get_sset_count(struct net_device *dev, int string_set)
{
struct net_device_context *ndc = netdev_priv(dev);
- struct netvsc_device *nvdev = rcu_dereference(ndc->nvdev);
+ struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
if (!nvdev)
return -ENODEV;
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 2/3] netvsc: fix net poll mode
2017-06-05 21:10 [PATCH net 0/3] netvsc bug fixes Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 1/3] netvsc: fix rcu dereference warning from ethtool Stephen Hemminger
@ 2017-06-05 21:10 ` Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 3/3] netvsc: fix RCU warning from set_multicast Stephen Hemminger
2 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2017-06-05 21:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Stephen Hemminger
The ndo_poll_controller function needs to schedule NAPI to pick
up arriving packets and send completions. Otherwise no data
will ever be received.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index d93e4da25fd2..252e5d52d17e 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1158,11 +1158,22 @@ netvsc_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
}
#ifdef CONFIG_NET_POLL_CONTROLLER
-static void netvsc_poll_controller(struct net_device *net)
+static void netvsc_poll_controller(struct net_device *dev)
{
- /* As netvsc_start_xmit() works synchronous we don't have to
- * trigger anything here.
- */
+ struct net_device_context *ndc = netdev_priv(dev);
+ struct netvsc_device *ndev;
+ int i;
+
+ rcu_read_lock();
+ ndev = rcu_dereference(ndc->nvdev);
+ if (ndev) {
+ for (i = 0; i < ndev->num_chn; i++) {
+ struct netvsc_channel *nvchan = &ndev->chan_table[i];
+
+ napi_schedule(&nvchan->napi);
+ }
+ }
+ rcu_read_unlock();
}
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 3/3] netvsc: fix RCU warning from set_multicast
2017-06-05 21:10 [PATCH net 0/3] netvsc bug fixes Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 1/3] netvsc: fix rcu dereference warning from ethtool Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 2/3] netvsc: fix net poll mode Stephen Hemminger
@ 2017-06-05 21:10 ` Stephen Hemminger
2017-06-06 5:17 ` kbuild test robot
` (2 more replies)
2 siblings, 3 replies; 7+ messages in thread
From: Stephen Hemminger @ 2017-06-05 21:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Stephen Hemminger
do_set_multicast runs in a work queue and therefore is not holding
RCU read lock. This causes:
WARNING: suspicious RCU usage
4.12.0-rc2-net-00284-ge23454766d55-dirty #2 Not tainted
-----------------------------
drivers/net/hyperv/netvsc_drv.c:65 suspicious rcu_dereference_check() usage!
Fix by acquiring rtnl_lock which is ok in the work queue context.
Better fix is to move do_set_mulitcast into rndis_filter.c but
that has more complex impacts, so hold off until net-next.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 252e5d52d17e..ba5c4d037b76 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -62,24 +62,26 @@ static void do_set_multicast(struct work_struct *w)
container_of(w, struct net_device_context, work);
struct hv_device *device_obj = ndevctx->device_ctx;
struct net_device *ndev = hv_get_drvdata(device_obj);
- struct netvsc_device *nvdev = rcu_dereference(ndevctx->nvdev);
+ struct netvsc_device *nvdev;
struct rndis_device *rdev;
- if (!nvdev)
- return;
-
- rdev = nvdev->extension;
- if (rdev == NULL)
- return;
-
- if (ndev->flags & IFF_PROMISC)
- rndis_filter_set_packet_filter(rdev,
- NDIS_PACKET_TYPE_PROMISCUOUS);
- else
- rndis_filter_set_packet_filter(rdev,
- NDIS_PACKET_TYPE_BROADCAST |
- NDIS_PACKET_TYPE_ALL_MULTICAST |
- NDIS_PACKET_TYPE_DIRECTED);
+ rtnl_lock();
+ nvdev = rtnl_dereference(ndevctx->nvdev);
+ if (nvdev)
+ rdev = nvdev->extension;
+
+ if (rdev) {
+ if (ndev->flags & IFF_PROMISC)
+ rndis_filter_set_packet_filter(rdev,
+ NDIS_PACKET_TYPE_PROMISCUOUS);
+ else
+ rndis_filter_set_packet_filter(rdev,
+ NDIS_PACKET_TYPE_BROADCAST |
+ NDIS_PACKET_TYPE_ALL_MULTICAST |
+ NDIS_PACKET_TYPE_DIRECTED);
+ }
+ }
+ rtnl_unlock();
}
static void netvsc_set_multicast_list(struct net_device *net)
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 3/3] netvsc: fix RCU warning from set_multicast
2017-06-05 21:10 ` [PATCH net 3/3] netvsc: fix RCU warning from set_multicast Stephen Hemminger
@ 2017-06-06 5:17 ` kbuild test robot
2017-06-06 5:24 ` kbuild test robot
2017-06-06 19:55 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2017-06-06 5:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: kbuild-all, davem, netdev, Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 2693 bytes --]
Hi Stephen,
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/Stephen-Hemminger/netvsc-bug-fixes/20170606-120730
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
drivers/net//hyperv/netvsc_drv.c: In function 'do_set_multicast':
drivers/net//hyperv/netvsc_drv.c:70:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (nvdev)
^~
drivers/net//hyperv/netvsc_drv.c:73:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
if (rdev) {
^~
drivers/net//hyperv/netvsc_drv.c: At top level:
>> drivers/net//hyperv/netvsc_drv.c:84:2: warning: data definition has no type or storage class
rtnl_unlock();
^~~~~~~~~~~
>> drivers/net//hyperv/netvsc_drv.c:84:2: error: type defaults to 'int' in declaration of 'rtnl_unlock' [-Werror=implicit-int]
>> drivers/net//hyperv/netvsc_drv.c:84:2: error: function declaration isn't a prototype [-Werror=strict-prototypes]
>> drivers/net//hyperv/netvsc_drv.c:84:2: error: conflicting types for 'rtnl_unlock'
In file included from include/linux/inetdevice.h:13:0,
from drivers/net//hyperv/netvsc_drv.c:30:
include/linux/rtnetlink.h:28:13: note: previous declaration of 'rtnl_unlock' was here
extern void rtnl_unlock(void);
^~~~~~~~~~~
>> drivers/net//hyperv/netvsc_drv.c:85:1: error: expected identifier or '(' before '}' token
}
^
cc1: some warnings being treated as errors
vim +84 drivers/net//hyperv/netvsc_drv.c
64 struct net_device *ndev = hv_get_drvdata(device_obj);
65 struct netvsc_device *nvdev;
66 struct rndis_device *rdev;
67
68 rtnl_lock();
69 nvdev = rtnl_dereference(ndevctx->nvdev);
> 70 if (nvdev)
71 rdev = nvdev->extension;
72
> 73 if (rdev) {
74 if (ndev->flags & IFF_PROMISC)
75 rndis_filter_set_packet_filter(rdev,
76 NDIS_PACKET_TYPE_PROMISCUOUS);
77 else
78 rndis_filter_set_packet_filter(rdev,
79 NDIS_PACKET_TYPE_BROADCAST |
80 NDIS_PACKET_TYPE_ALL_MULTICAST |
81 NDIS_PACKET_TYPE_DIRECTED);
82 }
83 }
> 84 rtnl_unlock();
> 85 }
86
87 static void netvsc_set_multicast_list(struct net_device *net)
88 {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38856 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 3/3] netvsc: fix RCU warning from set_multicast
2017-06-05 21:10 ` [PATCH net 3/3] netvsc: fix RCU warning from set_multicast Stephen Hemminger
2017-06-06 5:17 ` kbuild test robot
@ 2017-06-06 5:24 ` kbuild test robot
2017-06-06 19:55 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2017-06-06 5:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: kbuild-all, davem, netdev, Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 2799 bytes --]
Hi Stephen,
[auto build test WARNING on net/master]
url: https://github.com/0day-ci/linux/commits/Stephen-Hemminger/netvsc-bug-fixes/20170606-120730
config: x86_64-randconfig-x007-201723 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/net//hyperv/netvsc_drv.c: In function 'do_set_multicast':
>> drivers/net//hyperv/netvsc_drv.c:70:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (nvdev)
^~
drivers/net//hyperv/netvsc_drv.c:73:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
if (rdev) {
^~
drivers/net//hyperv/netvsc_drv.c: At top level:
drivers/net//hyperv/netvsc_drv.c:84:2: warning: data definition has no type or storage class
rtnl_unlock();
^~~~~~~~~~~
drivers/net//hyperv/netvsc_drv.c:84:2: error: type defaults to 'int' in declaration of 'rtnl_unlock' [-Werror=implicit-int]
drivers/net//hyperv/netvsc_drv.c:84:2: error: function declaration isn't a prototype [-Werror=strict-prototypes]
drivers/net//hyperv/netvsc_drv.c:84:2: error: conflicting types for 'rtnl_unlock'
In file included from include/linux/inetdevice.h:13:0,
from drivers/net//hyperv/netvsc_drv.c:30:
include/linux/rtnetlink.h:28:13: note: previous declaration of 'rtnl_unlock' was here
extern void rtnl_unlock(void);
^~~~~~~~~~~
drivers/net//hyperv/netvsc_drv.c:85:1: error: expected identifier or '(' before '}' token
}
^
cc1: some warnings being treated as errors
vim +/if +70 drivers/net//hyperv/netvsc_drv.c
54
55 static int debug = -1;
56 module_param(debug, int, S_IRUGO);
57 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
58
59 static void do_set_multicast(struct work_struct *w)
60 {
61 struct net_device_context *ndevctx =
62 container_of(w, struct net_device_context, work);
63 struct hv_device *device_obj = ndevctx->device_ctx;
64 struct net_device *ndev = hv_get_drvdata(device_obj);
65 struct netvsc_device *nvdev;
66 struct rndis_device *rdev;
67
68 rtnl_lock();
69 nvdev = rtnl_dereference(ndevctx->nvdev);
> 70 if (nvdev)
71 rdev = nvdev->extension;
72
73 if (rdev) {
74 if (ndev->flags & IFF_PROMISC)
75 rndis_filter_set_packet_filter(rdev,
76 NDIS_PACKET_TYPE_PROMISCUOUS);
77 else
78 rndis_filter_set_packet_filter(rdev,
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30708 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 3/3] netvsc: fix RCU warning from set_multicast
2017-06-05 21:10 ` [PATCH net 3/3] netvsc: fix RCU warning from set_multicast Stephen Hemminger
2017-06-06 5:17 ` kbuild test robot
2017-06-06 5:24 ` kbuild test robot
@ 2017-06-06 19:55 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2017-06-06 19:55 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 5 Jun 2017 14:10:10 -0700
> + nvdev = rtnl_dereference(ndevctx->nvdev);
> + if (nvdev)
> + rdev = nvdev->extension;
> +
> + if (rdev) {
> + if (ndev->flags & IFF_PROMISC)
> + rndis_filter_set_packet_filter(rdev,
> + NDIS_PACKET_TYPE_PROMISCUOUS);
> + else
> + rndis_filter_set_packet_filter(rdev,
> + NDIS_PACKET_TYPE_BROADCAST |
> + NDIS_PACKET_TYPE_ALL_MULTICAST |
> + NDIS_PACKET_TYPE_DIRECTED);
> + }
> + }
Stephen, please at least compile test your code.
This is getting rediculous.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-06 19:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-05 21:10 [PATCH net 0/3] netvsc bug fixes Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 1/3] netvsc: fix rcu dereference warning from ethtool Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 2/3] netvsc: fix net poll mode Stephen Hemminger
2017-06-05 21:10 ` [PATCH net 3/3] netvsc: fix RCU warning from set_multicast Stephen Hemminger
2017-06-06 5:17 ` kbuild test robot
2017-06-06 5:24 ` kbuild test robot
2017-06-06 19:55 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.