Regards,
Bala

On 2 February 2016 at 14:53, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolai...@nokia.com> wrote:
>
>
>> -----Original Message-----
>> From: EXT Balasubramanian Manoharan [mailto:bala.manoha...@linaro.org]
>> Sent: Monday, February 01, 2016 2:56 PM
>> To: lng-odp@lists.linaro.org
>> Cc: Savolainen, Petri (Nokia - FI/Espoo); Balasubramanian Manoharan
>> Subject: [API-NEXT PATCHv2 1/4] api: classification: add pmr create api
>>
>> Packet match rule creation is modified to include source and destination
>> class of service. Removes the ability to add any class of service directly
>> with pktio. If a PMR needs to be applied at the pktio level the same
>> should be applied to default class of service.
>>
>> Packet match rule destroy function is updated to removes the link between
>> the source and destination class of service.
>>
>> odp_pmr_match_set_t handle is removed and pmr create function is modified
>> to take number of terms and composite rule is created by providing more
>> than one term.
>>
>> Signed-off-by: Balasubramanian Manoharan <bala.manoha...@linaro.org>
>> ---
>> v2: removes pmr match set
>>
>>  include/odp/api/classification.h | 142 ++++++++++------------------------
>> -----
>>  1 file changed, 35 insertions(+), 107 deletions(-)
>>
>> diff --git a/include/odp/api/classification.h
>> b/include/odp/api/classification.h
>> index f46912e..263ea7c 100644
>> --- a/include/odp/api/classification.h
>> +++ b/include/odp/api/classification.h
>> @@ -50,16 +50,11 @@ extern "C" {
>>  /**
>>   * @def ODP_PMR_INVAL
>>   * Invalid odp_pmr_t value.
>> - * This value is returned from odp_pmr_create()
>> + * This value is returned from odp_cls_pmr_create()
>>   * function on failure.
>>   */
>>
>>  /**
>> - * @def ODP_PMR_SET_INVAL
>> - * Invalid odp_pmr_set_t value.
>> - */
>> -
>> -/**
>>   * class of service packet drop policies
>>   */
>>  typedef enum {
>> @@ -286,50 +281,54 @@ typedef struct odp_pmr_match_t {
>>  } odp_pmr_match_t;
>>
>>  /**
>> - * Create a packet match rule with mask and value
>> + * Create a packet match rule between source and destination class of
>> service.
>> + * This packet matching rule is applied on all packets arriving at the
>> source
>> + * class of service and packets satisfying this PMR are sent to the
>> destination
>> + * class of service.
>> + * A composite PMR rule is created when the number of terms in the match
>> rule
>> + * is more than one. The composite rule is considered as matching only if
>> + * the packet satisfies all the terms in Packet Match Rule.
>> + * The underlying platform may not support all or any specific
>> combination
>> + * of value match rules, and the application should take care
>> + * of inspecting the return value when installing such rules, and perform
>> + * appropriate fallback action.
>>   *
>> - * @param[in]        match   packet matching rule definition
>> + * @param[in]        num_terms       Number of terms in the match rule.
>> + * @param[in]        terms           Array of odp_pmr_match_t entries, one 
>> entry
>> per
>> + *                           term desired.
>> + * @param[in]        src_cos         source CoS handle
>> + * @param[in]        dst_cos         destination CoS handle
>>   *
>> - * @return           Handle of the matching rule
>> - * @retval           ODP_PMR_INVAL on failure
>> + * @retval                   Hanlde to the Packet Match Rule.
>
> Typo                          ^^^^^^
>
> Use @return instead of @retval, when the value is not predefined (a valid 
> handle).

Okay.
>
>
>> + * @retval                   ODP_PMR_INVAL on failure
>>   */
>> -odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
>> +odp_pmr_t odp_cls_pmr_create(int num_terms, const odp_pmr_match_t *terms,
>> +                          odp_cos_t src_cos, odp_cos_t dst_cos);
>
>
> To match similar function prototypes in other APIs (e.g. xxx_multi()):
> - first the table as an array
> - then number of elements in the array
>
> odp_pmr_t odp_cls_pmr_create(const odp_pmr_match_t terms[], int num_terms, 
> odp_cos_t src_cos, odp_cos_t dst_cos);
>
>
> For example, it's easier to understand what "1" (or num) stands for when 
> defined like this ...
>
> odp_cls_pmr_create(terms, 1, src_cos, dst_cos);
>
> ... instead of this.
>
> odp_cls_pmr_create(1, terms, src_cos, dst_cos);

Agreed.
>
>
>
>
>
>>
>>  /**
>> - * Invalidate a packet match rule and vacate its resources
>> + * Function to destroy a packet match rule
>> + * Destroying a PMR removes the link between the source and destination
>> + * class of service and this PMR will no longer be applied for packets
>> arriving
>> + * at the source class of service. All the resource associated with the
>> PMR
>> + * be release but the class of service will remain intact.
>> + * Depending on the implementation details, destroying a composite rule
>> + * may not guarantee the availability of hardware resources to create the
>> + * same or essentially similar rule.
>>   *
>>   * @param[in]        pmr_id  Identifier of the PMR to be destroyed
>>   *
>>   * @retval           0 on success
>>   * @retval           <0 on failure
>>   */
>> -int odp_pmr_destroy(odp_pmr_t pmr_id);
>> +int odp_cls_pmr_destroy(odp_pmr_t pmr_id);
>>
>> -/**
>> - * Apply a PMR to a pktio to assign a CoS.
>> - *
>> - * @param[in]        pmr_id          PMR to be activated
>> - * @param[in]        src_pktio       pktio to which this PMR is to be 
>> applied
>> - * @param[in]        dst_cos         CoS to be assigned by this PMR
>> +/* Return the maximum number of packet matching terms supported for
>> + * a Packet match rule.
>>   *
>> - * @retval           0 on success
>> - * @retval           <0 on failure
>> + * @param[out]       Maximum number of packet matching terms per
>> + *           PMR rule.
>>   */
>> -int odp_pktio_pmr_cos(odp_pmr_t pmr_id,
>> -                   odp_pktio_t src_pktio, odp_cos_t dst_cos);
>> -
>> -/**
>> - * Cascade a PMR to refine packets from one CoS to another.
>> - *
>> - * @param[in]        pmr_id          PMR to be activated
>> - * @param[in]        src_cos         CoS to be filtered
>> - * @param[in]        dst_cos         CoS to be assigned to packets filtered
>> - *                           from src_cos that match pmr_id.
>> - *
>> - * @retval           0 on success
>> - * @retval           <0 on failure
>> - */
>> -int odp_cos_pmr_cos(odp_pmr_t pmr_id, odp_cos_t src_cos, odp_cos_t
>> dst_cos);
>> +int odp_cls_pmr_num_terms(void);
>
>
> I think we could leave this out from this patch set, and introduce 
> odp_cls_capability_t and related function in another series.
>
> The capability struct should include:
> - this maximum num terms in pmr
> - may be also maximum total number of odp_cos_t and odp_pmr_t
> - Supported number of terms and actual terms as a bit field structure. Which 
> would make odp_pmr_terms_avail() and odp_pmr_terms_cap() redundant.
> - max (root) pmrs per pktio
>

Okay. So I will remove this function in this patch and add the
capability struct as a separate patch.

>
> -Petri
>
>
>
>>
>>  /**
>>   * Inquire about matching terms supported by the classifier
>> @@ -346,64 +345,6 @@ unsigned long long odp_pmr_terms_cap(void);
>>  unsigned odp_pmr_terms_avail(void);
>>
>>  /**
>> - * @typedef odp_pmr_set_t
>> - * An opaque handle to a composite packet match rule-set
>> - */
>> -
>> -/**
>> - * Create a composite packet match rule in the form of an array of
>> individual
>> - * match rules.
>> - * The underlying platform may not support all or any specific
>> combination
>> - * of value match rules, and the application should take care
>> - * of inspecting the return value when installing such rules, and perform
>> - * appropriate fallback action.
>> - *
>> - * @param[in]        num_terms       Number of terms in the match rule.
>> - * @param[in]        terms           Array of num_terms entries, one entry 
>> per
>> - *                           term desired.
>> - * @param[out]       pmr_set_id      Returned handle to the composite rule 
>> set.
>> - *
>> - * @return                   the number of terms elements
>> - *                           that have been successfully mapped to the
>> - *                           underlying platform classification engine
>> - * @retval                   <0 on failure
>> - */
>> -int odp_pmr_match_set_create(int num_terms, const odp_pmr_match_t *terms,
>> -                          odp_pmr_set_t *pmr_set_id);
>> -
>> -/**
>> - * Function to delete a composite packet match rule set
>> - * Depending on the implementation details, destroying a rule-set
>> - * may not guarantee the availability of hardware resources to create the
>> - * same or essentially similar rule-set.
>> - *
>> - * All of the resources pertaining to the match set associated with the
>> - * class-of-service will be released, but the class-of-service will
>> - * remain intact.
>> - *
>> - * @param[in]        pmr_set_id      A composite rule-set handle
>> - *                           returned when created.
>> - *
>> - * @retval                   0 on success
>> - * @retval                   <0 on failure
>> - */
>> -int odp_pmr_match_set_destroy(odp_pmr_set_t pmr_set_id);
>> -
>> -/**
>> - * Apply a PMR Match Set to a pktio to assign a CoS.
>> - *
>> - * @param[in]        pmr_set_id      PMR match set to be activated
>> - * @param[in]        src_pktio       pktio to which this PMR match
>> - *                           set is to be applied
>> - * @param[in]        dst_cos         CoS to be assigned by this PMR match
>> set
>> - *
>> - * @retval                   0 on success
>> - * @retval                   <0 on failure
>> - */
>> -int odp_pktio_pmr_match_set_cos(odp_pmr_set_t pmr_set_id, odp_pktio_t
>> src_pktio,
>> -                             odp_cos_t dst_cos);
>> -
>> -/**
>>  * Assigns a packet pool for a specific class of service.
>>  * All the packets belonging to the given class of service will
>>  * be allocated from the assigned packet pool.
>> @@ -456,19 +397,6 @@ uint64_t odp_cos_to_u64(odp_cos_t hdl);
>>  uint64_t odp_pmr_to_u64(odp_pmr_t hdl);
>>
>>  /**
>> - * Get printable value for an odp_pmr_set_t
>> - *
>> - * @param hdl  odp_pmr_set_t handle to be printed
>> - * @return     uint64_t value that can be used to print/display this
>> - *             handle
>> - *
>> - * @note This routine is intended to be used for diagnostic purposes
>> - * to enable applications to generate a printable value that represents
>> - * an odp_pmr_set_t handle.
>> - */
>> -uint64_t odp_pmr_set_to_u64(odp_pmr_set_t hdl);
>> -
>> -/**
>>   * @}
>>   */
>>
>> --
>> 1.9.1
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to