[PATCH net 0/3] netvsc bug fixes

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

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 <redacted>
---
 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

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 <redacted>
---
 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

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 <redacted>
---
 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

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:

quoted

drivers/net//hyperv/netvsc_drv.c:84:2: warning: data definition has no type or storage class

     rtnl_unlock();
     ^~~~~~~~~~~

quoted

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);
                ^~~~~~~~~~~

quoted

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

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':

quoted

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

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon,  5 Jun 2017 14:10:10 -0700

Stephen, please at least compile test your code.

This is getting rediculous.