On 03/29/2018 11:34 AM, Edgar Kaziakhmedov wrote:
@@ -804,9 +806,18 @@ those requests, the client MUST NOT use a *length* larger than anyadvertised maximum block size or which, when added to *offset*, would exceed the export size. The server SHOULD report an `EINVAL` error if the client's request is not aligned to advertised minimum block size -boundaries, or is larger than the advertised maximum block size, -although the server MAY instead initiate a hard disconnect if a large -*length* could be deemed as a denial of service attack. +boundaries, or is larger than the advertised maximum block size. +Notwithstanding any maximum block size advertised, either the server +or the client MAY initiate a hard disconnect if the payload of an +`NBD_CMD_WRITE` request or `NBD_CMD_READ` reply would be large enough +to be deemed a denial of service attack; however, for maximum +portability, any length less than 2^25 (33,554,432) bytes SHOULD NOT +be considered a denial of service attack (even if the advertised +maximum block size is smaller).So, as I get it from this paragraph, the 'NBD_CMD_WRITE' and 'NBD_CMD_READ' requests should be rejected with disconnection if the payload size is greater than maximum block size.
Not SHOULD, but MAY. Existing implementations do disconnect, some at 32M, some at 64M, on the grounds that large packets over the network can be exploited into Denial-of-Service attacks; but there's nothing wrong with an implementation that handles a 4G-1 packet if it does not choose to disconnect..
But you also mentioned that we could choose smaller maximum block size for WRITE and READ and exceeding one should not lead to hard disconnection. So, may be I missed something, but what is the purpose of such kind of exception for both requests?
We're trying to document the portability boundaries from both endpoints: a server SHOULD NOT disconnect for anything smaller than 32M, even though it may reject that write attempt with EINVAL for being larger than its max block size (where max block can be as small as 1M), because portable clients assume that 32M works, and because a client need not understand block size advertisements (backwards compatibility to clients that do not use NBD_OPT_GO).
Conversely, a portable client shouldn't send more than 32M in one write request to an unknown server (portability to older servers that don't use NBD_OPT_GO) so that it doesn't risk getting disconnected; but when a maximum block size has been communicated, then the client may safely change its maximum write size to match (whether smaller or larger).
Overall, this wording looks good to me.For all other commands, where the +*length* is not reflected in the payload (such as `NBD_CMD_TRIM` or +`NBD_CMD_WRITE_ZEROES`), a server SHOULD merely fail the command with +an `EINVAL` error for a client that exceeds the maximum block size, +rather than initiating a hard disconnect. ## Values
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org