Re: [PATCH 2/2 net-next] hyperv: Support batched notification

2015-03-12 Thread Olaf Hering
On Wed, Mar 11, KY Srinivasan wrote:

> This is the convention that we have used for patches submitted to
> David's tree.

I see, it refers to the directory name. In a bunch of backports it gives
the impression it affects the Hyper-V support as whole, while in fact a
given change is just for the network driver. Looks like I have to live
with that.

Olaf
--
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 2/2 net-next] hyperv: Support batched notification

2015-03-12 Thread Olaf Hering
On Wed, Mar 11, KY Srinivasan wrote:

 This is the convention that we have used for patches submitted to
 David's tree.

I see, it refers to the directory name. In a bunch of backports it gives
the impression it affects the Hyper-V support as whole, while in fact a
given change is just for the network driver. Looks like I have to live
with that.

Olaf
--
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 2/2 net-next] hyperv: Support batched notification

2015-03-11 Thread KY Srinivasan


> -Original Message-
> From: Olaf Hering [mailto:o...@aepfle.de]
> Sent: Wednesday, March 11, 2015 2:24 AM
> To: KY Srinivasan
> Cc: da...@davemloft.net; net...@vger.kernel.org;
> gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 2/2 net-next] hyperv: Support batched notification
> 
> On Tue, Mar 10, K. Y. Srinivasan wrote:
> 
> > Optimize notifying the host by deferring notification until there are
> > no more packets to be sent. This will help in batching the requests on
> > the host.
> 
> The subjects for the network driver say "hyperv:". But all such changes
> should get "netvsc:" as prefix to make it clear what each single patch is all
> about.
> 
> This should be changed for upcoming submissions.

Olaf,
This is the convention that we have used for patches submitted to David's tree. 
You will see
net-next in the subject line and you can key off of that.

K. Y
> 
> Thanks!
> 
> Olaf


RE: [PATCH 2/2 net-next] hyperv: Support batched notification

2015-03-11 Thread KY Srinivasan


