* [PATCH V2 0/3 net-next] hyperv: Enable batched notification @ 2015-03-11 19:03 K. Y. Srinivasan 2015-03-11 19:04 ` K. Y. Srinivasan 0 siblings, 1 reply; 17+ messages in thread From: K. Y. Srinivasan @ 2015-03-11 19:03 UTC (permalink / raw) To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, gregkh Cc: K. Y. Srinivasan Take into consideration the xmit_more flag in skb to decide if we should notify the host as we place packets in VMBUS. The VMBUS API that would give us this control is already in Greg's tree, in this patch-set, that API is exported so it can be used in the netvsc driver. Also included here is a siganlling related fix in the vmbus driver that is exposed by the netvsc driver. I would like to thank Jason Wang <jasowang@redhat.com> for pointing out this issue. This version of the patch-set, adds an additional patch to fix the signalling issue. K. Y. Srinivasan (3): Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q hyperv: Support batched notification drivers/hv/channel.c | 33 +++++++++++++++++++++++++++++++++ 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 +- 5 files changed, 46 insertions(+), 8 deletions(-) -- 1.7.4.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() 2015-03-11 19:03 [PATCH V2 0/3 net-next] hyperv: Enable batched notification K. Y. Srinivasan @ 2015-03-11 19:04 ` K. Y. Srinivasan 0 siblings, 0 replies; 17+ messages in thread From: K. Y. Srinivasan @ 2015-03-11 19:04 UTC (permalink / raw) To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, gregkh Cc: K. Y. Srinivasan Export the vmbus_sendpacket_pagebuffer_ctl() interface. This interface will be used in the netvsc driver to optimize signalling the host. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- drivers/hv/channel.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index da53180..e58cdb7 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -710,6 +710,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, return ret; } +EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); /* * vmbus_sendpacket_pagebuffer - Send a range of single-page buffer -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() @ 2015-03-11 19:04 ` K. Y. Srinivasan 0 siblings, 0 replies; 17+ messages in thread From: K. Y. Srinivasan @ 2015-03-11 19:04 UTC (permalink / raw) To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, gregkh Export the vmbus_sendpacket_pagebuffer_ctl() interface. This interface will be used in the netvsc driver to optimize signalling the host. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- drivers/hv/channel.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index da53180..e58cdb7 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -710,6 +710,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, return ret; } +EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); /* * vmbus_sendpacket_pagebuffer - Send a range of single-page buffer -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q 2015-03-11 19:04 ` K. Y. Srinivasan @ 2015-03-11 19:04 ` K. Y. Srinivasan -1 siblings, 0 replies; 17+ messages in thread From: K. Y. Srinivasan @ 2015-03-11 19:04 UTC (permalink / raw) To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, gregkh Cc: K. Y. Srinivasan When the caller specifies that signalling should be deferred, we need to address the case where we are not able to place the current packet because the buffer is full. In this case, we will signal the host as some packets may have been placed on the ring buffer. I would like to thank Jason Wang <jasowang@redhat.com> for pointing out this issue. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index e58cdb7..ae06ba9 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal); + /* + * Here is the logic for signalling the host: + * 1. If the host is already draining the ringbuffer, + * don't signal. This is indicated by the parameter + * "signal". + * + * 2. If we are not able to write, signal if kick_q is false. + * kick_q being false indicates that we may have placed zero or + * more packets with more packets to come. We will signal in + * this case even if potentially we may have not placed any + * packet. This is a rare enough condition that it should not + * matter. + */ + if ((ret == 0) && kick_q && signal) vmbus_setevent(channel); + else if ((ret != 0) && !kick_q) + vmbus_setevent(channel); return ret; } @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal); + /* + * Here is the logic for signalling the host: + * 1. If the host is already draining the ringbuffer, + * don't signal. This is indicated by the parameter + * "signal". + * + * 2. If we are not able to write, signal if kick_q is false. + * kick_q being false indicates that we may have placed zero or + * more packets with more packets to come. We will signal in + * this case even if potentially we may have not placed any + * packet. This is a rare enough condition that it should not + * matter. + */ + if ((ret == 0) && kick_q && signal) vmbus_setevent(channel); + else if ((ret != 0) && !kick_q) + vmbus_setevent(channel); return ret; } -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q @ 2015-03-11 19:04 ` K. Y. Srinivasan 0 siblings, 0 replies; 17+ messages in thread From: K. Y. Srinivasan @ 2015-03-11 19:04 UTC (permalink / raw) To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, gregkh When the caller specifies that signalling should be deferred, we need to address the case where we are not able to place the current packet because the buffer is full. In this case, we will signal the host as some packets may have been placed on the ring buffer. I would like to thank Jason Wang <jasowang@redhat.com> for pointing out this issue. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index e58cdb7..ae06ba9 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal); + /* + * Here is the logic for signalling the host: + * 1. If the host is already draining the ringbuffer, + * don't signal. This is indicated by the parameter + * "signal". + * + * 2. If we are not able to write, signal if kick_q is false. + * kick_q being false indicates that we may have placed zero or + * more packets with more packets to come. We will signal in + * this case even if potentially we may have not placed any + * packet. This is a rare enough condition that it should not + * matter. + */ + if ((ret == 0) && kick_q && signal) vmbus_setevent(channel); + else if ((ret != 0) && !kick_q) + vmbus_setevent(channel); return ret; } @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal); + /* + * Here is the logic for signalling the host: + * 1. If the host is already draining the ringbuffer, + * don't signal. This is indicated by the parameter + * "signal". + * + * 2. If we are not able to write, signal if kick_q is false. + * kick_q being false indicates that we may have placed zero or + * more packets with more packets to come. We will signal in + * this case even if potentially we may have not placed any + * packet. This is a rare enough condition that it should not + * matter. + */ + if ((ret == 0) && kick_q && signal) vmbus_setevent(channel); + else if ((ret != 0) && !kick_q) + vmbus_setevent(channel); return ret; } -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q 2015-03-11 19:04 ` K. Y. Srinivasan @ 2015-03-12 3:07 ` Jason Wang -1 siblings, 0 replies; 17+ messages in thread From: Jason Wang @ 2015-03-12 3:07 UTC (permalink / raw) To: K. Y. Srinivasan Cc: davem, netdev, linux-kernel, devel, olaf, apw, gregkh, K. Y. Srinivasan On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@microsoft.com> wrote: > When the caller specifies that signalling should be deferred, we need > to > address the case where we are not able to place the current packet > because > the buffer is full. In this case, we will signal the host as some > packets > may have been placed on the ring buffer. > I would like to thank Jason Wang <jasowang@redhat.com> for pointing > out this issue. > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > --- > drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ > 1 files changed, 32 insertions(+), 0 deletions(-) > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > index e58cdb7..ae06ba9 100644 > --- a/drivers/hv/channel.c > +++ b/drivers/hv/channel.c > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel > *channel, void *buffer, > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > &signal); > > + /* > + * Here is the logic for signalling the host: > + * 1. If the host is already draining the ringbuffer, > + * don't signal. This is indicated by the parameter > + * "signal". > + * > + * 2. If we are not able to write, signal if kick_q is false. > + * kick_q being false indicates that we may have placed zero or > + * more packets with more packets to come. We will signal in > + * this case even if potentially we may have not placed any > + * packet. This is a rare enough condition that it should not > + * matter. > + */ > + > if ((ret == 0) && kick_q && signal) > vmbus_setevent(channel); > + else if ((ret != 0) && !kick_q) > + vmbus_setevent(channel); > > return ret; > } > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > vmbus_channel *channel, > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > &signal); > > + /* > + * Here is the logic for signalling the host: > + * 1. If the host is already draining the ringbuffer, > + * don't signal. This is indicated by the parameter > + * "signal". > + * > + * 2. If we are not able to write, signal if kick_q is false. > + * kick_q being false indicates that we may have placed zero or > + * more packets with more packets to come. We will signal in > + * this case even if potentially we may have not placed any > + * packet. This is a rare enough condition that it should not > + * matter. > + */ > + > if ((ret == 0) && kick_q && signal) > vmbus_setevent(channel); > + else if ((ret != 0) && !kick_q) > + vmbus_setevent(channel); > > return ret; > } > -- Looks like we need to kick unconditionally here. Consider we may get -EAGAIN when we want to send the last skb (kick_q is true) from the list. We need kick host in this case. Btw, another method is let the driver to decide e.g exporting the vmbus_setevent() and call it in netvsc_start_xmit(). ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q @ 2015-03-12 3:07 ` Jason Wang 0 siblings, 0 replies; 17+ messages in thread From: Jason Wang @ 2015-03-12 3:07 UTC (permalink / raw) To: K. Y. Srinivasan; +Cc: olaf, netdev, linux-kernel, gregkh, apw, devel, davem On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@microsoft.com> wrote: > When the caller specifies that signalling should be deferred, we need > to > address the case where we are not able to place the current packet > because > the buffer is full. In this case, we will signal the host as some > packets > may have been placed on the ring buffer. > I would like to thank Jason Wang <jasowang@redhat.com> for pointing > out this issue. > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > --- > drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ > 1 files changed, 32 insertions(+), 0 deletions(-) > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > index e58cdb7..ae06ba9 100644 > --- a/drivers/hv/channel.c > +++ b/drivers/hv/channel.c > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel > *channel, void *buffer, > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > &signal); > > + /* > + * Here is the logic for signalling the host: > + * 1. If the host is already draining the ringbuffer, > + * don't signal. This is indicated by the parameter > + * "signal". > + * > + * 2. If we are not able to write, signal if kick_q is false. > + * kick_q being false indicates that we may have placed zero or > + * more packets with more packets to come. We will signal in > + * this case even if potentially we may have not placed any > + * packet. This is a rare enough condition that it should not > + * matter. > + */ > + > if ((ret == 0) && kick_q && signal) > vmbus_setevent(channel); > + else if ((ret != 0) && !kick_q) > + vmbus_setevent(channel); > > return ret; > } > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > vmbus_channel *channel, > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > &signal); > > + /* > + * Here is the logic for signalling the host: > + * 1. If the host is already draining the ringbuffer, > + * don't signal. This is indicated by the parameter > + * "signal". > + * > + * 2. If we are not able to write, signal if kick_q is false. > + * kick_q being false indicates that we may have placed zero or > + * more packets with more packets to come. We will signal in > + * this case even if potentially we may have not placed any > + * packet. This is a rare enough condition that it should not > + * matter. > + */ > + > if ((ret == 0) && kick_q && signal) > vmbus_setevent(channel); > + else if ((ret != 0) && !kick_q) > + vmbus_setevent(channel); > > return ret; > } > -- Looks like we need to kick unconditionally here. Consider we may get -EAGAIN when we want to send the last skb (kick_q is true) from the list. We need kick host in this case. Btw, another method is let the driver to decide e.g exporting the vmbus_setevent() and call it in netvsc_start_xmit(). ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q 2015-03-12 3:07 ` Jason Wang @ 2015-03-12 3:31 ` KY Srinivasan -1 siblings, 0 replies; 17+ messages in thread From: KY Srinivasan @ 2015-03-12 3:31 UTC (permalink / raw) To: Jason Wang; +Cc: davem, netdev, linux-kernel, devel, olaf, apw, gregkh [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3960 bytes --] > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Wednesday, March 11, 2015 8:08 PM > To: KY Srinivasan > Cc: davem@davemloft.net; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de; > apw@canonical.com; gregkh@linuxfoundation.org; KY Srinivasan > Subject: Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the > signalling logic with kick_q > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@microsoft.com> > wrote: > > When the caller specifies that signalling should be deferred, we need > > to address the case where we are not able to place the current packet > > because the buffer is full. In this case, we will signal the host as > > some packets may have been placed on the ring buffer. > > I would like to thank Jason Wang <jasowang@redhat.com> for pointing > > out this issue. > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > > --- > > drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index > > e58cdb7..ae06ba9 100644 > > --- a/drivers/hv/channel.c > > +++ b/drivers/hv/channel.c > > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel > > *channel, void *buffer, > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > &signal); > > > > + /* > > + * Here is the logic for signalling the host: > > + * 1. If the host is already draining the ringbuffer, > > + * don't signal. This is indicated by the parameter > > + * "signal". > > + * > > + * 2. If we are not able to write, signal if kick_q is false. > > + * kick_q being false indicates that we may have placed zero or > > + * more packets with more packets to come. We will signal in > > + * this case even if potentially we may have not placed any > > + * packet. This is a rare enough condition that it should not > > + * matter. > > + */ > > + > > if ((ret == 0) && kick_q && signal) > > vmbus_setevent(channel); > > + else if ((ret != 0) && !kick_q) > > + vmbus_setevent(channel); > > > > return ret; > > } > > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > > vmbus_channel *channel, > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > &signal); > > > > + /* > > + * Here is the logic for signalling the host: > > + * 1. If the host is already draining the ringbuffer, > > + * don't signal. This is indicated by the parameter > > + * "signal". > > + * > > + * 2. If we are not able to write, signal if kick_q is false. > > + * kick_q being false indicates that we may have placed zero or > > + * more packets with more packets to come. We will signal in > > + * this case even if potentially we may have not placed any > > + * packet. This is a rare enough condition that it should not > > + * matter. > > + */ > > + > > if ((ret == 0) && kick_q && signal) > > vmbus_setevent(channel); > > + else if ((ret != 0) && !kick_q) > > + vmbus_setevent(channel); > > > > return ret; > > } > > -- > > Looks like we need to kick unconditionally here. Consider we may get - > EAGAIN when we want to send the last skb (kick_q is true) from the list. We > need kick host in this case. > > Btw, another method is let the driver to decide e.g exporting the > vmbus_setevent() and call it in netvsc_start_xmit(). There is other state that governs if the host needs to be signaled and I don't think we want to expose that to netvsc. In any case, netvsc will have to signal the host when EGAIN is received. We might as well do it in the vmbus driver. Thank you Jason, I will respin and resubmit the series. K. Y > > > ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q @ 2015-03-12 3:31 ` KY Srinivasan 0 siblings, 0 replies; 17+ messages in thread From: KY Srinivasan @ 2015-03-12 3:31 UTC (permalink / raw) To: Jason Wang; +Cc: olaf, netdev, linux-kernel, gregkh, apw, devel, davem > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Wednesday, March 11, 2015 8:08 PM > To: KY Srinivasan > Cc: davem@davemloft.net; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de; > apw@canonical.com; gregkh@linuxfoundation.org; KY Srinivasan > Subject: Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the > signalling logic with kick_q > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@microsoft.com> > wrote: > > When the caller specifies that signalling should be deferred, we need > > to address the case where we are not able to place the current packet > > because the buffer is full. In this case, we will signal the host as > > some packets may have been placed on the ring buffer. > > I would like to thank Jason Wang <jasowang@redhat.com> for pointing > > out this issue. > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > > --- > > drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index > > e58cdb7..ae06ba9 100644 > > --- a/drivers/hv/channel.c > > +++ b/drivers/hv/channel.c > > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel > > *channel, void *buffer, > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > &signal); > > > > + /* > > + * Here is the logic for signalling the host: > > + * 1. If the host is already draining the ringbuffer, > > + * don't signal. This is indicated by the parameter > > + * "signal". > > + * > > + * 2. If we are not able to write, signal if kick_q is false. > > + * kick_q being false indicates that we may have placed zero or > > + * more packets with more packets to come. We will signal in > > + * this case even if potentially we may have not placed any > > + * packet. This is a rare enough condition that it should not > > + * matter. > > + */ > > + > > if ((ret == 0) && kick_q && signal) > > vmbus_setevent(channel); > > + else if ((ret != 0) && !kick_q) > > + vmbus_setevent(channel); > > > > return ret; > > } > > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > > vmbus_channel *channel, > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > &signal); > > > > + /* > > + * Here is the logic for signalling the host: > > + * 1. If the host is already draining the ringbuffer, > > + * don't signal. This is indicated by the parameter > > + * "signal". > > + * > > + * 2. If we are not able to write, signal if kick_q is false. > > + * kick_q being false indicates that we may have placed zero or > > + * more packets with more packets to come. We will signal in > > + * this case even if potentially we may have not placed any > > + * packet. This is a rare enough condition that it should not > > + * matter. > > + */ > > + > > if ((ret == 0) && kick_q && signal) > > vmbus_setevent(channel); > > + else if ((ret != 0) && !kick_q) > > + vmbus_setevent(channel); > > > > return ret; > > } > > -- > > Looks like we need to kick unconditionally here. Consider we may get - > EAGAIN when we want to send the last skb (kick_q is true) from the list. We > need kick host in this case. > > Btw, another method is let the driver to decide e.g exporting the > vmbus_setevent() and call it in netvsc_start_xmit(). There is other state that governs if the host needs to be signaled and I don't think we want to expose that to netvsc. In any case, netvsc will have to signal the host when EGAIN is received. We might as well do it in the vmbus driver. Thank you Jason, I will respin and resubmit the series. K. Y > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q 2015-03-12 3:31 ` KY Srinivasan (?) @ 2015-03-12 4:49 ` KY Srinivasan -1 siblings, 0 replies; 17+ messages in thread From: KY Srinivasan @ 2015-03-12 4:49 UTC (permalink / raw) To: KY Srinivasan, Jason Wang Cc: olaf, netdev, linux-kernel, gregkh, apw, devel, davem > -----Original Message----- > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On > Behalf Of KY Srinivasan > Sent: Wednesday, March 11, 2015 8:32 PM > To: Jason Wang > Cc: olaf@aepfle.de; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > gregkh@linuxfoundation.org; apw@canonical.com; > devel@linuxdriverproject.org; davem@davemloft.net > Subject: RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the > signalling logic with kick_q > > > > > -----Original Message----- > > From: Jason Wang [mailto:jasowang@redhat.com] > > Sent: Wednesday, March 11, 2015 8:08 PM > > To: KY Srinivasan > > Cc: davem@davemloft.net; netdev@vger.kernel.org; linux- > > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de; > > apw@canonical.com; gregkh@linuxfoundation.org; KY Srinivasan > > Subject: Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in > > the signalling logic with kick_q > > > > > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@microsoft.com> > > wrote: > > > When the caller specifies that signalling should be deferred, we > > > need to address the case where we are not able to place the current > > > packet because the buffer is full. In this case, we will signal the > > > host as some packets may have been placed on the ring buffer. > > > I would like to thank Jason Wang <jasowang@redhat.com> for pointing > > > out this issue. > > > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > > > --- > > > drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ > > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index > > > e58cdb7..ae06ba9 100644 > > > --- a/drivers/hv/channel.c > > > +++ b/drivers/hv/channel.c > > > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct > vmbus_channel > > > *channel, void *buffer, > > > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > > &signal); > > > > > > + /* > > > + * Here is the logic for signalling the host: > > > + * 1. If the host is already draining the ringbuffer, > > > + * don't signal. This is indicated by the parameter > > > + * "signal". > > > + * > > > + * 2. If we are not able to write, signal if kick_q is false. > > > + * kick_q being false indicates that we may have placed zero or > > > + * more packets with more packets to come. We will signal in > > > + * this case even if potentially we may have not placed any > > > + * packet. This is a rare enough condition that it should not > > > + * matter. > > > + */ > > > + > > > if ((ret == 0) && kick_q && signal) > > > vmbus_setevent(channel); > > > + else if ((ret != 0) && !kick_q) > > > + vmbus_setevent(channel); > > > > > > return ret; > > > } > > > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > > > vmbus_channel *channel, > > > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > > &signal); > > > > > > + /* > > > + * Here is the logic for signalling the host: > > > + * 1. If the host is already draining the ringbuffer, > > > + * don't signal. This is indicated by the parameter > > > + * "signal". > > > + * > > > + * 2. If we are not able to write, signal if kick_q is false. > > > + * kick_q being false indicates that we may have placed zero or > > > + * more packets with more packets to come. We will signal in > > > + * this case even if potentially we may have not placed any > > > + * packet. This is a rare enough condition that it should not > > > + * matter. > > > + */ > > > + > > > if ((ret == 0) && kick_q && signal) > > > vmbus_setevent(channel); > > > + else if ((ret != 0) && !kick_q) > > > + vmbus_setevent(channel); > > > > > > return ret; > > > } > > > -- > > > > Looks like we need to kick unconditionally here. Consider we may get - > > EAGAIN when we want to send the last skb (kick_q is true) from the > > list. We need kick host in this case. > > > > Btw, another method is let the driver to decide e.g exporting the > > vmbus_setevent() and call it in netvsc_start_xmit(). > > There is other state that governs if the host needs to be signaled and I don't > think we want to expose that to netvsc. In any case, netvsc will have to signal > the host when EGAIN is received. > We might as well do it in the vmbus driver. > > Thank you Jason, I will respin and resubmit the series. Looks like there may be a way to make the signaling more precise even in this case. I am going to experiment with it. K. Y > > K. Y > > > > > > > > _______________________________________________ > devel mailing list > devel@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2 3/3 net-next] hyperv: Support batched notification 2015-03-11 19:04 ` K. Y. Srinivasan @ 2015-03-11 19:04 ` K. Y. Srinivasan -1 siblings, 0 replies; 17+ messages in thread From: K. Y. Srinivasan @ 2015-03-11 19:04 UTC (permalink / raw) To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, gregkh Cc: K. Y. Srinivasan 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 <kys@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); } 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH V2 3/3 net-next] hyperv: Support batched notification @ 2015-03-11 19:04 ` K. Y. Srinivasan 0 siblings, 0 replies; 17+ messages in thread From: K. Y. Srinivasan @ 2015-03-11 19:04 UTC (permalink / raw) To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang, gregkh 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 <kys@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); } 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V2 3/3 net-next] hyperv: Support batched notification 2015-03-11 19:04 ` K. Y. Srinivasan (?) @ 2015-03-12 3:08 ` Jason Wang 2015-03-12 3:33 ` KY Srinivasan -1 siblings, 1 reply; 17+ messages in thread From: Jason Wang @ 2015-03-12 3:08 UTC (permalink / raw) To: K. Y. Srinivasan Cc: davem, netdev, linux-kernel, devel, olaf, apw, gregkh, K. Y. Srinivasan On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@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 <kys@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); > } 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); Maybe just a !skb->xmit_more here to save a local variable. > > > 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 netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH V2 3/3 net-next] hyperv: Support batched notification 2015-03-12 3:08 ` Jason Wang @ 2015-03-12 3:33 ` KY Srinivasan 0 siblings, 0 replies; 17+ messages in thread From: KY Srinivasan @ 2015-03-12 3:33 UTC (permalink / raw) To: Jason Wang; +Cc: davem, netdev, linux-kernel, devel, olaf, apw, gregkh [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4296 bytes --] > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Wednesday, March 11, 2015 8:09 PM > To: KY Srinivasan > Cc: davem@davemloft.net; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de; > apw@canonical.com; gregkh@linuxfoundation.org; KY Srinivasan > Subject: Re: [PATCH V2 3/3 net-next] hyperv: Support batched notification > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@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 <kys@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); > > } 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); > > Maybe just a !skb->xmit_more here to save a local variable. Will fix it. K. Y ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH V2 3/3 net-next] hyperv: Support batched notification @ 2015-03-12 3:33 ` KY Srinivasan 0 siblings, 0 replies; 17+ messages in thread From: KY Srinivasan @ 2015-03-12 3:33 UTC (permalink / raw) To: Jason Wang; +Cc: olaf, netdev, linux-kernel, gregkh, apw, devel, davem > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Wednesday, March 11, 2015 8:09 PM > To: KY Srinivasan > Cc: davem@davemloft.net; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de; > apw@canonical.com; gregkh@linuxfoundation.org; KY Srinivasan > Subject: Re: [PATCH V2 3/3 net-next] hyperv: Support batched notification > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@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 <kys@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); > > } 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); > > Maybe just a !skb->xmit_more here to save a local variable. Will fix it. K. Y ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() 2015-03-11 19:04 ` K. Y. Srinivasan @ 2015-03-12 2:59 ` Jason Wang -1 siblings, 0 replies; 17+ messages in thread From: Jason Wang @ 2015-03-12 2:59 UTC (permalink / raw) To: K. Y. Srinivasan Cc: davem, netdev, linux-kernel, devel, olaf, apw, gregkh, K. Y. Srinivasan On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@microsoft.com> wrote: > Export the vmbus_sendpacket_pagebuffer_ctl() interface. This interface > will be used in the netvsc driver to optimize signalling the host. > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > --- > drivers/hv/channel.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > index da53180..e58cdb7 100644 > --- a/drivers/hv/channel.c > +++ b/drivers/hv/channel.c > @@ -710,6 +710,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > vmbus_channel *channel, > > return ret; > } > +EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); > > /* > * vmbus_sendpacket_pagebuffer - Send a range of single-page buffer > -- > 1.7.4.1 Acked-by: Jason Wang <jasowang@redhat.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() @ 2015-03-12 2:59 ` Jason Wang 0 siblings, 0 replies; 17+ messages in thread From: Jason Wang @ 2015-03-12 2:59 UTC (permalink / raw) To: K. Y. Srinivasan; +Cc: olaf, netdev, linux-kernel, gregkh, apw, devel, davem On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@microsoft.com> wrote: > Export the vmbus_sendpacket_pagebuffer_ctl() interface. This interface > will be used in the netvsc driver to optimize signalling the host. > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> > --- > drivers/hv/channel.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > index da53180..e58cdb7 100644 > --- a/drivers/hv/channel.c > +++ b/drivers/hv/channel.c > @@ -710,6 +710,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > vmbus_channel *channel, > > return ret; > } > +EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); > > /* > * vmbus_sendpacket_pagebuffer - Send a range of single-page buffer > -- > 1.7.4.1 Acked-by: Jason Wang <jasowang@redhat.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-03-12 4:49 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-11 19:03 [PATCH V2 0/3 net-next] hyperv: Enable batched notification K. Y. Srinivasan 2015-03-11 19:04 ` [PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan 2015-03-11 19:04 ` K. Y. Srinivasan 2015-03-11 19:04 ` [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick_q K. Y. Srinivasan 2015-03-11 19:04 ` K. Y. Srinivasan 2015-03-12 3:07 ` Jason Wang 2015-03-12 3:07 ` Jason Wang 2015-03-12 3:31 ` KY Srinivasan 2015-03-12 3:31 ` KY Srinivasan 2015-03-12 4:49 ` KY Srinivasan 2015-03-11 19:04 ` [PATCH V2 3/3 net-next] hyperv: Support batched notification K. Y. Srinivasan 2015-03-11 19:04 ` K. Y. Srinivasan 2015-03-12 3:08 ` Jason Wang 2015-03-12 3:33 ` KY Srinivasan 2015-03-12 3:33 ` KY Srinivasan 2015-03-12 2:59 ` [PATCH V2 1/3 net-next] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() Jason Wang 2015-03-12 2:59 ` Jason Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.