Re: [PATCH v3 2/6] spec: Change maximum block size to maximum payload size

[Wouter Verhelst]

On Thu, Apr 13, 2023 at 05:02:37PM -0500, Eric Blake wrote:
> Commit 9f30fedb improved the spec to allow non-payload requests like
> NBD_CMD_TRIM that exceed any advertised maximum block size.  Take this
> one step further by documenting that the server may use NBD_EOVERFLOW
> as a hint to the client when a non-payload request is oversize (while
> permitting NBD_EINVAL for back-compat), and by rewording the text to
> explicitly call out that what is being advertised is the maximum
> payload length, not maximum block size.  Furthermore, favor the term
> 'maximum payload size' instead of 'maximum block size', as the real
> limitation here is how many bytes are sent in one direction as part of
> the command (a maximum payload size may be related to maximum block
> size, but existing implementations of both servers and clients that
> actually implement NBD_INFO_BLOCK_SIZE have generally been advertising
> things like a 32M or 64M data cap, and not an underlying block size
> constraint).
> 
> Document existing practice that structured replies can slightly exceed
> payload size (a client requesting a 32M NBD_CMD_READ can get a single
> NBD_REPLY_TYPE_OFFSET_DATA of size 32M+8 bytes, rather than the server
> splitting it across two chunks); the only hard limit here is that on
> client/server pairs that permit larger payloads than 32M, the reply
> type still has a 32-bit limit on payload size (no known client or
> server actually tries to do an NBD_CMD_READ of 4G-1 bytes, but the
> spec shouldn't prevent it).
> 
> This becomes more important when we add 64-bit extensions, where it
> becomes possible to extend `NBD_CMD_BLOCK_STATUS` to have both an
> effect length (how much of the image does the client want status on -
> may be larger than 32 bits) and an optional payload length (a way to
> filter the response to a subset of negotiated metadata contexts).  In
> the shorter term, it means that a server may (but not must) accept a
> read request larger than the maximum block size if it can use
> structured replies to keep each chunk of the response under the
> maximum payload limits.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> 
> v3: Completely drop 'maximum block length', and reword things even
> more to emphasize maximum payload.  Clarify that a server's structured
> reply CAN exceed the maximum payload by the amount of overhead
> required by the reply type.
> ---
>  doc/proto.md | 148 ++++++++++++++++++++++++++++++---------------------
>  1 file changed, 87 insertions(+), 61 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index 0cb84f2..2651f13 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -747,8 +747,8 @@ text unless the client insists on TLS.
> 
>  During transmission phase, several operations are constrained by the
>  export size sent by the final `NBD_OPT_EXPORT_NAME` or `NBD_OPT_GO`,
> -as well as by three block size constraints defined here (minimum,
> -preferred, and maximum).
> +as well as by three block size constraints defined here (minimum
> +block, preferred block, and maximum payload).

I think this may be reworded as:

"as well as by three size constraint defined here"

as they're now no longer all block size constraints.

(this occurs more below)

Other than that,

Reviewed-By: Wouter Verhelst <w@uter.be>

-- 
     w@uter.{be,co.za}
wouter@{grep.be,fosdem.org,debian.org}

I will have a Tin-Actinium-Potassium mixture, thanks.