> -Original Message-
> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Tuesday, March 10, 2015 8:34 PM
> To: KY Srinivasan
> Cc: da...@davemloft.net; net...@vger.kernel.org;
> gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
> Srinivasan
> Subject: Re: [PATCH 2/2 net-next] hyperv: Support batched notification
> 
> 
> 
> On Wed, Mar 11, 2015 at 2:50 AM, K. Y. Srinivasan 
> wrote:
> > Optimize notifying the host by deferring notification until there are
> > no more packets to be sent. This will help in batching the requests on
> > the host.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  drivers/net/hyperv/hyperv_net.h   |2 +-
> >  drivers/net/hyperv/netvsc.c   |   14 +-
> >  drivers/net/hyperv/netvsc_drv.c   |3 ++-
> >  drivers/net/hyperv/rndis_filter.c |2 +-
> >  4 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/hyperv_net.h
> > b/drivers/net/hyperv/hyperv_net.h index 4815843..3fd9896 100644
> > --- a/drivers/net/hyperv/hyperv_net.h
> > +++ b/drivers/net/hyperv/hyperv_net.h
> > @@ -184,7 +184,7 @@ struct rndis_device {  int
> > netvsc_device_add(struct hv_device *device, void *additional_info);
> > int netvsc_device_remove(struct hv_device *device);  int
> > netvsc_send(struct hv_device *device,
> > -   struct hv_netvsc_packet *packet);
> > +   struct hv_netvsc_packet *packet, bool kick_q);
> >  void netvsc_linkstatus_callback(struct hv_device *device_obj,
> > struct rndis_message *resp);
> >  int netvsc_recv_callback(struct hv_device *device_obj, diff --git
> > a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index
> > 208eb05..9003b94 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -707,7 +707,7 @@ static u32 netvsc_copy_to_send_buf(struct
> > netvsc_device *net_device,  }
> >
> >  int netvsc_send(struct hv_device *device,
> > -   struct hv_netvsc_packet *packet)
> > +   struct hv_netvsc_packet *packet, bool kick_q)
> >  {
> > struct netvsc_device *net_device;
> > int ret = 0;
> > @@ -719,6 +719,7 @@ int netvsc_send(struct hv_device *device,
> > u32 msg_size = 0;
> > struct sk_buff *skb = NULL;
> > u16 q_idx = packet->q_idx;
> > +   u32 vmbus_flags =
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
> >
> >
> > net_device = get_outbound_net_device(device); @@ -768,18
> +769,21 @@
> > int netvsc_send(struct hv_device *device,
> > return -ENODEV;
> >
> > if (packet->page_buf_cnt) {
> > -   ret = vmbus_sendpacket_pagebuffer(out_channel,
> > +   ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
> >   packet->page_buf,
> >   packet->page_buf_cnt,
> >   ,
> >   sizeof(struct
> nvsp_message),
> > - req_id);
> > + req_id,
> > + vmbus_flags,
> > + kick_q);
> 
> What if kick_q is false but ret is -EAGAIN here? Looks like in this case host
> won't get notified at all. How about checking whether txq and kicking if it 
> has
> been stopped like what other network driver did?

Good point. I am going to fix this issue in the VMBUS layer. The kick_q argument
is simply a hint to the vmbus level - the lower level can choose not to notify 
the
host (even if kick_q is true) based on other considerations.

I will resend this series with the logic in the vmbus driver. I will send the 
patch out
and Greg can decide if the vmbus change should go through Greg's tree or 
David's tree.

Regards,

K. Y
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH 2/2 net-next] hyperv: Support batched notification

2015-03-11 Thread Olaf Hering
On Tue, Mar 10, K. Y. Srinivasan wrote:

> Optimize notifying the host by deferring notification until there
> are no more packets to be sent. This will help in batching the requests
> on the host.

The subjects for the network driver say "hyperv:". But all such changes
should get "netvsc:" as prefix to make it clear what each single patch
is all about.

This should be changed for upcoming submissions.

Thanks!

Olaf
--
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 2/2 net-next] hyperv: Support batched notification

2015-03-11 Thread KY Srinivasan


 -Original Message-
 From: Jason Wang [mailto:jasow...@redhat.com]
 Sent: Tuesday, March 10, 2015 8:34 PM
 To: KY Srinivasan
 Cc: da...@davemloft.net; net...@vger.kernel.org;
 gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
 de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; KY
 Srinivasan
 Subject: Re: [PATCH 2/2 net-next] hyperv: Support batched notification
 
 
 
 On Wed, Mar 11, 2015 at 2:50 AM, K. Y. Srinivasan k...@microsoft.com
 wrote:
  Optimize notifying the host by deferring notification until there are
  no more packets to be sent. This will help in batching the requests on
  the host.
 
  Signed-off-by: K. Y. Srinivasan k...@microsoft.com
  ---
   drivers/net/hyperv/hyperv_net.h   |2 +-
   drivers/net/hyperv/netvsc.c   |   14 +-
   drivers/net/hyperv/netvsc_drv.c   |3 ++-
   drivers/net/hyperv/rndis_filter.c |2 +-
   4 files changed, 13 insertions(+), 8 deletions(-)
 
  diff --git a/drivers/net/hyperv/hyperv_net.h
  b/drivers/net/hyperv/hyperv_net.h index 4815843..3fd9896 100644
  --- a/drivers/net/hyperv/hyperv_net.h
  +++ b/drivers/net/hyperv/hyperv_net.h
  @@ -184,7 +184,7 @@ struct rndis_device {  int
  netvsc_device_add(struct hv_device *device, void *additional_info);
  int netvsc_device_remove(struct hv_device *device);  int
  netvsc_send(struct hv_device *device,
  -   struct hv_netvsc_packet *packet);
  +   struct hv_netvsc_packet *packet, bool kick_q);
   void netvsc_linkstatus_callback(struct hv_device *device_obj,
  struct rndis_message *resp);
   int netvsc_recv_callback(struct hv_device *device_obj, diff --git
  a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index
  208eb05..9003b94 100644
  --- a/drivers/net/hyperv/netvsc.c
  +++ b/drivers/net/hyperv/netvsc.c
  @@ -707,7 +707,7 @@ static u32 netvsc_copy_to_send_buf(struct
  netvsc_device *net_device,  }
 
   int netvsc_send(struct hv_device *device,
  -   struct hv_netvsc_packet *packet)
  +   struct hv_netvsc_packet *packet, bool kick_q)
   {
  struct netvsc_device *net_device;
  int ret = 0;
  @@ -719,6 +719,7 @@ int netvsc_send(struct hv_device *device,
  u32 msg_size = 0;
  struct sk_buff *skb = NULL;
  u16 q_idx = packet-q_idx;
  +   u32 vmbus_flags =
 VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
 
 
  net_device = get_outbound_net_device(device); @@ -768,18
 +769,21 @@
  int netvsc_send(struct hv_device *device,
  return -ENODEV;
 
  if (packet-page_buf_cnt) {
  -   ret = vmbus_sendpacket_pagebuffer(out_channel,
  +   ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
packet-page_buf,
packet-page_buf_cnt,
sendMessage,
sizeof(struct
 nvsp_message),
  - req_id);
  + req_id,
  + vmbus_flags,
  + kick_q);
 
 What if kick_q is false but ret is -EAGAIN here? Looks like in this case host
 won't get notified at all. How about checking whether txq and kicking if it 
 has
 been stopped like what other network driver did?

Good point. I am going to fix this issue in the VMBUS layer. The kick_q argument
is simply a hint to the vmbus level - the lower level can choose not to notify 
the
host (even if kick_q is true) based on other considerations.

I will resend this series with the logic in the vmbus driver. I will send the 
patch out
and Greg can decide if the vmbus change should go through Greg's tree or 
David's tree.

Regards,

K. Y
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 2/2 net-next] hyperv: Support batched notification

2015-03-11 Thread KY Srinivasan


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Wednesday, March 11, 2015 2:24 AM
 To: KY Srinivasan
 Cc: da...@davemloft.net; net...@vger.kernel.org;
 gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
 de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH 2/2 net-next] hyperv: Support batched notification
 
 On Tue, Mar 10, K. Y. Srinivasan wrote:
 
  Optimize notifying the host by deferring notification until there are
  no more packets to be sent. This will help in batching the requests on
  the host.
 
 The subjects for the network driver say hyperv:. But all such changes
 should get netvsc: as prefix to make it clear what each single patch is all
 about.
 
 This should be changed for upcoming submissions.

Olaf,
This is the convention that we have used for patches submitted to David's tree. 
You will see
net-next in the subject line and you can key off of that.

K. Y
 
 Thanks!
 
 Olaf


Re: [PATCH 2/2 net-next] hyperv: Support batched notification

2015-03-11 Thread Olaf Hering
On Tue, Mar 10, K. Y. Srinivasan wrote:

 Optimize notifying the host by deferring notification until there
 are no more packets to be sent. This will help in batching the requests
 on the host.

The subjects for the network driver say hyperv:. But all such changes
should get netvsc: as prefix to make it clear what each single patch
is all about.

This should be changed for upcoming submissions.

Thanks!

Olaf
--
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 2/2 net-next] hyperv: Support batched notification

2015-03-10 Thread Jason Wang



On Wed, Mar 11, 2015 at 2:50 AM, K. Y. Srinivasan  
wrote:

Optimize notifying the host by deferring notification until there
are no more packets to be sent. This will help in batching the 
requests

on the host.

Signed-off-by: K. Y. Srinivasan 
---
 drivers/net/hyperv/hyperv_net.h   |2 +-
 drivers/net/hyperv/netvsc.c   |   14 +-
 drivers/net/hyperv/netvsc_drv.c   |3 ++-
 drivers/net/hyperv/rndis_filter.c |2 +-
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h 
b/drivers/net/hyperv/hyperv_net.h

index 4815843..3fd9896 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -184,7 +184,7 @@ struct rndis_device {
 int netvsc_device_add(struct hv_device *device, void 
*additional_info);

 int netvsc_device_remove(struct hv_device *device);
 int netvsc_send(struct hv_device *device,
-   struct hv_netvsc_packet *packet);
+   struct hv_netvsc_packet *packet, bool kick_q);
 void netvsc_linkstatus_callback(struct hv_device *device_obj,
struct rndis_message *resp);
 int netvsc_recv_callback(struct hv_device *device_obj,
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 208eb05..9003b94 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -707,7 +707,7 @@ static u32 netvsc_copy_to_send_buf(struct 
netvsc_device *net_device,

 }
 
 int netvsc_send(struct hv_device *device,

-   struct hv_netvsc_packet *packet)
+   struct hv_netvsc_packet *packet, bool kick_q)
 {
struct netvsc_device *net_device;
int ret = 0;
@@ -719,6 +719,7 @@ int netvsc_send(struct hv_device *device,
u32 msg_size = 0;
struct sk_buff *skb = NULL;
u16 q_idx = packet->q_idx;
+   u32 vmbus_flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
 
 
 	net_device = get_outbound_net_device(device);

@@ -768,18 +769,21 @@ int netvsc_send(struct hv_device *device,
return -ENODEV;
 
 	if (packet->page_buf_cnt) {

-   ret = vmbus_sendpacket_pagebuffer(out_channel,
+   ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
  packet->page_buf,
  packet->page_buf_cnt,
  ,
  sizeof(struct nvsp_message),
- req_id);
+ req_id,
+ vmbus_flags,
+ kick_q);


What if kick_q is false but ret is -EAGAIN here? Looks like in this 
case host won't get notified at all. How about checking whether txq and 
kicking if it has been stopped like what other network driver did?


} else {
-   ret = vmbus_sendpacket(out_channel, ,
+   ret = vmbus_sendpacket_ctl(out_channel, ,
sizeof(struct nvsp_message),
req_id,
VM_PKT_DATA_INBAND,
-   VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+   vmbus_flags,
+   kick_q);
}
 
 	if (ret == 0) {
diff --git a/drivers/net/hyperv/netvsc_drv.c 
b/drivers/net/hyperv/netvsc_drv.c

index a06bd66..80b4b29 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -384,6 +384,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, 
struct net_device *net)

u32 net_trans_info;
u32 hash;
u32 skb_length = skb->len;
+   bool kick_q = !skb->xmit_more;
 
 
 	/* We will atmost need two pages to describe the rndis

@@ -556,7 +557,7 @@ do_send:
packet->page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size,
skb, >page_buf[0]);
 
-	ret = netvsc_send(net_device_ctx->device_ctx, packet);

+   ret = netvsc_send(net_device_ctx->device_ctx, packet, kick_q);
 
 drop:

if (ret == 0) {
diff --git a/drivers/net/hyperv/rndis_filter.c 
b/drivers/net/hyperv/rndis_filter.c

index ca81de0..05f3792 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -238,7 +238,7 @@ static int rndis_filter_send_request(struct 
rndis_device *dev,
 
 	packet->send_completion = NULL;
 
-	ret = netvsc_send(dev->net_dev->dev, packet);

+   ret = netvsc_send(dev->net_dev->dev, packet, true);
return ret;
 }
 
--

1.7.4.1



--
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 2/2 net-next] hyperv: Support batched notification

2015-03-10 Thread Jason Wang



On Wed, Mar 11, 2015 at 2:50 AM, K. Y. Srinivasan k...@microsoft.com 
wrote:

Optimize notifying the host by deferring notification until there
are no more packets to be sent. This will help in batching the 
requests

on the host.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
---
 drivers/net/hyperv/hyperv_net.h   |2 +-
 drivers/net/hyperv/netvsc.c   |   14 +-
 drivers/net/hyperv/netvsc_drv.c   |3 ++-
 drivers/net/hyperv/rndis_filter.c |2 +-
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h 
b/drivers/net/hyperv/hyperv_net.h

index 4815843..3fd9896 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -184,7 +184,7 @@ struct rndis_device {
 int netvsc_device_add(struct hv_device *device, void 
*additional_info);

 int netvsc_device_remove(struct hv_device *device);
 int netvsc_send(struct hv_device *device,
-   struct hv_netvsc_packet *packet);
+   struct hv_netvsc_packet *packet, bool kick_q);
 void netvsc_linkstatus_callback(struct hv_device *device_obj,
struct rndis_message *resp);
 int netvsc_recv_callback(struct hv_device *device_obj,
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 208eb05..9003b94 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -707,7 +707,7 @@ static u32 netvsc_copy_to_send_buf(struct 
netvsc_device *net_device,

 }
 
 int netvsc_send(struct hv_device *device,

-   struct hv_netvsc_packet *packet)
+   struct hv_netvsc_packet *packet, bool kick_q)
 {
struct netvsc_device *net_device;
int ret = 0;
@@ -719,6 +719,7 @@ int netvsc_send(struct hv_device *device,
u32 msg_size = 0;
struct sk_buff *skb = NULL;
u16 q_idx = packet-q_idx;
+   u32 vmbus_flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
 
 
 	net_device = get_outbound_net_device(device);

@@ -768,18 +769,21 @@ int netvsc_send(struct hv_device *device,
return -ENODEV;
 
 	if (packet-page_buf_cnt) {

-   ret = vmbus_sendpacket_pagebuffer(out_channel,
+   ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
  packet-page_buf,
  packet-page_buf_cnt,
  sendMessage,
  sizeof(struct nvsp_message),
- req_id);
+ req_id,
+ vmbus_flags,
+ kick_q);


What if kick_q is false but ret is -EAGAIN here? Looks like in this 
case host won't get notified at all. How about checking whether txq and 
kicking if it has been stopped like what other network driver did?


} else {
-   ret = vmbus_sendpacket(out_channel, sendMessage,
+   ret = vmbus_sendpacket_ctl(out_channel, sendMessage,
sizeof(struct nvsp_message),
req_id,
VM_PKT_DATA_INBAND,
-   VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+   vmbus_flags,
+   kick_q);
}
 
 	if (ret == 0) {
diff --git a/drivers/net/hyperv/netvsc_drv.c 
b/drivers/net/hyperv/netvsc_drv.c

index a06bd66..80b4b29 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -384,6 +384,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, 
struct net_device *net)

u32 net_trans_info;
u32 hash;
u32 skb_length = skb-len;
+   bool kick_q = !skb-xmit_more;
 
 
 	/* We will atmost need two pages to describe the rndis

@@ -556,7 +557,7 @@ do_send:
packet-page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size,
skb, packet-page_buf[0]);
 
-	ret = netvsc_send(net_device_ctx-device_ctx, packet);

+   ret = netvsc_send(net_device_ctx-device_ctx, packet, kick_q);
 
 drop:

if (ret == 0) {
diff --git a/drivers/net/hyperv/rndis_filter.c 
b/drivers/net/hyperv/rndis_filter.c

index ca81de0..05f3792 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -238,7 +238,7 @@ static int rndis_filter_send_request(struct 
rndis_device *dev,
 
 	packet-send_completion = NULL;
 
-	ret = netvsc_send(dev-net_dev-dev, packet);

+   ret = netvsc_send(dev-net_dev-dev, packet, true);
return ret;
 }
 
--

1.7.4.1



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