The reason for having odp_cls_xxx is that class of service module
contains class of service and packet matching rule and we need a
common syntax to identify the module hence odp_cls_xxx is used as
common prefix.
I agree all the functions in classifier module needs to be changed to
add the prefix odp_cls_xxx.I will add the rename as a separate patch
for the remaining functions.
Regards,
Bala
On 17 December 2015 at 08:51, Bill Fischofer <bill.fischo...@linaro.org> wrote:
>
>
> On Wed, Dec 16, 2015 at 3:28 AM, Balasubramanian Manoharan
> <bala.manoha...@linaro.org> wrote:
>>
>> class of service create function now takes pool, queue, drop policy and
>> name as input parameters.
>> Adds class of service parameter structure odp_cls_cos_param_t and
>> initialization function odp_cls_cos_param_init()
>>
>> Signed-off-by: Balasubramanian Manoharan <bala.manoha...@linaro.org>
>> ---
>> v2: additional documentation and review comments from Petri
>>
>> include/odp/api/classification.h | 37
>> +++++++++++++++++++++++++++++++------
>> 1 file changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/odp/api/classification.h
>> b/include/odp/api/classification.h
>> index 725e1ab..4db5bf0 100644
>> --- a/include/odp/api/classification.h
>> +++ b/include/odp/api/classification.h
>> @@ -37,7 +37,7 @@ extern "C" {
>>
>> /**
>> * @def ODP_COS_INVALID
>> - * This value is returned from odp_cos_create() on failure,
>> + * This value is returned from odp_cls_cos_create() on failure,
>> * May also be used as a sink class of service that
>> * results in packets being discarded.
>> */
>> @@ -60,9 +60,9 @@ extern "C" {
>> */
>>
>> /**
>> - * Class-of-service packet drop policies
>> + * class of service packet drop policies
>> */
>> -typedef enum odp_cos_drop {
>> +typedef enum odp_cls_drop {
>> ODP_COS_DROP_POOL, /**< Follow buffer pool drop policy */
>> ODP_COS_DROP_NEVER, /**< Never drop, ignoring buffer pool
>> policy */
>> } odp_drop_e;
>> @@ -89,14 +89,39 @@ typedef enum odp_cos_hdr_flow_fields {
>> } odp_cos_hdr_flow_fields_e;
>>
>> /**
>> + * Class of service parameters
>> + * 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_drop_e drop_policy; /**< Drop policy associated with CoS */
>> +} odp_cls_cos_param_t;
>
>
> Why odp_cls_cos_param_t and not just odp_cos_param_t ? See remarks under
> odp_cls_cos_create(). If that's changed then it would be consistent to
> change this as well.
>
>>
>> +
>> +/**
>> + * Initialize class of service parameters
>> + *
>> + * Initialize an odp_cls_cos_param_t to its default value for all fields
>> + *
>> + * @param param Address of the odp_cls_cos_param_t to be initialized
>> + */
>> +void odp_cls_cos_param_init(odp_cls_cos_param_t *param);
>> +
>> +/**
>> * Create a class-of-service
>> *
>> - * @param[in] name String intended for debugging purposes.
>> + * @param name String intended for debugging purposes.
>> *
>> - * @return Class of service instance identifier
>> + * @param param class of service parameters
>> + *
>> + * @retval class of service handle
>> * @retval ODP_COS_INVALID on failure.
>> + *
>> + * @note ODP_QUEUE_INVALID and ODP_POOL_INVALID are valid values for
>> queue
>> + * and pool associated with a class of service and when any one of these
>> values
>> + * are configured as INVALID then the packets assigned to the CoS gets
>> dropped.
>> */
>> -odp_cos_t odp_cos_create(const char *name);
>> +odp_cos_t odp_cls_cos_create(const char *name, odp_cls_cos_param_t
>> *param);
>
>
> The additional parameter seems reasonable, but it's not clear why the
> function needs to be renamed cos already stands for Class of Service so
> odp_cls_cos_create() expands to ODP Classifier Class of Service Create which
> seems somewhat redundant. It's also inconsistent in that the created object
> is of type odp_cos_t, not odp_cls_cos_t and is non-symmetric with the
> existing odp_cos_destroy() function. I'd leave this as just
> odp_cos_create() with the additional parameter.
>>
>>
>> /**
>> * Discard a class-of-service along with all its associated resources
>> --
>> 1.9.1
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
--
Regards,
Bala
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
