On Fri, Apr 8, 2022 at 6:47 PM Eric Blake <[email protected]> wrote: > > On Fri, Apr 08, 2022 at 04:48:59PM +0300, Nir Soffer wrote: ... > > > BTW attached is an nbdkit plugin that creates an NBD server that > > > responds with massive numbers of byte-granularity extents, in case > > > anyone wants to test how nbdkit and/or clients respond: > > > > > > $ chmod +x /var/tmp/lots-of-extents.py > > > $ /var/tmp/lots-of-extents.py -f > > > > > > $ nbdinfo --map nbd://localhost | head > > > 0 1 3 hole,zero > > > 1 1 0 data > > > 2 1 3 hole,zero > > > 3 1 0 data > > > 4 1 3 hole,zero > > > 5 1 0 data > > > 6 1 3 hole,zero > > > 7 1 0 data > > > 8 1 3 hole,zero > > > 9 1 0 data > > > $ nbdinfo --map --totals nbd://localhost > > > 524288 50.0% 0 data > > > 524288 50.0% 3 hole,zero > > > > This is a malicious server. A good client will drop the connection when > > receiving the first 1 byte chunk. > > Depends on the server. Most servers don't serve 1-byte extents, and > the NBD spec even recommends that extents be at least 512 bytes in > size, and requires that extents be a multiple of any minimum block > size if one was advertised by the server. > > But even though most servers don't have 1-byte extents does not mean > that the NBD protocol must forbid them.
Forbidding this simplifies clients without limiting real world use cases. What is a reason to allow this? > > The real issue here is not enforcing or suggesting a limit on the number of > > extent the server returns, but enforcing a limit on the minimum size of > > a chunk. > > > > Since this is the network *block device* protocol it should not allow chunks > > smaller than the device block size, so anything smaller than 512 bytes > > should be invalid response from the server. > > No, not an invalid response, but merely a discouraged one - and that > text is already present in the wording of NBD_CMD_BLOCK_STATUS. My suggestion is to make it an invalid response, because there are no block devices that can return such a response. > > Even the last chunk should not be smaller than 512 bytes. The fact that you > > can serve a file with size that is not aligned to 512 bytes does not mean > > that > > the export size can be unaligned to the logical block size. There are no > > real > > block devices that have such alignment so the protocol should not allow > > this. > > A good server will round the file size down the logical block size to avoid > > this > > issue. > > > > How about letting the client set a minimum size of a chunk? This way we > > avoid the issue of limiting the number of chunks. Merging small chunks > > is best done on the server side instead of wasting bandwidth and doing > > this on the client side. > > The client can't set the minimum block size, but the server can > certainly advertise one, and must obey that advertisement. Or are you > asking for a new extension where the client mandates what the minimum > granularity must be from the server in responses to NBD_CMD_READ and > NBD_CMD_BLOCK_STATUS, when the client wants a larger granularity than > what the server advertises? That's a different extension than this > patch, but may be worth considering. Yes, this should really be discussed in another thread. Nir
