[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH] structured-reply: Tweak some protocol requirements



On Mon, Oct 16, 2017 at 10:25:24AM -0500, Eric Blake wrote:
> Several tweaks noticed while implementing structured reply for qemu:
> - Document what a server may do if the client tries to negotiate
> structured replies more than once.
> - Reformat the paragraph on NBD_CMD_FLAG_DF.
> - Mention what a client should do if a server sends an unexpected
> structured reply
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> Intended for the extension-structured-reply branch, after first merging
> master into that branch.  Preview available at:
> 
> git://repo.or.cz/nbd/ericb.git extension-structured-reply
> http://repo.or.cz/nbd/ericb.git/shortlog/refs/heads/extension-structured-reply
> 
>  doc/proto.md | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index 5c83cb1..4604a1e 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -1093,6 +1093,8 @@ of the newstyle negotiation.
>        server MUST use structured replies to the `NBD_CMD_READ`
>        transmission request.  Other extensions that require structured
>        replies may now be negotiated.
> +    - `NBD_REP_ERR_INVALID`: Structured replies have already been
> +      previously negotiated.

I'm not sure that's really a problem.

If a client wants to do structured replies, the server should ensure
that the bit "structured replies are okay" is set. If it's already set,
it's already set and we're fine.

The TLS situation is somewhat different; if a client wants to do TLS,
the server should fire up the TLS library and initiate the encryption.
Once that's already done, you can't do it again. There is no useful
response to that.

Having said that though, I don't feel too strongly about it.

>      - For backwards compatibility, clients SHOULD be prepared to also
>        handle `NBD_REP_ERR_UNSUP`; in this case, no structured replies
>        will be sent.
> @@ -1303,11 +1305,12 @@ valid may depend on negotiation during the handshake phase.
>    if `NBD_FLAG_SEND_TRIM` was not set in the transmission flags field.
>    The server MUST support the use of this flag if it advertises
>    `NBD_FLAG_SEND_WRITE_ZEROES`.
> -- bit 2, `NBD_CMD_FLAG_DF`; the "don't fragment" flag, valid during `NBD_CMD_READ`.
> -   SHOULD be set to 1 if the client requires the server to send at most one
> -   content chunk in reply.  MUST NOT be set unless the transmission
> -   flags include `NBD_FLAG_SEND_DF`.  Use of this flag MAY trigger an
> -   `EOVERFLOW` error chunk, if the request length is too large.
> +- bit 2, `NBD_CMD_FLAG_DF`; the "don't fragment" flag, valid during
> +  `NBD_CMD_READ`.  SHOULD be set to 1 if the client requires the
> +  server to send at most one content chunk in reply.  MUST NOT be set
> +  unless the transmission flags include `NBD_FLAG_SEND_DF`.  Use of
> +  this flag MAY trigger an `EOVERFLOW` error chunk, if the request
> +  length is too large.
> 
>  ##### Structured reply flags
> 
> @@ -1332,7 +1335,9 @@ unrecognized flags.
>  These values are used in the "type" field of a structured reply.
>  Some chunk types can additionally be categorized by role, such as
>  *error chunks* or *content chunks*.  Each type determines how to
> -interpret the "length" bytes of payload.
> +interpret the "length" bytes of payload.  If the client receives
> +an unknown or unexpected type, other than an *error chunk*, it
> +MUST initiate a hard disconnect.

Yeah, that one makes sense.

-- 
Could you people please use IRC like normal people?!?

  -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008
     Hacklab


Reply to: