Re: [PATCH net-next] Add support for netvsc build without CONFIG_SYSFS flag

2014-05-08 Thread Greg KH
On Wed, May 07, 2014 at 03:45:04PM -0700, Haiyang Zhang wrote:
 This change ensures the driver can be built successfully without the
 CONFIG_SYSFS flag.
 MS-TFS: 182270
 
 Signed-off-by: Haiyang Zhang haiya...@microsoft.com
 Reviewed-by: K. Y. Srinivasan k...@microsoft.com
 ---
  drivers/net/hyperv/hyperv_net.h   |2 ++
  drivers/net/hyperv/netvsc_drv.c   |   12 +++-
  drivers/net/hyperv/rndis_filter.c |4 ++--
  3 files changed, 15 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
 index 4b7df5a..23b96bc 100644
 --- a/drivers/net/hyperv/hyperv_net.h
 +++ b/drivers/net/hyperv/hyperv_net.h
 @@ -87,6 +87,8 @@ struct ndis_recv_scale_cap { /* 
 NDIS_RECEIVE_SCALE_CAPABILITIES */
  #define HASH_KEYLEN NDIS_RSS_HASH_SECRET_KEY_MAX_SIZE_REVISION_2
  extern u8 netvsc_hash_key[];
  
 +extern unsigned int netvsc_num_queue;
 +
  struct ndis_recv_scale_param { /* NDIS_RECEIVE_SCALE_PARAMETERS */
   struct ndis_obj_header hdr;
  
 diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
 index 939e3af..07896f3 100644
 --- a/drivers/net/hyperv/netvsc_drv.c
 +++ b/drivers/net/hyperv/netvsc_drv.c
 @@ -52,6 +52,8 @@ static int ring_size = 128;
  module_param(ring_size, int, S_IRUGO);
  MODULE_PARM_DESC(ring_size, Ring buffer size (# of pages));
  
 +unsigned int netvsc_num_queue = 1;
 +
  static void do_set_multicast(struct work_struct *w)
  {
   struct net_device_context *ndevctx =
 @@ -639,9 +641,11 @@ int netvsc_recv_callback(struct hv_device *device_obj,
   __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
  packet-vlan_tci);
  
 +#ifdef CONFIG_SYSFS
   skb_record_rx_queue(skb, packet-channel-
   offermsg.offer.sub_channel_index %
   net-real_num_rx_queues);
 +#endif

Why is this a sysfs-only function?  It should have nothing to do with
sysfs, right?  So why would the driver care?

Don't put #ifdef in drivers, it's unmaintainable over time, if this
really is a sysfs-only function, this should already be handled
automatically in a .h file somewhere.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH net-next] Add support for netvsc build without CONFIG_SYSFS flag

2014-05-08 Thread Haiyang Zhang


 -Original Message-
 From: Greg KH [mailto:g...@kroah.com]
 Sent: Thursday, May 8, 2014 5:13 AM
 To: Haiyang Zhang
 Cc: da...@davemloft.net; net...@vger.kernel.org; o...@aepfle.de;
 jasow...@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
 ker...@vger.kernel.org
 Subject: Re: [PATCH net-next] Add support for netvsc build without
 CONFIG_SYSFS flag
 
 On Wed, May 07, 2014 at 03:45:04PM -0700, Haiyang Zhang wrote:
  This change ensures the driver can be built successfully without the
  CONFIG_SYSFS flag.
  MS-TFS: 182270
 
  Signed-off-by: Haiyang Zhang haiya...@microsoft.com
  Reviewed-by: K. Y. Srinivasan k...@microsoft.com
  ---
   drivers/net/hyperv/hyperv_net.h   |2 ++
   drivers/net/hyperv/netvsc_drv.c   |   12 +++-
   drivers/net/hyperv/rndis_filter.c |4 ++--
   3 files changed, 15 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/net/hyperv/hyperv_net.h
  b/drivers/net/hyperv/hyperv_net.h index 4b7df5a..23b96bc 100644
  --- a/drivers/net/hyperv/hyperv_net.h
  +++ b/drivers/net/hyperv/hyperv_net.h
  @@ -87,6 +87,8 @@ struct ndis_recv_scale_cap { /*
  NDIS_RECEIVE_SCALE_CAPABILITIES */  #define HASH_KEYLEN
  NDIS_RSS_HASH_SECRET_KEY_MAX_SIZE_REVISION_2
   extern u8 netvsc_hash_key[];
 
  +extern unsigned int netvsc_num_queue;
  +
   struct ndis_recv_scale_param { /* NDIS_RECEIVE_SCALE_PARAMETERS */
  struct ndis_obj_header hdr;
 
  diff --git a/drivers/net/hyperv/netvsc_drv.c
  b/drivers/net/hyperv/netvsc_drv.c index 939e3af..07896f3 100644
  --- a/drivers/net/hyperv/netvsc_drv.c
  +++ b/drivers/net/hyperv/netvsc_drv.c
  @@ -52,6 +52,8 @@ static int ring_size = 128;  module_param(ring_size,
  int, S_IRUGO);  MODULE_PARM_DESC(ring_size, Ring buffer size (# of
  pages));
 
  +unsigned int netvsc_num_queue = 1;
  +
   static void do_set_multicast(struct work_struct *w)  {
  struct net_device_context *ndevctx = @@ -639,9 +641,11 @@ int
  netvsc_recv_callback(struct hv_device *device_obj,
  __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
 packet-vlan_tci);
 
  +#ifdef CONFIG_SYSFS
  skb_record_rx_queue(skb, packet-channel-
  offermsg.offer.sub_channel_index %
  net-real_num_rx_queues);
  +#endif
 
 Why is this a sysfs-only function?  It should have nothing to do with sysfs, 
 right?
 So why would the driver care?

Without CONFIG_SYSFS the multi-receive queues are not allocated, see below, and 
the variable real_num_rx_queues is not defined, so we skip this function.

In alloc_netdev_mqs(), file net/core/dev.c:
#ifdef CONFIG_SYSFS
dev-num_rx_queues = rxqs;
dev-real_num_rx_queues = rxqs;
if (netif_alloc_rx_queues(dev))
goto free_all;
#endif

 Don't put #ifdef in drivers, it's unmaintainable over time, if this really is 
 a sysfs-
 only function, this should already be handled automatically in a .h file
 somewhere.

OK, I will move the #ifdef into hyperv_net.h.

Thanks,
- Haiyang

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH net-next] Add support for netvsc build without CONFIG_SYSFS flag

2014-05-08 Thread Haiyang Zhang


 -Original Message-
 From: Greg KH [mailto:g...@kroah.com]
 Sent: Thursday, May 8, 2014 3:07 PM
 To: Haiyang Zhang
 Cc: da...@davemloft.net; net...@vger.kernel.org; o...@aepfle.de;
 jasow...@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
 ker...@vger.kernel.org
 Subject: Re: [PATCH net-next] Add support for netvsc build without
 CONFIG_SYSFS flag
 
 On Thu, May 08, 2014 at 03:13:19PM +, Haiyang Zhang wrote:
 
 
   -Original Message-
   From: Greg KH [mailto:g...@kroah.com]
   Sent: Thursday, May 8, 2014 5:13 AM
   To: Haiyang Zhang
   Cc: da...@davemloft.net; net...@vger.kernel.org; o...@aepfle.de;
   jasow...@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
   ker...@vger.kernel.org
   Subject: Re: [PATCH net-next] Add support for netvsc build without
   CONFIG_SYSFS flag
  
   On Wed, May 07, 2014 at 03:45:04PM -0700, Haiyang Zhang wrote:
This change ensures the driver can be built successfully without
the CONFIG_SYSFS flag.
MS-TFS: 182270
   
Signed-off-by: Haiyang Zhang haiya...@microsoft.com
Reviewed-by: K. Y. Srinivasan k...@microsoft.com
---
 drivers/net/hyperv/hyperv_net.h   |2 ++
 drivers/net/hyperv/netvsc_drv.c   |   12 +++-
 drivers/net/hyperv/rndis_filter.c |4 ++--
 3 files changed, 15 insertions(+), 3 deletions(-)
   
diff --git a/drivers/net/hyperv/hyperv_net.h
b/drivers/net/hyperv/hyperv_net.h index 4b7df5a..23b96bc 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -87,6 +87,8 @@ struct ndis_recv_scale_cap { /*
NDIS_RECEIVE_SCALE_CAPABILITIES */  #define HASH_KEYLEN
NDIS_RSS_HASH_SECRET_KEY_MAX_SIZE_REVISION_2
 extern u8 netvsc_hash_key[];
   
+extern unsigned int netvsc_num_queue;
+
 struct ndis_recv_scale_param { /* NDIS_RECEIVE_SCALE_PARAMETERS
 */
struct ndis_obj_header hdr;
   
diff --git a/drivers/net/hyperv/netvsc_drv.c
b/drivers/net/hyperv/netvsc_drv.c index 939e3af..07896f3 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -52,6 +52,8 @@ static int ring_size = 128;
module_param(ring_size, int, S_IRUGO);
MODULE_PARM_DESC(ring_size, Ring buffer size (# of pages));
   
+unsigned int netvsc_num_queue = 1;
+
 static void do_set_multicast(struct work_struct *w)  {
struct net_device_context *ndevctx = @@ -639,9 +641,11 @@ int
netvsc_recv_callback(struct hv_device *device_obj,
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
   packet-vlan_tci);
   
+#ifdef CONFIG_SYSFS
skb_record_rx_queue(skb, packet-channel-
offermsg.offer.sub_channel_index %
net-real_num_rx_queues);
+#endif
  
   Why is this a sysfs-only function?  It should have nothing to do with 
   sysfs,
 right?
   So why would the driver care?
 
  Without CONFIG_SYSFS the multi-receive queues are not allocated, see
  below, and the variable real_num_rx_queues is not defined, so we skip this
 function.
 
  In alloc_netdev_mqs(), file net/core/dev.c:
  #ifdef CONFIG_SYSFS
  dev-num_rx_queues = rxqs;
  dev-real_num_rx_queues = rxqs;
  if (netif_alloc_rx_queues(dev))
  goto free_all;
  #endif
 
 Then you need to structure your driver better :)

I have submitted an update patch.

Thanks,
- Haiyang

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel