[Qemu-devel] [PATCH 0/3] nbd: use TCP_NODELAY

[Qemu-devel] [PATCH 0/3] nbd: use TCP_NODELAY

[Stefan Hajnoczi]

The nbd block driver should use TCP_NODELAY.  Nick Thomas <nick@bytemark.co.uk>
measured a 40 millisecond latency added by the Naggle algorithm.

This series turns on TCP_NODELAY.  This requires that we use TCP_CORK to
efficiently send NBD requests that contain a payload after the header.

Finally, fix a bug where we forget to unlock a mutex when sending fails.

Stefan Hajnoczi (3):
  nbd: unlock mutex in nbd_co_send_request() error path
  nbd: use TCP_CORK in nbd_co_send_request()
  nbd: set TCP_NODELAY

 block/nbd.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/3] nbd: unlock mutex in nbd_co_send_request() error path

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nbd.c b/block/nbd.c
index eff683c..662df16 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -339,7 +339,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
         ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
                             offset, request->len);
         if (ret != request->len) {
-            return -EIO;
+            rc = -EIO;
         }
     }
     qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL,
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/3] nbd: use TCP_CORK in nbd_co_send_request()

[Stefan Hajnoczi]

Use TCP_CORK to defer packet transmission until both the header and the
payload have been written.

Suggested-by: Nick Thomas <nick@bytemark.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nbd.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 662df16..485bbf0 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -334,13 +334,23 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
     s->send_coroutine = qemu_coroutine_self();
     qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write,
                             nbd_have_request, s);
-    rc = nbd_send_request(s->sock, request);
-    if (rc >= 0 && qiov) {
-        ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
-                            offset, request->len);
-        if (ret != request->len) {
-            rc = -EIO;
+    if (qiov) {
+        if (!s->is_unix) {
+            socket_set_cork(s->sock, 1);
         }
+        rc = nbd_send_request(s->sock, request);
+        if (rc >= 0) {
+            ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
+                                offset, request->len);
+            if (ret != request->len) {
+                rc = -EIO;
+            }
+        }
+        if (!s->is_unix) {
+            socket_set_cork(s->sock, 0);
+        }
+    } else {
+        rc = nbd_send_request(s->sock, request);
     }
     qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL,
                             nbd_have_request, s);
-- 
1.8.1.4

[Qemu-devel] [PATCH 3/3] nbd: set TCP_NODELAY

[Stefan Hajnoczi]

Disable the Nagle algorithm to reduce latency.  Note this means we must
also use TCP_CORK when sending header followed by payload to avoid
fragmenting lots of little packets.  The previous patch took care of
that.

Suggested-by: Nick Thomas <nick@bytemark.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nbd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 485bbf0..d9dc454 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -406,6 +406,9 @@ static int nbd_establish_connection(BlockDriverState *bs)
         sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path"));
     } else {
         sock = tcp_socket_outgoing_opts(s->socket_opts);
+        if (sock >= 0) {
+            socket_set_nodelay(sock);
+        }
     }
 
     /* Failed to establish connection */
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 0/3] nbd: use TCP_NODELAY

[Nicholas Thomas]

On Mon, 2013-04-15 at 16:14 +0200, Stefan Hajnoczi wrote:
> The nbd block driver should use TCP_NODELAY.  Nick Thomas <nick@bytemark.co.uk>
> measured a 40 millisecond latency added by the Naggle algorithm.
> 
> This series turns on TCP_NODELAY.  This requires that we use TCP_CORK to
> efficiently send NBD requests that contain a payload after the header.
> 
> Finally, fix a bug where we forget to unlock a mutex when sending fails.
> 
> Stefan Hajnoczi (3):
>   nbd: unlock mutex in nbd_co_send_request() error path
>   nbd: use TCP_CORK in nbd_co_send_request()
>   nbd: set TCP_NODELAY
> 
>  block/nbd.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 

Tested-by: Nick Thomas <nick@bytemark.co.uk>

Running "dd if=/dev/zero of=/dev/hda bs=4096 count=1" - latency is
reduced from ~40ms to ~0.2ms. The entire NBD write request ends up in a
single ethernet frame, where mtu supports it (verified with wireshark).

Thanks!

/Nick

Re: [Qemu-devel] [PATCH 0/3] nbd: use TCP_NODELAY

[Paolo Bonzini]

Il 15/04/2013 16:14, Stefan Hajnoczi ha scritto:
> The nbd block driver should use TCP_NODELAY.  Nick Thomas <nick@bytemark.co.uk>
> measured a 40 millisecond latency added by the Naggle algorithm.
> 
> This series turns on TCP_NODELAY.  This requires that we use TCP_CORK to
> efficiently send NBD requests that contain a payload after the header.
> 
> Finally, fix a bug where we forget to unlock a mutex when sending fails.
> 
> Stefan Hajnoczi (3):
>   nbd: unlock mutex in nbd_co_send_request() error path
>   nbd: use TCP_CORK in nbd_co_send_request()
>   nbd: set TCP_NODELAY
> 
>  block/nbd.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 

Thanks, applied all to nbd-next.

I also marked patch 1 for stable branches.

Paolo

[Qemu-devel] [PATCH 3/3] nbd: set TCP_NODELAY

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

Disable the Nagle algorithm to reduce latency.  Note this means we must
also use TCP_CORK when sending header followed by payload to avoid
fragmenting lots of little packets.  The previous patch took care of
that.

Suggested-by: Nick Thomas <nick@bytemark.co.uk>
Tested-by: Nick Thomas <nick@bytemark.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/nbd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 485bbf0..d9dc454 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -406,6 +406,9 @@ static int nbd_establish_connection(BlockDriverState *bs)
         sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path"));
     } else {
         sock = tcp_socket_outgoing_opts(s->socket_opts);
+        if (sock >= 0) {
+            socket_set_nodelay(sock);
+        }
     }
 
     /* Failed to establish connection */
-- 
1.8.1.4