On Tue, Apr 12, 2016 at 08:31:33PM +0100, Alex Bligh wrote:
> Improve the documentation as per the mailing list discussion.
> Here's what we deciced (broadly).
> 
> * One side MAY drop the connection if the other end violates a
>  MUST condition.
> 
> * The server MUST drop the connection in the 'no way out' situations
>  during the negotiation phase (error on NBD_OPT_EXPORT_NAME, error
>  in negotiating text).
> 
> * The server SHOULD NOT otherwise drop the connection. It can wait
>  and error the next command. Clearly there are situations where
>  this is going to happen (e.g. server shutdown).
> 
> * If the server does need to drop the connection, it SHOULD wait
>  until there are no commands in-flight in transmission mode,
>  it possible.
> 
> * If he client is going to drop the the connection, then other
>  than in the event of a protocol violation or a 'no way out'
>  situation (e.g. TLS negotiation fails), it MUST use NBD_CMD_DISC
>  or NBD_OPT_ABORT
> 
> * We should tidy up the semantics and descriptions of NBD_CMD_DISC
>  and NBD_OPT_ABORT, viz replies or not to the latter, shutting
>  down TLS properly etc.
> 
> Other changes:
> 
> * Added a reply to NBD_OPT_ABORT. No harm if the client closes
>   the connection anyway.
> 
> * Said the offset and length fields in NBD_CMD_DISC MUST be zero.
>   Not doing so is a protocol violation and would only lead to ...
>   the connection being closed, so this is a useful tidy up.
> 
> * Introduced consistent terminology for disconnection throughout.
> 
> This patch applies on top of:
>   0001-docs-proto.md-Clarify-SHOULD-MUST-MAY-etc v7
> 
> Signed-off-by: Alex Bligh <a...@alex.org.uk>
> ---
>  doc/proto.md | 143 
> ++++++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 107 insertions(+), 36 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index b88e054..db6b96d 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -122,7 +122,7 @@ C: 32 bits, flags
>  This completes the initial phase of negotiation; the client and server
>  now both know they understand the first version of the newstyle
>  handshake, with no options. The client SHOULD ignore any handshake flags
> -it does not recognize, while the server MUST close the connection if
> +it does not recognize, while the server MUST close the TCP connection if
>  it does not recognize the client's flags.  What follows is a repeating
>  group of options. In non-fixed newstyle only one option can be set
>  (`NBD_OPT_EXPORT_NAME`), and it is not optional.
> @@ -150,8 +150,8 @@ S: 16 bits, transmission flags
>  S: 124 bytes, zeroes (reserved) (unless `NBD_FLAG_C_NO_ZEROES` was
>     negotiated by the client)  
>  
> -If the server is unwilling to allow the export, it SHOULD close the
> -connection.
> +If the server is unwilling to allow the export, it MUST terminate
> +the session.
>  
>  The reason that the flags field is 16 bits large and not 32 as in the
>  oldstyle negotiation is that there are now 16 bits of transmission flags,
> @@ -201,22 +201,60 @@ request before sending the next one of the same type. 
> The server MAY
>  send replies in the order that the requests were received, but is not
>  required to.
>  
> +#### Termination of the session during option haggling
> +
> +There are three possible mechanisms to end option haggling:
> +
> +* Transmission mode can be entered (by the client sending
> +  `NBD_OPT_EXPORT_NAME` or by the server responding to an
> +  `NBD_OPT_GO` with `NBD_REP_ACK`). This is documented
> +  elsewhere.
> +
> +* The client can send (and the server can reply to) an
> +  `NBD_OPT_ABORT`. This MUST be followed by the client
> +  shutting down TLS (if it is running), and the client
> +  dropping the connection. This is referred to as
> +  'initiating a soft disconnect'; soft disconnects can
> +  only be initiated by the client.
> +
> +* The client or the server can disconnect the TCP session
> +  without activity at the NBD protocol level. If TLS is
> +  negotiated, the party intitiating the transaction SHOULD
> +  shutdown TLS first if it is running. This is referrred
> +  to as 'initiating a hard disconnect'.
> +
> +This section concerns the second and third of these, together
> +called 'terminating the session', and under which circumstances
> +they are valid.
> +
> +If either the client or the server detects a violation of a
> +mandatory condition ('MUST' etc.) by the other party, it MAY
> +initiate a hard discconect.
> +
> +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)

[...]
> +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"?

[...]
> +#### Terminating the transmission phase
> +
> +There are two methods of terminating the transmission phase:
> +
> +* The client sends `NBD_CMD_DISC` whereupon the server MUST
> +  close down the TLS session (if one is running) and then
> +  close the TCP connection. This is referred to as 'initiating
> +  a soft disconnect'. Soft disconnects can only be
> +  initiated by the client.
> +
> +* 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'.
> +
> +Together these are refrred to as 'terminating transmission'.
> +
> +Either side MAY initiate a hard disconnect if it detects
> +a violation by the other party of a mandatory condition
> +within this document.
> +
> +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.

[...]
> -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?

[...]
>  - `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?

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Reply via email to