Re: [PATCH v2] nbd/server: Suppress Broken pipe errors on abrupt disconnection

[Eric Blake]

On Wed, Sep 15, 2021 at 10:00:25AM +0100, Richard W.M. Jones wrote:
> > >I would say the spec is at best contradictory, but if you read other
> > >parts of the spec, then it's pretty clear that we're allowed to drop
> > >the connection whenever we like.  This section says as much:
> > >
> > >https://github.com/NetworkBlockDevice/nbd/blob/5805b25ad3da96e7c0b3160cda51ea19eb518d5b/doc/proto.md#terminating-the-transmission-phase
> > 
> > Hmm, that was exactly the section I read and quoted :)
> > 
> > >
> > >   There are two methods of terminating the transmission phase:
> > >   ...
> > >   "The client or the server drops the TCP session (in which case it
> > >   SHOULD shut down the TLS session first). This is referred to as
> > >   'initiating a hard disconnect'."
> > 
> > Right. Here the method is defined, no word that client can do it at any 
> > time.
> 
> I don't read this as a definition.
> 
> But we should probably clarify the spec to note that dropping the
> connection is fine, because it is - no one has made any argument so
> far that there's an actual reason to waste time on NBD_CMD_DISC.
> 
> Rich.
> 
> > Next, in same section:
> > 
> >    Either side MAY initiate a hard disconnect if it detects a violation by 
> > the other party of a mandatory condition within this document.
> > 
> > Next
> >    The client MAY issue a soft disconnect at any time
> > 
> > And finally:
> > 
> >    The client and the server MUST NOT initiate any form of disconnect other 
> > than in one of the above circumstances.

I went and re-read the upstream NBD discussion when this part of the
doc was added...

https://lists.debian.org/nbd/2016/04/msg00420.html
https://lists.debian.org/nbd/2016/04/msg00425.html

The argument back then was that sending CMD_DISC is desirable (clients
want to be nice to the server) but not mandatory (server must be
prepared for clients that aren't nice).  Sending CMD_DISC is what
counts as initiating soft disconnect; terminating abruptly without
CMD_DISC is hard disconnect (servers must not do it without detecting
some other first, but clients doing it when they have nothing further
to send is something we have to live with).

At the time the text was added, there was a question of whether to add
a command NBD_CMD_CLOSE that guaranteed clean server shutdown (the
client had to wait for the server's response):
https://lists.debian.org/nbd/2016/04/msg00236.html

but in the end, it was decided that the semantics of NBD_CMD_DISC were
already good enough for that purpose.  Which in turn means that
clients really do expect servers to gracefully flush things to disk on
NBD_CMD_DISC, and merely disconnecting (a hard shutdown) after
NBD_CMD_FLUSH is a bit riskier than sending a soft shutdown request.

> > 
> > 
> > Or do you mean some other spec section I missed?
> > 
> > >
> > >Anyway we're dropping the TCP connection because sometimes we are just
> > >interrogating an NBD server eg to find out what it supports, and doing
> > >a graceful shutdown is a waste of time and internet.

You're right that it costs another couple of packets, particularly for
things like 'nbdinfo --list', but is it really dominating the time
spent in the normal use of NBD?  Micro-optimizing the shutdown doesn't
have as much payoff as optimizing the speed of NBD_CMD_READ/WRITE.

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