Taras,
It is partly my fault, I did not realize the exact scope of the changes you
proposed,
and in some aspect I thought you may be proposing to get rid of the single-rule
support.
Here are some comments addressing your patch directly, hope this time I am
being helpful.
- Leo
>
> Combine odp_pmr_t and odp_pmr_set_t into one type.
> ---
> include/odp/api/classification.h | 58
> ++++------------------------------------
> 1 file changed, 5 insertions(+), 53 deletions(-)
>
> diff --git a/include/odp/api/classification.h
> b/include/odp/api/classification.h
> index 97c872e..a03128c 100644
> --- a/include/odp/api/classification.h
> +++ b/include/odp/api/classification.h
> @@ -185,7 +185,8 @@ 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
> + * Has one or more matching terms. Each term is up to 32 bit of ternary
> + * matching of one of the available header fields.
> */
>
I disagree. This type is intended to represent a *single" match rule that uses
a single 32-bit
ternary hardware resource.
As long as a rule incorporates a single set of value and mask, the meaning of a
plurality of terms
is unclear at best.
Also, some platforms which are capable of matching one header field at a time,
may not be able
to match multiple header fields against a single value/mask at the same time.
On platforms that are capable of matching a single value/mask with multiple
header fields,
it creates an ambiguity regarding the action to be taken of only one of the
terms of a term set
matches the value/mask.
> /**
> @@ -318,57 +319,20 @@ typedef struct odp_pmr_match_t {
> } odp_pmr_match_t;
>
> /**
> - * @typedef odp_pmr_set_t
> - * An opaque handle to a composite packet match rule-set
> - */
> -
In other words, a single-PMR and a PMR-set are two distinct abstractions,
and I disagree with the idea to combine them.
> -/**
> * 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.
> + * @param[out] pmr_set Returned handle to the composite PMR.
> *
> * @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
> - * 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);
> +int odp_pmr_set_create(int num_terms, odp_pmr_match_t *terms,
> + odp_pmr_t *pmr_set);
>
While I have confidence that several platforms should be capable supporting the
single-PMR
abstraction that I originally proposed, I have doubts that many platforms will
be able to implement
the PMR-set abstraction, and for this reason I prefer to keep them separate.
Of course if there is a platform that can support PMR-set, then it should be
able to implement the single-PMR
as well.
> /**
> * Get printable value for an odp_cos_t
> @@ -396,18 +360,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
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
