Patchwork net: use socket_set_nodelay() for -netdev socket

login
register
mail settings
Submitter Stefan Hajnoczi
Date Feb. 27, 2013, 2:05 p.m.
Message ID <1361973947-6136-1-git-send-email-stefanha@redhat.com>
Download mbox | patch
Permalink /patch/223607/
State New
Headers show

Comments

Stefan Hajnoczi - Feb. 27, 2013, 2:05 p.m.
Reduce -netdev socket latency by disabling the Nagle algorithm on
SOCK_STREAM sockets in net/socket.c.  Since we are tunelling Ethernet
over TCP we shouldn't artificially delay outgoing packets, let the guest
decide packet scheduling.

I already get sub-millisecond -netdev socket ping times on localhost, so
there was no measurable difference in my testing.  This won't hurt
though and may improve remote socket performance.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/socket.c | 3 +++
 1 file changed, 3 insertions(+)
Benoît Canet - Feb. 27, 2013, 4:29 p.m.
Le Wednesday 27 Feb 2013 à 15:05:47 (+0100), Stefan Hajnoczi a écrit :
> Reduce -netdev socket latency by disabling the Nagle algorithm on
> SOCK_STREAM sockets in net/socket.c.  Since we are tunelling Ethernet
> over TCP we shouldn't artificially delay outgoing packets, let the guest
> decide packet scheduling.
> 
> I already get sub-millisecond -netdev socket ping times on localhost, so
> there was no measurable difference in my testing.  This won't hurt
> though and may improve remote socket performance.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  net/socket.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/socket.c b/net/socket.c
> index 396dc8c..c0dcc31 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -439,6 +439,9 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
>      s->fd = fd;
>      s->listen_fd = -1;
>  
> +    /* Disable Nagle algorithm on TCP sockets to reduce latency */
> +    socket_set_nodelay(fd);
> +
>      if (is_connected) {
>          net_socket_connect(s);
>      } else {
> -- 
> 1.8.1.2
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Daniel P. Berrange - Feb. 27, 2013, 4:49 p.m.
On Wed, Feb 27, 2013 at 03:05:47PM +0100, Stefan Hajnoczi wrote:
> Reduce -netdev socket latency by disabling the Nagle algorithm on
> SOCK_STREAM sockets in net/socket.c.  Since we are tunelling Ethernet
> over TCP we shouldn't artificially delay outgoing packets, let the guest
> decide packet scheduling.
> 
> I already get sub-millisecond -netdev socket ping times on localhost, so
> there was no measurable difference in my testing.  This won't hurt
> though and may improve remote socket performance.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

ACK.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

> ---
>  net/socket.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/socket.c b/net/socket.c
> index 396dc8c..c0dcc31 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -439,6 +439,9 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
>      s->fd = fd;
>      s->listen_fd = -1;
>  
> +    /* Disable Nagle algorithm on TCP sockets to reduce latency */
> +    socket_set_nodelay(fd);
> +
>      if (is_connected) {
>          net_socket_connect(s);
>      } else {
> -- 


Daniel
Stefan Hajnoczi - Feb. 28, 2013, 1:49 p.m.
On Wed, Feb 27, 2013 at 04:49:16PM +0000, Daniel P. Berrange wrote:
> On Wed, Feb 27, 2013 at 03:05:47PM +0100, Stefan Hajnoczi wrote:
> > Reduce -netdev socket latency by disabling the Nagle algorithm on
> > SOCK_STREAM sockets in net/socket.c.  Since we are tunelling Ethernet
> > over TCP we shouldn't artificially delay outgoing packets, let the guest
> > decide packet scheduling.
> > 
> > I already get sub-millisecond -netdev socket ping times on localhost, so
> > there was no measurable difference in my testing.  This won't hurt
> > though and may improve remote socket performance.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> ACK.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Hi Dan,
QEMU usually only uses Singed-off-by: for authors and subsystem
maintainers who merge the patch.

Is it okay if I add a Reviewed-by: Daniel P. Berrange
<berrange@redhat.com> instead?

Stefan
Daniel P. Berrange - Feb. 28, 2013, 1:55 p.m.
On Thu, Feb 28, 2013 at 02:49:51PM +0100, Stefan Hajnoczi wrote:
> On Wed, Feb 27, 2013 at 04:49:16PM +0000, Daniel P. Berrange wrote:
> > On Wed, Feb 27, 2013 at 03:05:47PM +0100, Stefan Hajnoczi wrote:
> > > Reduce -netdev socket latency by disabling the Nagle algorithm on
> > > SOCK_STREAM sockets in net/socket.c.  Since we are tunelling Ethernet
> > > over TCP we shouldn't artificially delay outgoing packets, let the guest
> > > decide packet scheduling.
> > > 
> > > I already get sub-millisecond -netdev socket ping times on localhost, so
> > > there was no measurable difference in my testing.  This won't hurt
> > > though and may improve remote socket performance.
> > > 
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > ACK.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> 
> Hi Dan,
> QEMU usually only uses Singed-off-by: for authors and subsystem
> maintainers who merge the patch.

Ah, ok.

> 
> Is it okay if I add a Reviewed-by: Daniel P. Berrange
> <berrange@redhat.com> instead?

Of course. 

Daniel
Stefan Hajnoczi - March 1, 2013, 9:35 a.m.
On Thu, Feb 28, 2013 at 01:55:58PM +0000, Daniel P. Berrange wrote:
> On Thu, Feb 28, 2013 at 02:49:51PM +0100, Stefan Hajnoczi wrote:
> > On Wed, Feb 27, 2013 at 04:49:16PM +0000, Daniel P. Berrange wrote:
> > > On Wed, Feb 27, 2013 at 03:05:47PM +0100, Stefan Hajnoczi wrote:
> > > > Reduce -netdev socket latency by disabling the Nagle algorithm on
> > > > SOCK_STREAM sockets in net/socket.c.  Since we are tunelling Ethernet
> > > > over TCP we shouldn't artificially delay outgoing packets, let the guest
> > > > decide packet scheduling.
> > > > 
> > > > I already get sub-millisecond -netdev socket ping times on localhost, so
> > > > there was no measurable difference in my testing.  This won't hurt
> > > > though and may improve remote socket performance.
> > > > 
> > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > 
> > > ACK.
> > > 
> > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > 
> > Hi Dan,
> > QEMU usually only uses Singed-off-by: for authors and subsystem
> > maintainers who merge the patch.
> 
> Ah, ok.
> 
> > 
> > Is it okay if I add a Reviewed-by: Daniel P. Berrange
> > <berrange@redhat.com> instead?
> 
> Of course. 

Thanks, done.

Stefan

Patch

diff --git a/net/socket.c b/net/socket.c
index 396dc8c..c0dcc31 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -439,6 +439,9 @@  static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
     s->fd = fd;
     s->listen_fd = -1;
 
+    /* Disable Nagle algorithm on TCP sockets to reduce latency */
+    socket_set_nodelay(fd);
+
     if (is_connected) {
         net_socket_connect(s);
     } else {