Re: [PATCH V4] netif.h: Document xen-net{back, front} multi-queue feature

[Andrew Bennieston <andrew.bennieston@citrix.com>]

On 02/05/14 20:57, Konrad Rzeszutek Wilk wrote:
> On Fri, May 02, 2014 at 05:33:57PM +0100, Andrew J. Bennieston wrote:
>> From: "Andrew J. Bennieston" <andrew.bennieston@citrix.com>
>>
>> Document the multi-queue feature in terms of XenStore keys to be written
>> by the backend and by the frontend.
>>
>> Signed-off-by: Andrew J. Bennieston <andrew.bennieston@citrix.com>
>>
>> ---
>> V4
>> - Fix broken sentence regarging feature-split-event-channels
>>    in conjunction with multiple queues.
>>
>> ---
>>   xen/include/public/io/netif.h |   46 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
>> index d7fb771..c232c22 100644
>> --- a/xen/include/public/io/netif.h
>> +++ b/xen/include/public/io/netif.h
>> @@ -69,6 +69,52 @@
>>    */
>>
>>   /*
>> + * Multiple transmit and receive queues:
>> + * If supported, the backend will write the key "multi-queue-max-queues" to
>> + * the directory for that vif, and set its value to the maximum supported
>> + * number of queues.
>> + * Frontends that are aware of this feature and wish to use it can write the
>> + * key "multi-queue-num-queues", set to the number they wish to use.
>
> Um, should you say that said number has to be lower than 'multi-queue-max-queues'?
>
> It should be obvious but there are some devious folks who would write
> say '-1' in it for fun.

I can say that it has to be > 0 and <= multi-queue-max-queues. I assumed 
this was an obvious implication of the above statements.

>
>> + *
>> + * Queues replicate the shared rings and event channels.
>> + * "feature-split-event-channels" may optionally be used when using
>> + * multiple queues, but is not mandatory.
>> + *
>> + * Each queue consists of one shared ring pair, i.e. there must be the same
>> + * number of tx and rx rings.
>> + *
>> + * For frontends requesting just one queue, the usual event-channel and
>> + * ring-ref keys are written as before, simplifying the backend processing
>> + * to avoid distinguishing between a frontend that doesn't understand the
>> + * multi-queue feature, and one that does, but requested only one queue.
>> + *
>> + * Frontends requesting two or more queues must not write the toplevel
>> + * event-channel (or event-channel-{tx,rx}) and {tx,rx}-ring-ref keys,
>> + * instead writing those keys under sub-keys having the name "queue-N" where
>> + * N is the integer ID of the queue for which those keys belong. Queues
>> + * are indexed from zero. For example, a frontend with two queues and split
>> + * event channels may write the following set of queue-related keys:
>> + *
>
> Should you say what to be done if the multi-queue-num-queues = N doesn't
> match with the 'queue-N-1' values? Say you N=4 and there are just
> two sub-directories: queue-0 and queue-1 ?

I haven't given any thought to what the most sensible thing to do in 
this case is; the current implementation will bail entirely if it 
doesn't find _all_ of the keys it expects, for _all_ of the queues 
requested. It's possible that you might want to set up those that were 
completely specified , ignoring the rest, but I'd rather enforce that 
the XenStore data is self-consistent, i.e. if four queues were 
requested, the subkeys for four queues must also be present. This makes 
it easier to handle rogue or broken frontends (something missing or 
contradictory? Just tear down those queues already set up, and error 
out) and more obvious when something has gone wrong.

I'll update the docs to reflect this reasoning.

Andrew

>
>> + * /local/domain/1/device/vif/0/multi-queue-num-queues = "2"
>> + * /local/domain/1/device/vif/0/queue-0 = ""
>> + * /local/domain/1/device/vif/0/queue-0/tx-ring-ref = "<ring-ref-tx0>"
>> + * /local/domain/1/device/vif/0/queue-0/rx-ring-ref = "<ring-ref-rx0>"
>> + * /local/domain/1/device/vif/0/queue-0/event-channel-tx = "<evtchn-tx0>"
>> + * /local/domain/1/device/vif/0/queue-0/event-channel-rx = "<evtchn-rx0>"
>> + * /local/domain/1/device/vif/0/queue-1 = ""
>> + * /local/domain/1/device/vif/0/queue-1/tx-ring-ref = "<ring-ref-tx1>"
>> + * /local/domain/1/device/vif/0/queue-1/rx-ring-ref = "<ring-ref-rx1"
>> + * /local/domain/1/device/vif/0/queue-1/event-channel-tx = "<evtchn-tx1>"
>> + * /local/domain/1/device/vif/0/queue-1/event-channel-rx = "<evtchn-rx1>"
>> + *
>> + * Mapping of packets to queues is considered to be a function of the
>> + * transmitting system (backend or frontend) and is not negotiated
>> + * between the two. Guests are free to transmit packets on any queue
>> + * they choose, provided it has been set up correctly. Guests must be
>> + * prepared to receive packets on any queue they have requested be set up.
>> + */
>> +
>> +/*
>>    * "feature-no-csum-offload" should be used to turn IPv4 TCP/UDP checksum
>>    * offload off or on. If it is missing then the feature is assumed to be on.
>>    * "feature-ipv6-csum-offload" should be used to turn IPv6 TCP/UDP checksum
>> --
>> 1.7.10.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel