Hi Bala,

Thanks for your feedback!

> odp_pmr_match_t structure was already defined and this patch just adds 
> "offset" variable so why don't we just add the additional variable in 
> the existing structure and maybe move the odp_pmr_create() function 
> below this structure if that was the concern.
> It narrows down the changes done by this patch and will be easier to 
> understand the changes required for adding the term PMR_CUSTOM_FRAME.

I disagree, for 2 reasons:
  1) move odp_create would in my opinion complicate code reads  by human:
I usually expect the function to be grouped together, ie finding
odp_create just above odp_destroy
  2) I tried what you suggests and the diffstat is identical (see below)
So it seems to me that it does not reduce the patch size and in the
contrary the header will be harder to read.

Here's the new diff:
 include/odp/api/classification.h | 55 ++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/include/odp/api/classification.h b/include/odp/api/classification.h
index f597b26..b63a6c8 100644
--- a/include/odp/api/classification.h
+++ b/include/odp/api/classification.h
@@ -215,31 +215,17 @@ typedef enum odp_pmr_term {
        ODP_PMR_IPSEC_SPI,      /**< IPsec session identifier(*val=uint32_t)*/
        ODP_PMR_LD_VNI,         /**< NVGRE/VXLAN network identifier
                                (*val=uint32_t)*/
+       ODP_PMR_CUSTOM_FRAME,   /**< Custom match rule, offset from start of
+                               frame. The match is defined by the offset, the
+                               expected value, and its size. They must be
+                               applied before any other PMR.
+                               (*val=uint8_t[val_sz])*/
 
        /** Inner header may repeat above values with this offset */
        ODP_PMR_INNER_HDR_OFF = 32
 } 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(odp_pmr_term_e term, const void *val,
-                        const void *mask, uint32_t val_sz);
-
-/**
  * Invalidate a packet match rule and vacate its resources
  *
  * @param[in]  pmr_id  Identifier of the PMR to be destroyed
@@ -290,27 +276,40 @@ unsigned long long odp_pmr_terms_cap(void);
 unsigned odp_pmr_terms_avail(void);
 
 /**
- * Following structure is used to define composite packet matching rules
- * 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.
+ * Following structure is used to define a packet matching rule
  */
 typedef struct odp_pmr_match_t {
        odp_pmr_term_e  term;   /**< PMR term value to be matched */
        const void      *val;   /**< Value to be matched */
        const void      *mask;  /**< Masked set of bits to be matched */
-       unsigned int    val_sz;  /**< Size of the term value */
+       uint32_t        val_sz;  /**< Size of the term value */
+       uint32_t        offset;  /**< User-defined offset in packet
+                                Used if term == ODP_PMR_CUSTOM_FRAME only,
+                                otherwise must be 0 */
 } odp_pmr_match_t;
 
 /**
+ * Create a packet match rule with mask and value
+ *
+ * @param[in]  match   packet matching rule definition
+ *
+ * @return             Handle of the matching rule
+ * @retval             ODP_PMR_INVAL on failure
+ */
+odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
+
+/**
  * @typedef odp_pmr_set_t
  * An opaque handle to a composite packet match rule-set
  */
 
 /**
- * Create a composite packet match rule
+ * 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
@@ -322,7 +321,7 @@ typedef struct odp_pmr_match_t {
  *                             underlying platform classification engine
  * @retval                     <0 on failure
  */
-int odp_pmr_match_set_create(int num_terms, odp_pmr_match_t *terms,
+int odp_pmr_match_set_create(int num_terms, const odp_pmr_match_t *terms,
                             odp_pmr_set_t *pmr_set_id);
 
 /**

ben
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to