On 9 April 2015 at 14:38, Taras Kondratiuk <taras.kondrat...@linaro.org> wrote:
> + mailing list that I've missed initially in the RFC.
>
> On 04/08/2015 10:25 PM, Rosenboim, Leonid wrote:
>>
>> Taras,
>>
>> I actually agree with you that a change in the API is justified
>> that combines the in and out CoS values are provided at the time
>> of PMR creation, instead of using a separate function.
>> The main reason is that the input CoS is one of the rules that
>> the ternary CAM must match, and the output CoS is the action
>> that it must be assigned.
>>
>> Here the key changes I propose, granted that there will
>> need to be additional changes made in other APIs in a similar manner.
>
>
> Yes this is an alternative approach.
>
> odp_pktio_pmr_cos() should be renamed to something like
> odp_pktio_cos_set()
> Is there still an implicit 'default' CoS assigned to Pktio on
> odp_pktio_open()? Or this function should be used to set a default one?
IMO, it might be better to have assigning of default CoS as a separate
function rather than on odp_pktio_open() since it gives freedom for
platforms which do not support classifier and also it treats
classifier as a separate module
Regards,
Bala
>
> This approach highlights one more implementation issue on my platform:
> CoS (pool and queue) assignment is the last step of classification, so
> CoS'es cannot be cascaded directly. Instead PMRs (LUT entries) are
> cascaded. CoS cascading can be mapped to PMRs cascading in simple
> use-cases, but if a first CoS has a several PMRs pointing to it, then
> next cascaded CoS will lead to creation of multiple LUT entries.
> I'll think how it can mapped in a better way.
>
>
>>
>> diff --git a/include/odp/api/classification.h
>> b/include/odp/api/classification.h
>> index 7db3645..b511cb4 100644
>> --- a/include/odp/api/classification.h
>> +++ b/include/odp/api/classification.h
>> @@ -232,6 +232,9 @@ typedef enum odp_pmr_term {
>> * @param[in] val_sz Size of the val and mask arguments,
>> * that must match the value size requirement of the
>> * specific term.
>> + * @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.
>> *
>> * @return Handle of the matching rule
>> * @retval ODP_PMR_INVAL on failure
>> @@ -239,7 +242,9 @@ typedef enum odp_pmr_term {
>> odp_pmr_t odp_pmr_create_match(odp_pmr_term_e term,
>> const void *val,
>> const void *mask,
>> - uint32_t val_sz);
>> + uint32_t val_sz,
>> + odp_cos_t src_cos,
>> + odp_cos_t dst_cos);
>>
>> /**
>> * Create a packet match rule with value range
>> @@ -250,6 +255,9 @@ odp_pmr_t odp_pmr_create_match(odp_pmr_term_e term,
>> * @param[in] val_sz Size of the val1 and val2 arguments,
>> * that must match the value size requirement of the
>> * specific term.
>> + * @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.
>> *
>> * @return Handle of the matching rule
>> * @retval ODP_PMR_INVAL on failure
>> @@ -258,7 +266,10 @@ odp_pmr_t odp_pmr_create_match(odp_pmr_term_e term,
>> odp_pmr_t odp_pmr_create_range(odp_pmr_term_e term,
>> const void *val1,
>> const void *val2,
>> - uint32_t val_sz);
>> + uint32_t val_sz,
>> + odp_cos_t src_cos,
>> + odp_cos_t dst_cos);
>> +
>> /**
>> * Invalidate a packet match rule and vacate its resources
>> *
>> @@ -270,30 +281,20 @@ odp_pmr_t odp_pmr_create_range(odp_pmr_term_e term,
>> int odp_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
>> + * Apply a initial CoS to a pktio.
>> *
>> - * @retval 0 on success
>> - * @retval <0 on failure
>> - */
>> -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.
>> + * This is the initial CoS that is assigned to packets arriving
>> + * from the 'pktio' channel, that may later be changed by any
>> + * Packet matching rules (PMRs) that hae the 'pktio_cos' assigned
>> + * as its 'src_cos'.
>> *
>> - * @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.
>> + * @param[in] src_pktio pktio to which this CoS is to be applied
>> + * @param[in] pktio_cos CoS to be assigned by this pktio
>> *
>> * @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_pktio_pmr_cos( odp_pktio_t src_pktio, odp_cos_t pktio_cos);
>>
>> /**
>> * Inquire about matching terms supported by the classifier
>>
>>
>> ________________________________________
>> From: Taras Kondratiuk <taras.kondrat...@linaro.org>
>> Sent: Wednesday, April 8, 2015 3:47 AM
>> To: Rosenboim, Leonid; bala.manoha...@linaro.org
>> Cc: bill.fischo...@linaro.org; Taras Kondratiuk
>> Subject: [RFC] api: classification: Create PMR handle on its application
>>
>> Current odp_pmr_t usage model has several issues:
>> 1. The same PMR can be applied into different places in a classification
>> tree, but there is no way to modify each of its applications
>> separately. One can only destroy a rule completely which should
>> destroy all of its instances in a classification tree.
>> 2. Initial intention behind odp_pmr_t was to abstarct a handle to some HW
>> resource which imlements a PMR. But on platforms I'm aware of a
>> separate
>> HW resource should be allocated for each PRM application. So there is
>> no 1:1 mapping between odp_pmr_t and HW resource.
>> If odp_pmr_t doesn't map to HW resource directly, then it just
>> represent a PRM description. It is an entry in some PRM descriptions
>> 'database' which impelemntation have to maintain for no good reason.
>>
>> Creating odp_pmr_t handle as a result of a rule application (to pktio
>> or CoS) solves these issues.
>>
>> Same is valid for odp_pmr_set_t. I still think that a separate
>> abstration for PMR sets is not needed, but it is preserved in the patch
>> to keep discussion focused.
>>
>> Function order should be changed to be more logical, but I've kept an
>> initial order to minimize a size of diff for RFC.
>> ---
>> include/odp/api/classification.h | 137
>> +++++++++++++++------------------------
>> 1 file changed, 52 insertions(+), 85 deletions(-)
>>
>> diff --git a/include/odp/api/classification.h
>> b/include/odp/api/classification.h
>> index 7db3645..04f9e60 100644
>> --- a/include/odp/api/classification.h
>> +++ b/include/odp/api/classification.h
>> @@ -184,8 +184,7 @@ int odp_cos_with_l3_qos(odp_pktio_t pktio_in,
>>
>> /**
>> * @typedef odp_pmr_t
>> - * PMR - Packet Matching Rule
>> - * Up to 32 bit of ternary matching of one of the available header fields
>> + * Applied Packet Matching Rule (PMR) handle
>> */
>>
>> /**
>> @@ -221,45 +220,6 @@ typedef enum odp_pmr_term {
>> } odp_pmr_term_e;
>>
>> /**
>> - * Create a packet match rule with mask and value
>> - *
>> - * @param[in] term One of the enumerated values supported
>> - * @param[in] val Value to match against the packet header
>> - * in native byte order.
>> - * @param[in] mask Mask to indicate which bits of the header
>> - * should be matched ('1') and
>> - * which should be ignored ('0')
>> - * @param[in] val_sz Size of the val and mask arguments,
>> - * that must match the value size requirement of the
>> - * specific term.
>> - *
>> - * @return Handle of the matching rule
>> - * @retval ODP_PMR_INVAL on failure
>> - */
>> -odp_pmr_t odp_pmr_create_match(odp_pmr_term_e term,
>> - const void *val,
>> - const void *mask,
>> - uint32_t val_sz);
>> -
>> -/**
>> - * Create a packet match rule with value range
>> - *
>> - * @param[in] term One of the enumerated values supported
>> - * @param[in] val1 Lower bound of the header field range.
>> - * @param[in] val2 Upper bound of the header field range.
>> - * @param[in] val_sz Size of the val1 and val2 arguments,
>> - * that must match the value size requirement of the
>> - * specific term.
>> - *
>> - * @return Handle of the matching rule
>> - * @retval ODP_PMR_INVAL on failure
>> - * @note: Range is inclusive [val1..val2].
>> - */
>> -odp_pmr_t odp_pmr_create_range(odp_pmr_term_e term,
>> - const void *val1,
>> - const void *val2,
>> - uint32_t val_sz);
>> -/**
>> * Invalidate a packet match rule and vacate its resources
>> *
>> * @param[in] pmr_id Identifier of the PMR to be destroyed
>> @@ -272,28 +232,29 @@ int odp_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] match Pointer to PMR description structure
>> * @param[in] src_pktio pktio to which this PMR is to be applied
>> * @param[in] dst_cos CoS to be assigned by this PMR
>> *
>> - * @retval 0 on success
>> - * @retval <0 on failure
>> + * @return Handle of an applied PMR
>> + * @retval ODP_PMR_INVALID on failure
>> */
>> -int odp_pktio_pmr_cos(odp_pmr_t pmr_id,
>> - odp_pktio_t src_pktio, odp_cos_t dst_cos);
>> +odp_pmr_t odp_pktio_pmr_cos(odp_pmr_match_t *match,
>> + 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] match Pointer to PMR description structure
>> * @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
>> + * @return Handle of an applied PMR
>> + * @retval ODP_PMR_INVALID on failure
>> */
>> -int odp_cos_pmr_cos(odp_pmr_t pmr_id, odp_cos_t src_cos, odp_cos_t
>> dst_cos);
>> +odp_pmr_t odp_cos_pmr_cos(odp_pmr_match_t *match,
>> + odp_cos_t src_cos, odp_cos_t dst_cos);
>>
>> /**
>> * Inquire about matching terms supported by the classifier
>> @@ -320,27 +281,44 @@ typedef enum odp_pmr_match_type {
>> } odp_pmr_match_type_e;
>>
>> /**
>> - * Following structure is used to define composite packet matching rules
>> - * in the form of an array of individual match or range rules.
>> - * The underlying platform may not support all or any specific
>> combination
>> - * of value match or range rules, and the application should take care
>> - * of inspecting the return value when installing such rules, and perform
>> - * appropriate fallback action.
>> + * Packet Matching Rule (PMR) description structure
>> + *
>> + * Describes ternary matching of one of the available header fields
>> */
>> -typedef struct odp_pmr_match_t {
>> +typedef struct odp_pmr_match_s {
>> odp_pmr_match_type_e match_type; /**< Packet Match Type*/
>> union {
>> struct {
>> - odp_pmr_term_e term;
>> - const void *val;
>> - const void *mask;
>> - unsigned int val_sz;
>> + /** One of the enumerated values supported */
>> + odp_pmr_term_e term;
>> + /**
>> + * Value to match against the packet header
>> + * in native byte order
>> + */
>> + const void *val;
>> + /**
>> + * Mask to indicate which bits of the header
>> should be
>> + * matched ('1') and which should be ignored ('0')
>> + */
>> + const void *mask;
>> + /**
>> + * Size of the val and mask arguments, that must
>> match
>> + * the value size requirement of the specific term
>> + */
>> + unsigned int val_sz;
>> } mask; /**< Match a masked set of bits */
>> struct {
>> - odp_pmr_term_e term;
>> - const void *val1;
>> - const void *val2;
>> - unsigned int val_sz;
>> + /** One of the enumerated values supported */
>> + odp_pmr_term_e term;
>> + /** Lower bound of the header field range */
>> + const void *val1;
>> + /** Upper bound of the header field range */
>> + const void *val2;
>> + /**
>> + * Size of the val1 and val2 arguments, that must
>> match
>> + * the value size requirement of the specific term
>> + */
>> + unsigned int val_sz;
>> } range; /**< Match an integer range */
>> };
>> } odp_pmr_match_t;
>> @@ -351,22 +329,6 @@ typedef struct odp_pmr_match_t {
>> */
>>
>> /**
>> - * Create a composite packet match rule
>> - *
>> - * @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, 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
>> @@ -387,16 +349,21 @@ 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] num_terms Number of rules in a set
>> + * @param[in] terms Array of PMR description structures
>> * @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
>> + * @param[out] pmr_set Pointer to a handle of an applied PMR set
>> *
>> - * @retval 0 on success
>> + * @return the number of terms elements
>> + * that have been successfully mapped to the
>> + * underlying platform classification engine
>> * @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);
>> +int odp_pktio_pmrset_cos(uint32_t num_terms, odp_pmr_match_t *terms,
>> + odp_pktio_t src_pktio, odp_cos_t dst_cos,
>> + odp_pmr_set_t *pmr_set);
>>
>> /**
>> * Get printable value for an odp_cos_t
>> --
>> 1.9.1
>>
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
