Hi Ori,
>
>Hi Pavan,
>
>> -----Original Message-----
>> From: dev <[email protected]> On Behalf Of Pavan Nikhilesh
>Bhagavatula
>> Sent: Sunday, March 1, 2020 3:23 PM
>> To: Ori Kam <[email protected]>; Jerin Jacob Kollanukkaran
>> <[email protected]>; [email protected]
>> Cc: [email protected]; Shahaf Shuler <[email protected]>;
>> [email protected]; Opher Reviv <[email protected]>;
>Alex
>> Rosenbaum <[email protected]>; Dovrat Zifroni
><[email protected]>;
>> Prasun Kapoor <[email protected]>; [email protected];
>> [email protected]; [email protected];
>[email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>[email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; Thomas Monjalon
>> <[email protected]>
>> 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 <[email protected]> On Behalf Of Pavan
>Nikhilesh
>> >Bhagavatula
>> >> Sent: Sunday, March 1, 2020 8:13 AM
>> >> To: Ori Kam <[email protected]>; Jerin Jacob Kollanukkaran
>> >> <[email protected]>; [email protected]
>> >> Cc: [email protected]; Shahaf Shuler <[email protected]>;
>> >> [email protected]; Opher Reviv <[email protected]>;
>> >Alex
>> >> Rosenbaum <[email protected]>; Dovrat Zifroni
>> ><[email protected]>;
>> >> Prasun Kapoor <[email protected]>; [email protected];
>> >> [email protected]; [email protected];
>> >[email protected];
>> >> [email protected]; [email protected];
>> >> [email protected]; [email protected];
>> >[email protected];
>> >> [email protected]; [email protected];
>> >> [email protected]; [email protected];
>> >> [email protected]; [email protected]; [email protected];
>> >> [email protected]; [email protected]; [email protected];
>> >> [email protected]; [email protected]; Thomas Monjalon
>> >> <[email protected]>
>> >> 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)
>
>> >
>> >> >+ /**< 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
>> >> >+ */
>> >> >+};
