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

Re: [PATCH 1/2] doc: Clarify maximum size limits



On 03/01/2018 08:40 AM, Vladimir Sementsov-Ogievskiy wrote:

+If block 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 block size
+of the smaller of the export size or 0xffffffff (effectively
+unlimited), while a client desiring maximum interoperability SHOULD

You don't change it, but "the smaller of the export size or 0xffffffff" - is it right wording? for me it sounds something like "(the smaller of the export size) or 0xffffffff", so I've spent some time to understand.. should not it be "the smaller of the export size and 0xffffffff"? Or may be "the minimum from export size and 0xffffffff"..

// I understand that it is a consequence of my poor knowledge of English, just leave it as is, if it is ok.

Since two people questioned, I'm trying to improve the wording. The intent is "the smaller of (the export size, 0xffffffff)". Here's what I'm settling on:

"and a maximum block size that is effectively unlimited (0xffffffff, or the export size if that is smaller)"


+constrain its requests to a minimum block size of 2^9 (512), and limit
+`NBD_CMD_READ` and `NBD_CMD_WRITE` commands to a maximum block size of
+2^25 (33,554,432).

note: the only significant change of the paragraph is this new sentence ^^^

Yeah, paragraph reflowing makes it harder to spot diffs.


  A server that wants to enforce block sizes other
+than the defaults specified here MAY refuse to go into transmission
+phase with a client that uses `NBD_OPT_EXPORT_NAME` (via a hard
+disconnect) or which fails to use `NBD_INFO_BLOCK_SIZE` with
+`NBD_OPT_GO` (where the server uses `NBD_REP_ERR_BLOCK_SIZE_REQD`),
+although a server SHOULD permit such clients if block size constraints
+are the default or can be agreed on externally.  When allowing such
+clients, the server MUST cleanly error commands that fall outside
+block size constraints without corrupting data; even so, this may
+limit interoperability.

not changed, but I can't understand.
what means "clients, which fails to use" ?

A client that used NBD_OPT_GO, but didn't pass NBD_INFO_BLOCK_SIZE as one of the INFO items requested during NBD_OPT_GO. I'll reword that to:

"MAY refuse to go into transmission phase with a client that uses `NBD_OPT_EXPORT_NAME` (via a hard disconnect) or which uses `NBD_OPT_GO` without requesting `NBD_INFO_BLOCK_SIZE` (where the server uses `NBD_REP_ERR_BLOCK_SIZE_REQD`)


and why we say that server which "wants to enforce block sizes _OTHER_ than the defaults" .. "SHOULD permit such clients if block size constraints _ARE_ the default" ? I don't follow =(

It's been a while since we originally came up with that wording, but I seem to recall that we were trying to state that if a server was planning on advertising 1/4096/0xffffffff but the client never asked for sizes, then the server should not reject the client merely because the client didn't ask.

. And where server should get these constrains?

Out-of-band communication includes cases like "we implement both the client and server, and thus know that both sides agree to abide by a 512-byte sector alignment even without using NBD_OPT_GO". The point the spec was trying to make is that maximum interoperability demands servers to be flexible to any client, but when you have control over a specific client and server, you can rely on additional guarantees not available to the generic cross-implementation setup.



-A client MAY choose to operate as if tighter block 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), a preferred block size of 2^16 (65,536), and a maximum
-block size of 2^25 (33,554,432)).  Notwithstanding any maximum block
-size advertised, either the server or the client MAY initiate a hard
-disconnect if the size of a request or a reply is large enough to be
-deemed a denial of service attack.
+A client MAY choose to operate as if tighter block 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)).

it's obvious. Client may operate with requests length which are multiply of minimum-block-size, and the example is just a case of this definition.. Is it worth saying?


  The minimum block size represents the smallest addressable length and
  alignment within the export, although writing to an area that small
@@ -788,8 +788,8 @@ The maximum block size represents the maximum length that the server
  is willing to handle in one request.  If advertised, it MUST be either
  an integer multiple of the minimum block size or the value 0xffffffff
  for no inherent limit, MUST be at least as large as the smaller of the
-preferred block size or export size, and SHOULD be at least 2^25
-(33,554,432) if the export is that large, but MAY be something other
+preferred block size or export size, and SHOULD be at least 2^20
+(1,048,576) if the export is that large, but MAY be something other
  than a power of 2.

ohh.. it must, must and should if but may..

must:
  1. max-block = multiply of min-block or 0xffffffff
  2. max-block >= min(pref-block, exp-size)


Correct.

if export is that large.. what is "that"? 2^20? so,

      if export-size = 2^20: max-block should be at least 2^20

The difference between this statement


but it follows from (2.)... or, even if it is

      if export-size => 2^20: max-block should be at least 2^20

and this one was '=' vs '=>'.


it follows from (2.) anyway

You're right that point 2 means that if exp-size <= 2^20, then max-block < 2^20 is permissible (as long as max-block >= exp-size). If the first three clauses were all MUST (instead of the third clause being SHOULD), you could write:

max-block >= MIN(exp-size, pref-block, 2^20)

The third clause being a SHOULD instead of MUST means that a server that advertises max-block < 2^20, even when exp->size > 2^20, is not ideal but also not breaking the protocol (a client is not required to deal with a server like that, but a client that cares about maximum interoperability will manage with the smaller max-block just fine, even if it means more overhead because requests have to be chopped into smaller pieces).


and, "but MAY be something...", looks like this "but.." is under "if the export is that large",
this "but..." works only if "the export is that large".

There are four clauses, the 'but' starts the fourth clause without reference to any of the earlier three (I used 'but' because this clause uses MAY rather than MUST/SHOULD). Easier to read might be:

If advertised, it MAY be something other than a power of 2, but MUST be either an integer multiple of the minimum block size or the value 0xffffffff for no inherent limit, MUST be at least as large as the smaller of the preferred block size or export size, and SHOULD be at least 2^20 (1,048,576) if the export is that large.

We also explicitly chose to allow 0xffffffff as max-block even when min-block is greater than 1, rather than requiring servers to round down to an aligned boundary, although a client can't request that much without violating min-block alignment constraints.


// I understand, that this should be actually about minimum possible max-block-size,
    and the only change is to reduce it from 2^25 to 2^20....

Yes.


   For convenience, the server MAY advertise a
  maximum block size that is larger than the export size, although in
  that case, the client MUST treat the export size as the effective
@@ -804,9 +804,17 @@ those requests, the client MUST NOT use a *length* larger than any
  advertised 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 max-block-size is 2^20... (just note)

The difference here is intentional - even if the max-block-size is 2^20, a well-behaved server will allow a client to send an NBD_CMD_WRITE with 2^25 bytes and reply gracefully with EINVAL (for alignment constraint violation) rather than disconnecting abruptly (the server may have to loop up to 2^5 times over its 2^20 receive buffer to clear out the client's request); but a client sending 2^26 bytes can be disconnected (as the length gets longer, the amount of time spent dealing with that much garbage over the wire in order to just send EINVAL and move on to the next request is not worth it, and a disconnect is appropriate rather than continuing to talk to a client that is violating max-block that badly). But you're right that a note that 2^25 should be graceful even when max-block is smaller is worth making.


  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.

I'll send a v2 with wording tweaks based on your review.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


Reply to: