Hi Ori,
>
>Hi Pavan,
>Thanks for the comments please see below.
>
>> -----Original Message-----
>> From: dev <dev-boun...@dpdk.org> On Behalf Of Pavan Nikhilesh
>Bhagavatula
>> Sent: Sunday, March 1, 2020 8:13 AM
>> To: Ori Kam <or...@mellanox.com>; Jerin Jacob Kollanukkaran
>> <jer...@marvell.com>; xiang.w.w...@intel.com
>> Cc: dev@dpdk.org; Shahaf Shuler <shah...@mellanox.com>;
>> hemant.agra...@nxp.com; Opher Reviv <op...@mellanox.com>;
>Alex
>> Rosenbaum <al...@mellanox.com>; Dovrat Zifroni
><dov...@marvell.com>;
>> Prasun Kapoor <pkap...@marvell.com>; nipun.gu...@nxp.com;
>> bruce.richard...@intel.com; yang.a.h...@intel.com;
>harry.ch...@intel.com;
>> gu.ji...@zte.com.cn; shanjia...@chinatelecom.cn;
>> zhangy....@chinatelecom.cn; lixin...@huachentel.com;
>wush...@inspur.com;
>> yuying...@yxlink.com; fanchengg...@sunyainfo.com;
>> davidf...@tencent.com; liuzho...@chinaunicom.cn;
>> zhaoyon...@huawei.com; o...@yunify.com; j...@netgate.com;
>> hongjun...@intel.com; j.bromh...@titan-ic.com; d...@ntop.org;
>> f...@napatech.com; arthur...@lionic.com; Thomas Monjalon
>> <tho...@monjalon.net>
>> Subject: Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce regexdev
>subsystem
>>
>> Hi Ori,
>>
>> Minor comments below.
>>
>> <snip>
>>
>> >+/**
>> >+ * The generic *rte_regex_ops* structure to hold the RegEx
>attributes
>> >+ * for enqueue and dequeue operation.
>> >+ */
>> >+struct rte_regex_ops {
>> >+ /* W0 */
>> >+ uint16_t req_flags;
>> >+ /**< Request flags for the RegEx ops.
>> >+ * @see RTE_REGEX_OPS_REQ_*
>> >+ */
>> >+ uint16_t rsp_flags;
>> >+ /**< Response flags for the RegEx ops.
>> >+ * @see RTE_REGEX_OPS_RSP_*
>> >+ */
>> >+ uint16_t nb_actual_matches;
>> >+ /**< The total number of actual matches detected by the
>> >Regex device.*/
>> >+ uint16_t nb_matches;
>> >+ /**< The total number of matches returned by the RegEx
>> >device for this
>> >+ * scan. The size of *rte_regex_ops::matches* zero length array
>> >will be
>> >+ * this value.
>> >+ *
>> >+ * @see struct rte_regex_ops::matches, struct
>> >rte_regex_match
>> >+ */
>> >+
>> >+ /* W1 */
>> >+ struct rte_mbuf mbuf; /**< source mbuf, to search in. */
>>
>> This should be *mbuf.
>
>Yes you are correct will fix.
>
>>
>> >+
>> >+ /* W2 */
>> >+ uint16_t group_id0;
>>
>> This should be group_id1.
>>
>No this is correct is should be id0. We are starting from group 0.
>The comment below states that the first group, meaning group 0 must
>be
>valid group while group 1 doesn’t have to be vaild.
Would that mean that group_id0 is always valid?
Since there is no `RTE_REGEX_OPS_REQ_GROUP_ID0_VALID_F` flag.
>
>> >+ /**< First group_id to match the rule against. Minimum one
>> >group id
>> >+ * must be provided by application.
>> >+ * When RTE_REGEX_OPS_REQ_GROUP_ID1_VALID_F set then
>> >group_id1
>> >+ * is valid, respectively similar flags for group_id2 and group_id3.
>> >+ * Upon the match, struct rte_regex_match::group_id shall be
>> >updated
>> >+ * with matching group ID by the device. Group ID scheme
>> >provides
>> >+ * rule isolation and effective pattern matching.
>> >+ */
>> >+ uint16_t group_id1;
>> >+ /**< Second group_id to match the rule against.
>> >+ *
>> >+ * @see RTE_REGEX_OPS_REQ_GROUP_ID1_VALID_F
>> >+ */
>>
>> The above `group_id1` should be removed as its duplicate.
>>
>
>This is not duplicate, see above comment.
>
>> >+ uint16_t group_id2;
>> >+ /**< Third group_id to match the rule against.
>> >+ *
>> >+ * @see RTE_REGEX_OPS_REQ_GROUP_ID2_VALID_F
>> >+ */
>> >+ uint16_t group_id3;
>> >+ /**< Forth group_id to match the rule against.
>> >+ *
>> >+ * @see RTE_REGEX_OPS_REQ_GROUP_ID3_VALID_F
>> >+ */
>> >+
>> >+ /* W3 */
>> >+ RTE_STD_C11
>> >+ union {
>> >+ uint64_t user_id;
>> >+ /**< Application specific opaque value. An application
>> >may use
>> >+ * this field to hold application specific value to share
>> >+ * between dequeue and enqueue operation.
>> >+ * Implementation should not modify this field.
>> >+ */
>> >+ void *user_ptr;
>> >+ /**< Pointer representation of *user_id* */
>> >+ };
>> >+
>> >+ /* W4 */
>> >+ struct rte_regex_match matches[];
>> >+ /**< Zero length array to hold the match tuples.
>> >+ * The struct rte_regex_ops::nb_matches value holds the
>> >number of
>> >+ * elements in this array.
>> >+ *
>> >+ * @see struct rte_regex_ops::nb_matches
>> >+ */
>> >+};
