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

On Tue, Apr 18, 2023 at 11:26:40AM +0200, Wouter Verhelst wrote: > 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). > > .... > > @@ -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) Concur; how about squashing in: diff --git i/doc/proto.md w/doc/proto.md index 9098c42..7918179 100644 --- i/doc/proto.md +++ w/doc/proto.md @@ -409,7 +409,7 @@ cases, a server SHOULD gracefully consume *length* bytes of payload (even if it then replies with an `NBD_EINVAL` failure because the particular command was not expecting a payload), and proceed with the next client command. Thus, only when *length* is used as an -effective length will it utilize a full 64-bit value. +effect length will it utilize a full 64-bit value. #### Simple reply message @@ -841,24 +841,24 @@ exports. It is not possible to avoid downgrade attacks on exports which may be served either via TLS or in plain text unless the client insists on TLS. -## Block size constraints +## Size constraints 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 +as well as by three size constraints defined here (minimum block, preferred block, and maximum payload). -If a client can honour server block size constraints (as set out below +If a client can honour server size constraints (as set out below and under `NBD_INFO_BLOCK_SIZE`), it SHOULD announce this during the handshake phase by using `NBD_OPT_GO` (and `NBD_OPT_INFO` if used) with an `NBD_INFO_BLOCK_SIZE` information request, and MUST use `NBD_OPT_GO` rather than `NBD_OPT_EXPORT_NAME` (except in the case of a fallback where the server did not support `NBD_OPT_INFO` or `NBD_OPT_GO`). -A server with block size constraints other than the default SHOULD -advertise the block size constraints during handshake phase via +A server with size constraints other than the default SHOULD +advertise the size constraints during handshake phase via `NBD_INFO_BLOCK_SIZE` in response to `NBD_OPT_INFO` or `NBD_OPT_GO`, -and MUST do so unless it has agreed on block size constraints via out +and MUST do so unless it has agreed on size constraints via out of band means. Some servers are able to make optimizations, such as opening files @@ -866,11 +866,11 @@ with `O_DIRECT`, if they know that the client will obey a particular minimum block size, where it must fall back to safer but slower code if the client might send unaligned requests. For that reason, if a client issues an `NBD_OPT_GO` including an `NBD_INFO_BLOCK_SIZE` -information request, it MUST abide by the block size constraints it +information request, it MUST abide by the size constraints it receives. Clients MAY issue `NBD_OPT_INFO` with `NBD_INFO_BLOCK_SIZE` to learn the server's constraints without committing to them. -If block size constraints have not been advertised or agreed on +If size constraints have not been advertised or agreed on externally, then a server SHOULD support a default minimum block size of 1, a preferred block size of 2^12 (4,096), and a maximum payload size that is at least 2^25 (33,554,432) (even if the export @@ -886,12 +886,12 @@ a hard disconnect) or which uses `NBD_OPT_GO` without requesting that do not request sizing information when the server supports default sizing or where sizing constraints can be agreed on externally. When allowing clients that did not negotiate sizing via -NBD, a server that enforces stricter block size constraints than the +NBD, a server that enforces stricter size constraints than the defaults MUST cleanly error commands that fall outside the constraints without corrupting data; even so, enforcing constraints in this manner may limit interoperability. -A client MAY choose to operate as if tighter block size constraints +A client MAY choose to operate as if tighter size constraints had been specified (for example, even when the server advertises the default minimum block size of 1, a client may safely use a minimum block size of 2^9 (512)). @@ -1392,13 +1392,13 @@ of the newstyle negotiation. `NBD_REP_INFO` replies, but a SELECTIVETLS server MAY do so if this is a TLS-only export. - `NBD_REP_ERR_BLOCK_SIZE_REQD`: The server requires the client to - request block size constraints using `NBD_INFO_BLOCK_SIZE` prior + request size constraints using `NBD_INFO_BLOCK_SIZE` prior to entering transmission phase, because the server will be using non-default block sizes constraints. The server MUST NOT send this - error if block size constraints were requested with + error if size constraints were requested with `NBD_INFO_BLOCK_SIZE` with the `NBD_OPT_INFO` or `NBD_OPT_GO` request. The server SHOULD NOT send this error if it is using - default block size constraints or block size constraints + default size constraints or size constraints negotiated out of band. A server sending an `NBD_REP_ERR_BLOCK_SIZE_REQD` error SHOULD ensure it first sends an `NBD_INFO_BLOCK_SIZE` information reply in order @@ -1748,15 +1748,15 @@ during option haggling in the fixed newstyle negotiation. * `NBD_INFO_BLOCK_SIZE` (3) - Represents the server's advertised block size constraints; see the - "Block size constraints" section for more details on what these + Represents the server's advertised size constraints; see the + "Size constraints" section for more details on what these values represent, and on constraints on their values. The server MUST send this info if it is requested and it intends to enforce - block size constraints other than the defaults. After + size constraints other than the defaults. After sending this information in response to an `NBD_OPT_GO` in which the client specifically requested `NBD_INFO_BLOCK_SIZE`, the server can legitimately assume that any client that continues the session - will support the block size constraints supplied (note that this + will support the size constraints supplied (note that this assumption cannot be made solely on the basis of an `NBD_OPT_INFO` with an `NBD_INFO_BLOCK_SIZE` request, or an `NBD_OPT_GO` without an explicit `NBD_INFO_BLOCK_SIZE` request). The *length* MUST be 14, @@ -2644,7 +2644,7 @@ implement the following features: - Servers that implement block constraints through `NBD_INFO_BLOCK_SIZE` and desire maximum interoperability SHOULD NOT require them. Similarly, clients that desire maximum interoperability SHOULD - implement querying for block size constraints. Since some clients + implement querying for size constraints. Since some clients default to a block size of 512 bytes, implementations desiring maximum interoperability MAY default to that size. - Clients or servers that desire interoperability with older @@ -2652,7 +2652,7 @@ implement the following features: addition to `NBD_OPT_INFO` and `NBD_OPT_GO`. - For data safety, implementing `NBD_CMD_FLUSH` and the `NBD_CMD_FLAG_FUA` flag to `NBD_CMD_WRITE` is strongly recommended. - Clients that do not implement querying for block size constraints + Clients that do not implement querying for size constraints SHOULD abide by the rules laid out in the section "Block size constraints", above. - Servers that implement extended headers but desire interoperability