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

Re: [Nbd] [PATCH/RFCv2] Remove NBD_OPT_BLOCK_SIZE



Wouter,

Some comments in line.

On 26 Apr 2016, at 09:30, Wouter Verhelst <w@...112...> wrote:
> I understand (and share) the desire to keep things simple, but I think
> this is a mistake.

Well, that was the point of the RFC :-)

> We create extensions not only for people who implement the NBD protocol
> today, but also for future implementors. As such, I want to make as few
> as possible cases of "if you support feature A, you MUST also support
> feature B", because it allows a possible misreading of the standard and,
> as a result, implementation bugs.

Right. I agree with that, and that's why I proposed NBD_OPT_BLOCK_SIZE
in the first place.

However, in this instance the block sizing info comes through (and
*only* comes through) NBD_OPT_INFO. So in order to support block sizes
you need to support NBD_OPT_INFO because there is no other way to
obtain the block sizes anyway (*). The converse is really what we
are arguing about. Can you legitimately support NBD_OPT_INFO (as a
client) then ignore block sizes? I'm arguing that we can legitimately
make block size support part of the info spec, and say if you
ask for info, you must be prepared to deal with it. The alternate
view is that info and blocksizes are different things, in which case
the server needs some way of knowing whether the client will
respect block sizes if given.

(*) = I am ignoring negotiating block sizes out of band, because if
you negotiate block sizes out of band or indeed anything else
out of band, it can impose whatever SHOULD/MUST/MAY you like.

> In some cases this is unavoidable, of course. If you want to do STARTTLS
> you must also implement newstyle negotiation; but this is obvious by the
> fact that STARTTLS can be negotiated only by a newstyle negotiation, so
> even if you read the spec less than carefully, you'll notice that
> there's only one way to implement STARTTLS, and that is to implement
> newstyle.

Sure.

> The same isn't true for this proposed change. The idea of "NBD_OPT_INFO"
> is not implemented in terms of things provided by the block sizes
> extension (rather, the reverse is true). As such, people may miss the
> fact that if you say "get me some information on the export", you're now
> suddenly *required* to also align requests to these block sizes sent in
> the information replies. Worse, if we ever add a second extension that
> requires the client to signal that it will abide by the stuff the server
> laid out in NBD_REP_INFO messages, we suddenly have a situation where,
> for historical reasons, you need to acknowledge and affirm the use of
> one feature but not the other. This is going to confuse not only third
> parties but, I'm sure, us as well.

I think that's a fair argument. However, in not doing it we have
the opposite problem. People will think they can merely ask for info
about block sizes, get it back, and then not use it, forgetting to
send the NBD_OPT_BLOCK_SIZE option at all, and the server never know
when to optimise.

An alternative here would be to take a leaf out of DHCP's (dubious)
book. When asking for NBD_OPT_INFO you'd supply a list of extensions
you understanderstand (i.e. a list of the NBD_REP_INFO codes), and
you only get sent back the ones you ask for. The fact you've explicitly
asked for it CAN be taken to mean you've understood it.

I guess up to this point in the email, we are talking about how best
for a client to signal to the server that it can respect the
server's block size constraints. I proposed but then decided I
didn't much like NBD_OPT_BLOCK_SIZE. There's another idea above.
Yet another idea would be a single 'I understand block sizes'
bit in the flags the client sends right at the start of the negotiation
(perhaps mirrored by a server bit - I have something useful to
say about block sizes); I think I'd prefer that.

But we have more contentiousness in the following.

> In addition, while I understand the reasoning behind announcing block
> sizes, I do not want to make "abiding by announced block sizes" a
> prerequisite of "using the more modern way of choosing an export". The
> protocol currently has no requirement for a server to use a particular
> block size, and I think this is a feature. Sure, most clients *do*
> currently request data in block-sized units, but that doesn't mean we
> need to make that a requirement. A simple user-space client which wants
> to inspect (and possibly modify) some parts of an export might not care
> about block sizes; we should make writing such clients not harder than
> needs be.

Well, actually every server I know of ALREADY has block size constraints.
nbd-server.c is weakest, in that it won't support operations >= 1>>31
bytes. Qemu will fail stuff >32Mb. gonbdserver.c fails on stuff >128Mb
(and I won't be removing that - the DoS opportunity is too great).
Many servers perform substantially better if reads/writes are aligned.

If you write a server which has arcane block size constraints it
will fail if some dumb clients talk to it. I don't think that's an
issue. It's better they fail early than try to connect and fail on
the 37th write.

> In that same light, I think we should discourage (but allow) the use of
> NBD_REP_ERR_BLOCK_SIZE_REQD (although we might want to make that error's
> name a bit shorter -- boy that's a lot of typing) in the same way that
> we currently allow but discourage ENOMEM.

Don't agree.

If don't support block sizes under 4096 bytes (say) it's pretty
non-negotiable, or you risk something that fails obscurely later.

Much better to fail at negotiation stage if you really can't
support (or can't risk pretending to support) the client.

If what you are saying is 'it is unreasonable to use the nbd
protocol unless you can always somehow support byte-aligned
writes' I don't really agree. Just as you are arguing it should
be possible to write a simple client, I'd argue it should be possible
to write a simple server.

Alex

> 
> [...]
> --
> < ron> I mean, the main *practical* problem with C++, is there's like a dozen
>       people in the world who think they really understand all of its rules,
>       and pretty much all of them are just lying to themselves too.
> -- #debian-devel, OFTC, 2016-02-12

--
Alex Bligh




Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail


Reply to: