Eric, On 11 May 2016, at 21:21, Eric Blake <[email protected]> wrote:
> Requiring a server to support 32M as its maximum block size > may be a bit large for some setups; rewrite things to make it > obvious that a server may advertise a maximum as small as 1M, > but that without an advertisement, there are existing clients > (hello, kernel NBD module) that expect 32M to work. Do we know the kernel never asks for more than 32M? Do we know what the maximum size of a TRIM sent by the kernel is? > Add some clarity that servers SHOULD NOT treat anything > smaller than 32M as a denial of service (and in contrast to > the recent patch for a 16k limit during option haggling), > and that denial-of-service limits may differ according to > command. That principle is fair enough. > Also add a way for servers to advertise on a per-command basis > when it is willing to accept larger limits because there is no > payload involved; the two obvious commands are NBD_CMD_TRIM and > NBD_CMD_WRITE_ZEROES, which now have NBD_INFO_TRIM_SIZE and > NBD_INFO_ZERO_SIZE. These new limits are listed separately > from NBD_INFO_BLOCK_SIZE since both commands are optional and > need not be implemented by a minimal server; but letting the > server advertise them makes sense since at least scsi devices > natively have three different size constraints for read/write, > unmap, and writesame. A server MAY honor larger trim requests > even without advertising, but a client cannot rely on that > working unless the additional sizes are advertised. I'd be interested what Wouter thinks here. I know SCSI does this. However, I wonder whether or not this is not overengineering and excess complexity. I think here we're in danger of pushing a pile of work onto the server which is in all fairness the client's responsibility. > The full text for NBD_INFO_ZERO_SIZE will be written later once > either that extension (or this one) is promoted to stable; it can > copy the pattern of NBD_INFO_TRIM_SIZE, except that where > CMD_TRIM can round the request (and only actually trim a subset > of the client request), CMD_WRITE_ZEROES must write actual zeroes > over the entire request (but if it can punch holes, only a > subset of the client request need result in holes). One problem with branches and extensions :-) > Signed-off-by: Eric Blake <[email protected]> > --- > > For the extension-info branch, and builds on the earlier > patch sent for the master branch about handshake limits > > doc/proto.md | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 79 insertions(+), 13 deletions(-) > > diff --git a/doc/proto.md b/doc/proto.md > index 5692be5..bf29b3a 100644 > --- a/doc/proto.md > +++ b/doc/proto.md > @@ -686,7 +686,9 @@ 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. > +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. OK subject to knowing whether 2^25 bytes is in fact the maximum the kernel and other clients say. > > The minimum block size represents the smallest addressable length and > alignment within the export, although writing to an area that small > @@ -710,29 +712,47 @@ preferred block size for that export. The server MAY > advertise an > export size that is not an integer multiple of the preferred block > size. > > -The maximum block size represents the maximum length that the server > -is willing to handle in one request. If advertised, it MUST be either > +The maximum block size represents the maximum payload length that the > +server is willing to handle in one request (whether the payload is > +from the client as in `NBD_CMD_WRITE` or from the server as in > +`NBD_CMD_READ`). So you're making this a payload length rather than a request length maximum? If we go down this route I'd rather make this a request length maximum, then have a list of 'exceptions' for different commands. After all, at least 2 servers I know implement (or have a patch to implement) WRITE_ZEROES simply by allocating the memory as zeroes and then doing a normal write. > If advertised, it SHOULD 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 ok subject to discussion on the value. > than a power of 2. 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 > maximum block size (as further constrained by a non-zero offset). > > +Some commands (such as `NBD_CMD_TRIM`) allow the client to specify > +*length* but do not involve a payload; for these commands, the client > +and server may negotiate additional information about larger size > +limits for those commands (such as `NBD_INFO_TRIM_SIZE`). For these > +commands, a client SHOULD NOT use a *length* larger than the > +negotiated maximum block size from `NBD_INFO_BLOCK_SIZE` unless the > +additional information was successfully negotiated. So again, I think I'd just say (if we're going to down this route) that it should be possible to send a list of: - 16 bits, `NBD_INFO_CMD_SIZE` - 16 bits, command number to which maximum / minimum apply - 32 bits, minimum command size - 32 bits, maximum command size and then try and touch the rest of the document as little as possible. I remain unconvinced about the need at all though! -- Alex Bligh ------------------------------------------------------------------------------ Mobile security can be enabling, not merely restricting. Employees who bring their own devices (BYOD) to work are irked by the imposition of MDM restrictions. Mobile Device Manager Plus allows you to control only the apps on BYO-devices by containerizing them, leaving personal data untouched! https://ad.doubleclick.net/ddm/clk/304595813;131938128;j _______________________________________________ Nbd-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/nbd-general
