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.
> >
> >> >+ /**< 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
> >> >+ */
> >> >+};
