Re: [Nbd] [Qemu-devel] [PATCH 3/1] doc: Propose Structured Replies extension

2016-03-29 Thread Eric Blake 
On 03/29/2016 12:19 PM, Wouter Verhelst wrote:
> On Tue, Mar 29, 2016 at 12:07:59PM -0600, Eric Blake wrote:
>> On 03/29/2016 12:03 PM, Wouter Verhelst wrote:
>>> On Tue, Mar 29, 2016 at 11:45:45AM -0600, Eric Blake wrote:
 Supporting DF merely transfers the burden of collection between server
 and client.  I suspect that there are cases where the server does NOT
 want to support DF (because it would require the server to allocate
 memory to collect the data before sending a single structured read
 reply),
>>>
>>> There are other ways to handle that; e.g., the server could have a
>>> "request too large for non-fragmented read" error message. The spec
>>> should give a minimum size that the server MUST support (which should be
>>> reasonably large), and should state that a server MAY reply to any
>>> request with DF set for a block larger than that minimum, with that
>>> error.
>>
>> How does 64k sound?
> 
> Dunno. It might make sense for this number to be based upon some
> "standard" minimum request size in things like ATA or SCSI if such a
> number exists there, but I don't know enough about either standard to
> answer that question myself.
> 
> If such a number doesn't exist (or nobody who knows speaks up soon
> enough), 64k is certainly good enough, I suppose.

And as mentioned in another email, we may want to propose an independent
extension that allows NBD_OPT_LIST and friends to start advertising the
minimum and preferred sizes of operations on a given export, where the
server can give hard errors if the client requests a read or write not
aligned to the minimum, and where the server must not fail a DF set for
anything smaller than preferred size.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [Qemu-devel] [PATCH 3/1] doc: Propose Structured Replies extension

2016-03-29 Thread Alex Bligh 

On 29 Mar 2016, at 19:03, Wouter Verhelst  wrote:

> There are other ways to handle that; e.g., the server could have a
> "request too large for non-fragmented read" error message. The spec
> should give a minimum size that the server MUST support (which should be
> reasonably large), and should state that a server MAY reply to any
> request with DF set for a block larger than that minimum, with that
> error.

Yeah something like that. Or the server could simply publish this
as part of the option negotiation.

-- 
Alex Bligh





--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [Qemu-devel] [PATCH 3/1] doc: Propose Structured Replies extension

2016-03-29 Thread Eric Blake 
On 03/29/2016 12:03 PM, Wouter Verhelst wrote:
> On Tue, Mar 29, 2016 at 11:45:45AM -0600, Eric Blake wrote:
>> On 03/29/2016 11:34 AM, Alex Bligh wrote:
>>> I would agree. I think if it supports the structured reply semantics,
>>> it should also support 'DF'. So if you know the server supports
>>> structured replies, you know you can set DF on them without any
>>> further requirements.
>>
>> Supporting DF merely transfers the burden of collection between server
>> and client.  I suspect that there are cases where the server does NOT
>> want to support DF (because it would require the server to allocate
>> memory to collect the data before sending a single structured read
>> reply),
> 
> There are other ways to handle that; e.g., the server could have a
> "request too large for non-fragmented read" error message. The spec
> should give a minimum size that the server MUST support (which should be
> reasonably large), and should state that a server MAY reply to any
> request with DF set for a block larger than that minimum, with that
> error.

How does 64k sound?

> 
> Otherwise the client could conceivably send out heaps of requests for
> (UINT32_MAX - 8) bytes with DF set and basically DoS the server.

Point taken.

> 
>> just as you have stated that there are cases where the client
>> has an additional burden if DF is not supported.  So for v2, I'm going
>> to explicitly document a DF export flag, and recommend (but not require)
>> that the server support it.
> 
> I'd prefer not to see that.

Okay, good thing I haven't sent v2 yet; I'm making more edits to drop
the export flag, and require unconditional support for the command flag
once structured reads are negotiated.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [Qemu-devel] [PATCH 3/1] doc: Propose Structured Replies extension

2016-03-29 Thread Eric Blake 
On 03/29/2016 11:34 AM, Alex Bligh wrote:
> 
> On 29 Mar 2016, at 16:12, Eric Blake  wrote:
>>>
>>> More a way of guaranteeing avoiding a fragmentation on 'simple' reads.
>>> Perhaps a 'DF' bit (don't fragment)! If the server doesn't like it, it
>>> can always error the command.
>>
>> Okay, that makes sense.  Does reusing NBD_CMD_FLAG_FUA sound reasonable
>> for this purpose, or should it be a new flag?  I guess from the
>> standpoint of client/server negotiation, we want to support this
>> don't-fragment request even if NBD_FLAG_SEND_FUA was not listed in
>> export flags, so it sounds like a new flag is best.
> 
> I think it should be separate from FUA, as there are possibly
> servers out there that support FUA but not this, but ...
> 
>> Next, should it be
>> independently negotiated, or implied by negotiating
>> NBD_FLAG_C_STRUCTURED_REPLIES?  I'm leaning towards implied - it's
>> all-or-none for use of the improved read structures.
> 
> I would agree. I think if it supports the structured reply semantics,
> it should also support 'DF'. So if you know the server supports
> structured replies, you know you can set DF on them without any
> further requirements.

Supporting DF merely transfers the burden of collection between server
and client.  I suspect that there are cases where the server does NOT
want to support DF (because it would require the server to allocate
memory to collect the data before sending a single structured read
reply), just as you have stated that there are cases where the client
has an additional burden if DF is not supported.  So for v2, I'm going
to explicitly document a DF export flag, and recommend (but not require)
that the server support it.

> 
>> I'm also leaning
>> towards the name NBD_FLAG_C_STRUCTURED_READ, since elsewhere I'm
>> documenting that negotiating this particular global flag affects only
>> the replies to NBD_CMD_READ (other commands may use structured replies,
>> but those commands will be independently negotiated).
> 
> I suspect the name is the thing that makes the least difference, and
> hence do not feel strongly at all, but:
> 
> a) Why '_C_'?

