Re: [Nbd] [PATCH] Docs: improve description of disconnection methods

[Alex Bligh <alex@...872...>]

Wouter,

>> +A client MAY use a soft disconnect to terminate the session
>> +whenever it wishes, provided that there are no outstanding
>> +replies to options.
> 
> NAK. A client MAY use a soft disconnect *at any time*, but the server
> MUST NOT act upon it until there are no outstanding replies, and the
> client MUST NOT send any further options after sending NBD_OPT_ABORT.
> 
> (same for CMD_DISC)

This gets to the root of the unresolved issues I think. I suspect
the answer may be different for NBD_OPT_ABORT and NBD_CMD_DISC.

NBD commands are asynchronous and can be replied and acted on
in any order (so, from the document):
 
  "The server MAY process commands out of order, and MAY reply
   out of order"

I wouldn't want to loose that. So if the client sends NBD_CMD_DISC
without waiting for all his inflight commands to complete, those
inflight commands may not be executed at all, because the server
is free to process commands in any order. It's going to make
server design very awkward if you can only process /some/ commands
out of order. Another alternative would be to make the server
wait for all commands to complete before acting on the disconnect
(as opposed to or in addition to making the client wait to send
it). I'm reasonably relaxed about which one we do, but I think
we should do one or the other (or at least say that if the
client sends NBD_CMD_DISC without waiting for commands to complete
then those commands must not be executed). There are thus
various choices for NBD_CMD_DISC.

I think the option haggling phase is different (or rather need
not be the same). Fundamentally options MUST be processed in
the order they are issued, and there is only ever one in
flight at a time. My understanding is (though perhaps this
not explicit in the document) that the client should not be
sending ANY option until it has got a reply to the last one.
Certainly I know of no servers which can process options in
parallel, and as we don't have an 'Id' field to line up
replies they would have to be processed sequentially anyway.
I think we should document (somewhere) that the client MUST NOT
send an option until it has received a final reply to the previous
option. If we don't do that, we should document how concurrency
with options is meant to work.

So for this case I think it is completely correct that NBD_OPT_ABORT
must not (like any other option) be sent until there are no
outstanding *option* replies.

Let's see if we can resolve this one on list.

>> +terminate the session. In the client's case, if it wishes to
>> +do so it MUST use soft disconnect. In the server's case it
>> +MUST (save where set out above) simply error inbound options until
>> +the client gets the hint that it is unwelcome.
> 
> It might be good to add a "NBD_REP_ERR_NOSERVICE" error, for "server
> shutting down"?

I think that's probably a good idea. I suspect I'm going to go
and remove the bit that prescribes exactly what errors are permitted
for NBD_OPT_STARTTLS etc. as in the past 2 days we've now found two more
(NBD_REP_ERR_PLATFORM and this)!

>> +On a server shutdown, the server SHOULD wait for inflight
>> +requests to be serviced prior to initiating a hard disconnect.
> 
> I still think it's a good idea for the server to be allowed to send an
> unsolicited error reply in that case, but it might be better if indeed
> that would be negotiated first.

I still don't, but would object to it less than at the option phase.
I don't see what is actually gained by an unsolicited error reply over
a simple TCP shutdown (TLS layer first if necessary). I suppose
the only thing would be to know 'this is not a network break'. I
think the gain from that is minimal compared to the complexity for a
'simple case' client that just does A->B and B->A request/replies
in lockstep.

> [...]
>> -Clients MUST NOT set any other flags; the server MUST drop the
>> +Clients MUST NOT set any other flags; the server MUST drop the TCP
> 
> initiate a hard disconnect?

My thought was that we aren't (yet) in the option haggling phase,
so should not use option haggling terminology.

> [...]
>> - `NBD_OPT_ABORT` (2)
>> 
>> -    The client desires to abort the negotiation and close the
>> -    connection.
>> +    The client desires to abort the negotiation and terminate the
>> +    session. The server MUST reply with `NBD_REP_ACK`.
> 
> Maybe add a note that this hasn't always been a requirement, and that
> therefore older servers may not send this reply?

+1.

-- 
Alex Bligh