Hi OrI,
>
>Hi Pavan,
>
>
>> -----Original Message-----
>> From: dev <dev-boun...@dpdk.org> On Behalf Of Pavan Nikhilesh
>Bhagavatula
>> Sent: Sunday, March 1, 2020 4:38 PM
>> 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,
>>
>> >
>> >Hi Pavan,
>> >
>> >> -----Original Message-----
>> >> From: dev <dev-boun...@dpdk.org> On Behalf Of Pavan
>Nikhilesh
>> >Bhagavatula
>> >> Sent: Sunday, March 1, 2020 3:23 PM
>> >> 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,
>> >>
>> >> >
>> >> >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.
>> >>
>> >Yes, you must have at least one group.
>>
>> Makes sense, I think we need to update the comment a bit as it only
>mentions
>> that
>> at least one group but it should be group_id0 has to be always valid.
>>
>> (An application can erroneously set valid group_id1 instead of
>group_id0)
>>
>
>What about the next comment?
>/**< First group_id to match the rule against. This group must be valid.
>In
> * order to support more group (up to 4 groups). The group number
>should
> * be set. For example to enable group 1 group_id1 should be set
> * with the group value and and the
>RTE_REGEX_OPS_REQ_GROUP_ID1_VALID_F flag should be set.
> * 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.
>*/
Looks good with minor corrections as below
/**< First group_id to match the rule against. This group must be valid.
* In order to support more than one group per each op (up to 4 groups), any
of the group_id<1-3> should
* hold a valid group id along with RTE_REGEX_OPS_REQ_GROUP_ID<1-3>_VALID_F
flag set.
* For example, to match against group 100 and 101, group_id0 should be set to
100 and group_id1 should
* be set to 101 and the RTE_REGEX_OPS_REQ_GROUP_ID1_VALID_F flag should be
set.
* 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.
*/
Thanks,
Pavan.
>
>/**< 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.
>
>> >
>> >> >
>> >> >> >+ /**< 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
>> >> >> >+ */
>> >> >> >+};
