[PATCH for-8.0] nbd/server: Request TCP_NODELAY

Eric Blake posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230327192947.1324372-1-eblake@redhat.com
Maintainers: Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
There is a newer version of this series
nbd/server.c | 1 +
1 file changed, 1 insertion(+)
[PATCH for-8.0] nbd/server: Request TCP_NODELAY
Posted by Eric Blake 1 year ago
Nagle's algorithm adds latency in order to reduce network packet
overhead on small packets.  But when we are already using corking to
merge smaller packets into transactional requests, the extra delay
from TCP defaults just gets in the way.

For reference, qemu as an NBD client already requests TCP_NODELAY (see
nbd_connect() in nbd/client-connection.c); as does libnbd as a client
[1], and nbdkit as a server [2].

[1] https://gitlab.com/nbdkit/libnbd/-/blob/a48a1142/generator/states-connect.c#L39
[2] https://gitlab.com/nbdkit/nbdkit/-/blob/45b72f5b/server/sockets.c#L430

CC: Florian Westphal <fw@strlen.de>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 nbd/server.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nbd/server.c b/nbd/server.c
index a4750e41880..976223860bf 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2755,6 +2755,7 @@ void nbd_client_new(QIOChannelSocket *sioc,
     }
     client->tlsauthz = g_strdup(tlsauthz);
     client->sioc = sioc;
+    qio_channel_set_delay(QIO_CHANNEL(cioc), false);
     object_ref(OBJECT(client->sioc));
     client->ioc = QIO_CHANNEL(sioc);
     object_ref(OBJECT(client->ioc));

base-commit: e3debd5e7d0ce031356024878a0a18b9d109354a
-- 
2.39.2
Re: [PATCH for-8.0] nbd/server: Request TCP_NODELAY
Posted by Florian Westphal 1 year ago
Eric Blake <eblake@redhat.com> wrote:
> Nagle's algorithm adds latency in order to reduce network packet
> overhead on small packets.  But when we are already using corking to
> merge smaller packets into transactional requests, the extra delay
> from TCP defaults just gets in the way.
> 
> For reference, qemu as an NBD client already requests TCP_NODELAY (see
> nbd_connect() in nbd/client-connection.c); as does libnbd as a client
> [1], and nbdkit as a server [2].
> 
> [1] https://gitlab.com/nbdkit/libnbd/-/blob/a48a1142/generator/states-connect.c#L39
> [2] https://gitlab.com/nbdkit/nbdkit/-/blob/45b72f5b/server/sockets.c#L430
> 
> CC: Florian Westphal <fw@strlen.de>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  nbd/server.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index a4750e41880..976223860bf 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2755,6 +2755,7 @@ void nbd_client_new(QIOChannelSocket *sioc,
>      }
>      client->tlsauthz = g_strdup(tlsauthz);
>      client->sioc = sioc;
> +    qio_channel_set_delay(QIO_CHANNEL(cioc), false);

../nbd/server.c: In function 'nbd_client_new':
../nbd/server.c:2763:39: error: 'cioc' undeclared (first use in this function); did you mean 'sioc'?

Other than that this looks good to me.
Re: [PATCH for-8.0] nbd/server: Request TCP_NODELAY
Posted by Eric Blake 1 year ago
On Tue, Mar 28, 2023 at 12:42:59AM +0200, Florian Westphal wrote:
> Eric Blake <eblake@redhat.com> wrote:
> > Nagle's algorithm adds latency in order to reduce network packet
> > overhead on small packets.  But when we are already using corking to
> > merge smaller packets into transactional requests, the extra delay
> > from TCP defaults just gets in the way.
> > 
> > For reference, qemu as an NBD client already requests TCP_NODELAY (see
> > nbd_connect() in nbd/client-connection.c); as does libnbd as a client
> > [1], and nbdkit as a server [2].
> > 
> > [1] https://gitlab.com/nbdkit/libnbd/-/blob/a48a1142/generator/states-connect.c#L39
> > [2] https://gitlab.com/nbdkit/nbdkit/-/blob/45b72f5b/server/sockets.c#L430
> > 
> > CC: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> >  nbd/server.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/nbd/server.c b/nbd/server.c
> > index a4750e41880..976223860bf 100644
> > --- a/nbd/server.c
> > +++ b/nbd/server.c
> > @@ -2755,6 +2755,7 @@ void nbd_client_new(QIOChannelSocket *sioc,
> >      }
> >      client->tlsauthz = g_strdup(tlsauthz);
> >      client->sioc = sioc;
> > +    qio_channel_set_delay(QIO_CHANNEL(cioc), false);
> 
> ../nbd/server.c: In function 'nbd_client_new':
> ../nbd/server.c:2763:39: error: 'cioc' undeclared (first use in this function); did you mean 'sioc'?
> 
> Other than that this looks good to me.

Arrgh. Bitten by hitting send before saving the edits in my buffer.
Yes, the obvious fix is needed and intended.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org