Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
From: Sergei Shtylyov Date: Thu, 20 Mar 2014 00:56:05 +0300 > Hello. > > On 03/19/2014 11:40 PM, David Miller wrote: > >>> +extern u8 netvsc_hash_key[]; > >> We no longer use extern in header file function declarations. > >Wait, this is a variable declaration. :-) My bad :) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
> -Original Message- > > + > > + > > +/* ndis_recv_scale_cap/cap_flag */ > > One empty line is sufficient, we don't need two of them here. I will fix this and other formats you pointed out below, also check the whole patch for similar issues. > > - net = alloc_etherdev(sizeof(struct net_device_context)); > > + net = alloc_etherdev_mq(sizeof(struct net_device_context), > > + num_online_cpus()); > > if (!net) > > return -ENOMEM; > > > > num_online_cpus() can change, will your driver accomodate this? HyperV currently doesn't support hot-add/remove cpu, so this is constant for this driver. Thanks, - Haiyang -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
On Thu, 20 Mar 2014 00:56:05 +0300 Sergei Shtylyov wrote: > > On 03/19/2014 11:40 PM, David Miller wrote: > > >> +extern u8 netvsc_hash_key[]; > > > We no longer use extern in header file function declarations. > > Wait, this is a variable declaration. :-) And the need for the extern on variable declarations was one reason I gave for not removing it from the function declarations (consistency is good :-)). -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpPvvKuoqFZW.pgp Description: PGP signature
Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
Hello. On 03/19/2014 11:40 PM, David Miller wrote: +extern u8 netvsc_hash_key[]; We no longer use extern in header file function declarations. Wait, this is a variable declaration. :-) WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
From: Haiyang Zhang
Date: Tue, 18 Mar 2014 15:15:22 -0700
> +struct ndis_obj_header {
> + u8 type;
> + u8 rev;
> + u16 size;
> +} __packed;
> +
> +
> +/* ndis_recv_scale_cap/cap_flag */
One empty line is sufficient, we don't need two of them here.
> +extern u8 netvsc_hash_key[];
We no longer use extern in header file function declarations.
> + u32 processor_masks_offset;
> + u32 num_processor_masks;
> + u32 processor_masks_entry_size;
> +};
> +
> +
> /* Fwd declaration */
Again, one empty line is enough.
> @@ -120,6 +217,7 @@ void netvsc_linkstatus_callback(struct hv_device
> *device_obj,
> int netvsc_recv_callback(struct hv_device *device_obj,
> struct hv_netvsc_packet *packet,
> struct ndis_tcp_ip_checksum_info *csum_info);
> +extern void netvsc_channel_cb(void *context);
Again, please remove this extern in function declarations.
> + if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) &&
> + !net_device->start_remove &&
> + (hv_ringbuf_avail_percent(>outbound) >
> + RING_AVAIL_PERCENT_HIWATER || queue_sends < 1))
For the second time, this is not properly indented. Multi-line
conditionals should be formatted like this:
if (CONDITION1 OPERATOR
CONDITION2 OPERATOR
...)
etc. For example, this specifically should be:
if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) &&
!net_device->start_remove &&
(hv_ringbuf_avail_percent(>outbound) >
RING_AVAIL_PERCENT_HIWATER || queue_sends < 1))
Generally speaking, if you are only using TAB characters to indent,
you are probably doing it wrong.
You should use the appropriate number of TAB _and_ SPACE characters
necessary to start the line at exactly the first column after the
openning parenthesis of the appropriate construct.
> @@ -668,9 +756,11 @@ static int netvsc_probe(struct hv_device *dev,
> struct net_device *net = NULL;
> struct net_device_context *net_device_ctx;
> struct netvsc_device_info device_info;
> + struct netvsc_device *nvdev;
> int ret;
>
> - net = alloc_etherdev(sizeof(struct net_device_context));
> + net = alloc_etherdev_mq(sizeof(struct net_device_context),
> + num_online_cpus());
> if (!net)
> return -ENOMEM;
>
num_online_cpus() can change, will your driver accomodate this?
> +}
> +
> +
> static int rndis_filter_query_device_link_status(struct rndis_device *dev)
One empty line is sufficient.
> }
>
> +
> +static void netvsc_sc_open(struct vmbus_channel *new_sc)
Likewise.
> + ret = vmbus_open(new_sc, nvscdev->ring_size * PAGE_SIZE,
> + nvscdev->ring_size * PAGE_SIZE, NULL, 0,
> + netvsc_channel_cb, new_sc);
When function arguments span multiple lines the function call
shall be indented as:
func(A, B, C,
D, E, F);
That is, the second and subsequent lines are indented, using
the appropriate number of TAB _and_ SPACE characters, necessary
to reach exactly the first column after the openning parenthesis.
There are several other cases of this, please audit your entire
submission for this problem.
If proper indentation causes lines to significantly exceed 80
columns, consider helper functions or local variables to
allieve the problem.
> + ret = rndis_filter_query_device(rndis_device,
> + OID_GEN_RECEIVE_SCALE_CAPABILITIES, , _size);
Same problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
From: Haiyang Zhang haiya...@microsoft.com
Date: Tue, 18 Mar 2014 15:15:22 -0700
+struct ndis_obj_header {
+ u8 type;
+ u8 rev;
+ u16 size;
+} __packed;
+
+
+/* ndis_recv_scale_cap/cap_flag */
One empty line is sufficient, we don't need two of them here.
+extern u8 netvsc_hash_key[];
We no longer use extern in header file function declarations.
+ u32 processor_masks_offset;
+ u32 num_processor_masks;
+ u32 processor_masks_entry_size;
+};
+
+
/* Fwd declaration */
Again, one empty line is enough.
@@ -120,6 +217,7 @@ void netvsc_linkstatus_callback(struct hv_device
*device_obj,
int netvsc_recv_callback(struct hv_device *device_obj,
struct hv_netvsc_packet *packet,
struct ndis_tcp_ip_checksum_info *csum_info);
+extern void netvsc_channel_cb(void *context);
Again, please remove this extern in function declarations.
+ if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx))
+ !net_device-start_remove
+ (hv_ringbuf_avail_percent(channel-outbound)
+ RING_AVAIL_PERCENT_HIWATER || queue_sends 1))
For the second time, this is not properly indented. Multi-line
conditionals should be formatted like this:
if (CONDITION1 OPERATOR
CONDITION2 OPERATOR
...)
etc. For example, this specifically should be:
if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx))
!net_device-start_remove
(hv_ringbuf_avail_percent(channel-outbound)
RING_AVAIL_PERCENT_HIWATER || queue_sends 1))
Generally speaking, if you are only using TAB characters to indent,
you are probably doing it wrong.
You should use the appropriate number of TAB _and_ SPACE characters
necessary to start the line at exactly the first column after the
openning parenthesis of the appropriate construct.
@@ -668,9 +756,11 @@ static int netvsc_probe(struct hv_device *dev,
struct net_device *net = NULL;
struct net_device_context *net_device_ctx;
struct netvsc_device_info device_info;
+ struct netvsc_device *nvdev;
int ret;
- net = alloc_etherdev(sizeof(struct net_device_context));
+ net = alloc_etherdev_mq(sizeof(struct net_device_context),
+ num_online_cpus());
if (!net)
return -ENOMEM;
num_online_cpus() can change, will your driver accomodate this?
+}
+
+
static int rndis_filter_query_device_link_status(struct rndis_device *dev)
One empty line is sufficient.
}
+
+static void netvsc_sc_open(struct vmbus_channel *new_sc)
Likewise.
+ ret = vmbus_open(new_sc, nvscdev-ring_size * PAGE_SIZE,
+ nvscdev-ring_size * PAGE_SIZE, NULL, 0,
+ netvsc_channel_cb, new_sc);
When function arguments span multiple lines the function call
shall be indented as:
func(A, B, C,
D, E, F);
That is, the second and subsequent lines are indented, using
the appropriate number of TAB _and_ SPACE characters, necessary
to reach exactly the first column after the openning parenthesis.
There are several other cases of this, please audit your entire
submission for this problem.
If proper indentation causes lines to significantly exceed 80
columns, consider helper functions or local variables to
allieve the problem.
+ ret = rndis_filter_query_device(rndis_device,
+ OID_GEN_RECEIVE_SCALE_CAPABILITIES, rsscap, rsscap_size);
Same problem.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
Hello. On 03/19/2014 11:40 PM, David Miller wrote: +extern u8 netvsc_hash_key[]; We no longer use extern in header file function declarations. Wait, this is a variable declaration. :-) WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
On Thu, 20 Mar 2014 00:56:05 +0300 Sergei Shtylyov sergei.shtyl...@cogentembedded.com wrote: On 03/19/2014 11:40 PM, David Miller wrote: +extern u8 netvsc_hash_key[]; We no longer use extern in header file function declarations. Wait, this is a variable declaration. :-) And the need for the extern on variable declarations was one reason I gave for not removing it from the function declarations (consistency is good :-)). -- Cheers, Stephen Rothwells...@canb.auug.org.au pgpPvvKuoqFZW.pgp Description: PGP signature
RE: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
-Original Message- + + +/* ndis_recv_scale_cap/cap_flag */ One empty line is sufficient, we don't need two of them here. I will fix this and other formats you pointed out below, also check the whole patch for similar issues. - net = alloc_etherdev(sizeof(struct net_device_context)); + net = alloc_etherdev_mq(sizeof(struct net_device_context), + num_online_cpus()); if (!net) return -ENOMEM; num_online_cpus() can change, will your driver accomodate this? HyperV currently doesn't support hot-add/remove cpu, so this is constant for this driver. Thanks, - Haiyang -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next,v3] hyperv: Add support for virtual Receive Side Scaling (vRSS)
From: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Date: Thu, 20 Mar 2014 00:56:05 +0300 Hello. On 03/19/2014 11:40 PM, David Miller wrote: +extern u8 netvsc_hash_key[]; We no longer use extern in header file function declarations. Wait, this is a variable declaration. :-) My bad :) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