Matches existing convention on client flags; but Wouter's idea of using
NBD_OPT_ instead of global/client flags is better, so the _C_ disappears
in v2.

> 
> b) Throughout the current documentation you've called them 'structured
>replies', not 'structured reads' and said that in the future multiple
>commands might support them. So you should arguably call the flag
>'*_STRUCTURED_REPLY' or change the text.

I'm changing the text, and favoring the name STRUCTURED_READ except in
the description of the transmission phase, where Structured Reply is the
header used for ANY form of reply with data (to make it more obvious
that structured read is a subset of structured replies), while at the
same time emphasizing that NBD_CMD_READ is the only command that can get
away with data in a non-structured reply, and only when structured read
was not negotiated.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [Qemu-devel] [PATCH 3/1] doc: Propose Structured Replies extension

2016-03-29 Thread Alex Bligh 

On 29 Mar 2016, at 16:12, Eric Blake  wrote:
>> 
>> More a way of guaranteeing avoiding a fragmentation on 'simple' reads.
>> Perhaps a 'DF' bit (don't fragment)! If the server doesn't like it, it
>> can always error the command.
> 
> Okay, that makes sense.  Does reusing NBD_CMD_FLAG_FUA sound reasonable
> for this purpose, or should it be a new flag?  I guess from the
> standpoint of client/server negotiation, we want to support this
> don't-fragment request even if NBD_FLAG_SEND_FUA was not listed in
> export flags, so it sounds like a new flag is best.

I think it should be separate from FUA, as there are possibly
servers out there that support FUA but not this, but ...

> Next, should it be
> independently negotiated, or implied by negotiating
> NBD_FLAG_C_STRUCTURED_REPLIES?  I'm leaning towards implied - it's
> all-or-none for use of the improved read structures.

I would agree. I think if it supports the structured reply semantics,
it should also support 'DF'. So if you know the server supports
structured replies, you know you can set DF on them without any
further requirements.

> I'm also leaning
> towards the name NBD_FLAG_C_STRUCTURED_READ, since elsewhere I'm
> documenting that negotiating this particular global flag affects only
> the replies to NBD_CMD_READ (other commands may use structured replies,
> but those commands will be independently negotiated).

I suspect the name is the thing that makes the least difference, and
hence do not feel strongly at all, but:

a) Why '_C_'?

b) Throughout the current documentation you've called them 'structured
   replies', not 'structured reads' and said that in the future multiple
   commands might support them. So you should arguably call the flag
   '*_STRUCTURED_REPLY' or change the text.

>>> Sure.  But keep in mind that if (when?) we add a flag for allowing the
>>> server to skip read chunks on holes, we'll have to tweak the wording to
>>> allow the server to send fewer chunks than the client's length, where
>>> the client must then assume zeroes for all chunks not received.
>> 
>> Or alternatively a chunk representing a hole. I wonder whether you
>> might be better to extend the chunk structure by 4 bytes to allow for
>> future modifications like this (e.g. NBD_CHUNK_FLAG_HOLE means
>> the chunk is full of zeroes and has no payload).
> 
> Seems reasonable (then I need to reword everything from len-8 to instead
> be len-12 when dealing with data size within the len bytes of payload).
> Network traffic-wise, I think it's better to always send the chunk
> flags, than it would be to try and make the sending of the chunk flags
> dependent on the client's choice of command flags (that is, we already
> argued that wire format should never be changed based merely on command
> flags, as it makes the server stream harder to decipher in isolation).

Absolutely. And that way if we have to add anything to the chunk (e.g.
we run out of flags!), we can use one or more bits to indicate this.

> Thanks for the good feedback, by the way; it will make v2 better.

No problem. Some time ago I rewrote chunks of the nbd test suite and
wrote the bit that tested parallel outstanding commands. At the back
of my mind is whether I should extend the test suite to test this
and how we could persuade a server to 'often fragment' so we can
test reassembly (some form of debug setting on the server like
'max fragment size' or similar I suspect).

--
Alex Bligh






signature.asc
Description: Message signed with OpenPGP using GPGMail
--
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471=/4140___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general