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