The next commit wants to teach libnbd to ensure that a server will read EOF from a bi-directional connection as soon as libnbd is done writing, rather than making libnbd dependent on the server being the connection endpoint to initiate closing the socket or waiting on the user to eventually call nbd_close(). For that to work, we need to expose the equivalent of shutdown(SHUT_WR) for our various connection types. Note that shutdown() is only applicable to sockets, since it only really aids in bi-directional communication on a single fd. For now, libnbd only uses sockets (even for nbd_connect_command, we use socketpair() to pass a single fd to both stdin and stdout, rather than two pipe() calls to create a pair of one-way communication). But if we were to ever support NBD connections over a pair of pipe()s, the obvious implementation would be to close() the write pipe during .shut_writes, while leaving the read pipe open until .close. Also note that shutdown() is effectively non-blocking, but that gnutls_bye() can fail with GNUTLS_E_AGAIN if it is waiting for space to send notification to the server. But at the same time, while socket read and write normally distinguish between success, EAGAIN, and other failures, we don't need the tri-state return from our new hook (we will only ever attempt to shut down writes after sending NBD_CMD_DISC, and moving to CLOSED instead of DEAD at that point doesn't change the results of any earlier requests). Finally, note that the semantics of this new callback are specifically only for SHUT_WR, and not generic enough for SHUT_RDWR. This is because gnutls_bye() handles SHUT_WR with only writes to the socket, but when handling SHUT_RDWR it also waits for a subsequent read. Getting the ping-pong I/O correct would require more states, for no real gain when the user can just blindly call nbd_close() and close the entire socket rather than having to massage their poll() loop through more states. --- lib/internal.h | 3 ++- lib/crypto.c | 21 +++++++++++++++++---- lib/socket.c | 12 +++++++++++- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 6eb50d8..0d02687 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -1,5 +1,5 @@ /* nbd client library in userspace: internal definitions - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -267,6 +267,7 @@ struct socket_ops { struct socket *sock, const void *buf, size_t len, int flags); bool (*pending) (struct socket *sock); int (*get_fd) (struct socket *sock); + bool (*shut_writes) (struct nbd_handle *h, struct socket *sock); int (*close) (struct socket *sock); }; diff --git a/lib/crypto.c b/lib/crypto.c index 3e33af7..516651c 100644 --- a/lib/crypto.c +++ b/lib/crypto.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -230,9 +230,21 @@ tls_get_fd (struct socket *sock) return sock->u.tls.oldsock->ops->get_fd (sock->u.tls.oldsock); } -/* XXX Calling gnutls_bye is possible, but it may send and receive - * data over the wire which would require modifications to the state - * machine. So instead we abruptly drop the TLS session. +static bool +tls_shut_writes (struct nbd_handle *h, struct socket *sock) +{ + int r = gnutls_bye (sock->u.tls.session, GNUTLS_SHUT_WR); + + if (r == GNUTLS_E_AGAIN || r == GNUTLS_E_INTERRUPTED) + return false; + if (r != 0) + debug (h, "ignoring gnutls_bye failure: %s", gnutls_strerror (r)); + return sock->u.tls.oldsock->ops->shut_writes (h, sock->u.tls.oldsock); +} + +/* XXX Calling gnutls_bye(GNUTLS_SHUT_RDWR) is possible, but it may send + * and receive data over the wire which would require further modifications + * to the state machine. So instead we abruptly drop the TLS session. */ static int tls_close (struct socket *sock) @@ -254,6 +266,7 @@ static struct socket_ops crypto_ops = { .send = tls_send, .pending = tls_pending, .get_fd = tls_get_fd, + .shut_writes = tls_shut_writes, .close = tls_close, }; diff --git a/lib/socket.c b/lib/socket.c index ac60286..6698941 100644 --- a/lib/socket.c +++ b/lib/socket.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -63,6 +63,15 @@ socket_get_fd (struct socket *sock) return sock->u.fd; } +static bool +socket_shut_writes (struct nbd_handle *h, struct socket *sock) +{ + if (shutdown (sock->u.fd, SHUT_WR) == -1) + debug (h, "ignoring shutdown failure: %s", strerror (errno)); + /* Regardless of any errors, we don't need to retry. */ + return true; +} + static int socket_close (struct socket *sock) { @@ -76,6 +85,7 @@ static struct socket_ops socket_ops = { .recv = socket_recv, .send = socket_send, .get_fd = socket_get_fd, + .shut_writes = socket_shut_writes, .close = socket_close, }; -- 2.26.0.rc2