Regards,
Bala
On 10 December 2016 at 02:06, Bill Fischofer <bill.fischo...@linaro.org> wrote:
> Is this supposed to be instead of queue groups or in addition to queue
> group support?
Yes. Basically the changes is that this proposal merges queue group
and queue together.
>
> On Fri, Dec 9, 2016 at 5:54 AM, Balasubramanian Manoharan
> <bala.manoha...@linaro.org> wrote:
>> Adds support to spread packet from a single CoS to multiple queues by
>> configuring hashing at CoS level.
>>
>> Signed-off-by: Balasubramanian Manoharan <bala.manoha...@linaro.org>
>> ---
>> include/odp/api/spec/classification.h | 45
>> ++++++++++++++++++++++++++++++++---
>> 1 file changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/odp/api/spec/classification.h
>> b/include/odp/api/spec/classification.h
>> index 0e442c7..220b029 100644
>> --- a/include/odp/api/spec/classification.h
>> +++ b/include/odp/api/spec/classification.h
>> @@ -126,6 +126,12 @@ typedef struct odp_cls_capability_t {
>> /** Maximum number of CoS supported */
>> unsigned max_cos;
>>
>> + /** Maximun number of queue supported per CoS */
>
> typo: queues. Having the variable being max_queue_supported is OK, but
> a better name might be cos_max_queues or max_cos_queues to be explicit
> about this being a CoS limit.
>
>> + unsigned max_queue_supported;
>> +
>> + /** Protocol header combination supported for Hashing */
>> + odp_pktin_hash_proto_t hash_supported;
>> +
>> /** A Boolean to denote support of PMR range */
>> odp_bool_t pmr_range_supported;
>> } odp_cls_capability_t;
>> @@ -164,9 +170,25 @@ typedef enum {
>> * Used to communicate class of service creation options
>> */
>> typedef struct odp_cls_cos_param {
>> - odp_queue_t queue; /**< Queue associated with CoS */
>> - odp_pool_t pool; /**< Pool associated with CoS */
>> - odp_cls_drop_t drop_policy; /**< Drop policy associated with CoS
>> */
>> + /* Minimum number of queues to be linked to this CoS.
>> + * If the number is greater than 1 then hashing has to be
>> + * enabled */
>> + uint32_t num_queue;
>
> If an application wishes to assign a queue itself (as is done today)
> then is this set to 0 or 1? This should be stated explicitly. For
> consistency I'd think we'd have 0 = I'm supplying my own queue, >0 =
> implementation should create num_queues.
I thought about this also but having number of queues as 0 seems
contradicting to the basic behaviour of class of service.
>
>> + /* Denotes whether hashing is enabled for queues in this CoS
>> + * When hashing is enabled the queues are created by the
>> implementation
>> + * and application need not configure any queue to the class of
>> service
>> + */
>> + odp_bool_t enable_hashing;
>
> Why is this needed if it must be specified if num_queues >1 (and
> presumably shouldn't be specified otherwise)? This seems redundant.
IMO, I understand but having a boolean for enabling or disabling
hashing seemed cleaner since hashing is required to support more than
one queues.
>
>> + /* Protocol header fields which are included in packet input
>> + * hash calculation */
>> + odp_pktin_hash_proto_t hash;
>> + /* If hashing is disabled, then application has to configure
>> + * this queue and packets are delivered to this queue */
>> + odp_queue_t queue;
>
> Again, this seems like it would be clearer to state that this is
> supplied by the user if num_queues = 0 and ignored otherwise.
>
>> + /* Pool associated with CoS */
>> + odp_pool_t pool;
>> + /* Drop policy associated with CoS */
>> + odp_cls_drop_t drop_policy;
>> } odp_cls_cos_param_t;
>>
>> /**
>> @@ -209,6 +231,23 @@ int odp_cls_capability(odp_cls_capability_t
>> *capability);
>> odp_cos_t odp_cls_cos_create(const char *name, odp_cls_cos_param_t *param);
>>
>> /**
>> + * Queue hash result
>> + * Returns the queue within a CoS in which a particular packet will be
>> enqueued
>> + * based on the packet parameters and hash protocol field configured with
>> the
>> + * class of service.
>> + *
>> + * @param cos class of service
>> + * @param packet Packet handle
>> + *
>> + * @retval Returns the queue handle on which this packet will be
>> + * enqueued.
>> + * @retval ODP_QUEUE_INVALID for error case
>> + *
>> + * @note The packet has to be updated with valid header pointers L2, L3 and
>> L4.
>> + */
>> +odp_queue_t odp_queue_hash_result(odp_cos_t cos, odp_packet_t packet);
>
> What is the use case for this API? It's not obvious why this would be needed.
This mechanism is useful when the application wants to initialise any
context by knowing what will be the queue handle for expected packet
headers. This is required since the queues are created by the
implementation during hashing and application does not know the queue
handle beforehand. Maybe I will update the documentation.
-Bala
>
>> +
>> +/**
>> * Discard a class-of-service along with all its associated resources
>> *
>> * @param[in] cos_id class-of-service instance.
>> --
>> 1.9.1
>>
