Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce regexdev subsystem

From: Jerin Jacob <hidden>
Date: 2020-03-02 07:19:19

On Sun, Mar 1, 2020 at 9:28 PM Pavan Nikhilesh Bhagavatula
[off-list ref] wrote:

Hi OrI,

quoted

Hi Pavan,

quoted

-----Original Message-----
From: dev <redacted> On Behalf Of Pavan Nikhilesh

Bhagavatula

quoted

Sent: Sunday, March 1, 2020 4:38 PM
To: Ori Kam <redacted>; Jerin Jacob Kollanukkaran
[off-list ref]; xiang.w.wang@intel.com
Cc: dev@dpdk.org; Shahaf Shuler <redacted>;
hemant.agrawal@nxp.com; Opher Reviv [off-list ref];

Alex

quoted

Rosenbaum [off-list ref]; Dovrat Zifroni

[off-list ref];

quoted

Prasun Kapoor [off-list ref]; nipun.gupta@nxp.com;
bruce.richardson@intel.com; yang.a.hong@intel.com;

harry.chang@intel.com;

quoted

gu.jian1@zte.com.cn; shanjiangh@chinatelecom.cn;
zhangy.yun@chinatelecom.cn; lixingfu@huachentel.com;

wushuai@inspur.com;

quoted

yuyingxia@yxlink.com; fanchenggang@sunyainfo.com;
davidfgao@tencent.com; liuzhong1@chinaunicom.cn;
zhaoyong11@huawei.com; oc@yunify.com; jim@netgate.com;
hongjun.ni@intel.com; j.bromhead@titan-ic.com; deri@ntop.org;
fc@napatech.com; arthur.su@lionic.com; Thomas Monjalon
[off-list ref]
Subject: Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce regexdev

subsystem

quoted

Hi Ori,

quoted

Hi Pavan,

quoted

-----Original Message-----
From: dev <redacted> On Behalf Of Pavan

Nikhilesh

quoted

Bhagavatula

quoted

Sent: Sunday, March 1, 2020 3:23 PM
To: Ori Kam <redacted>; Jerin Jacob Kollanukkaran
[off-list ref]; xiang.w.wang@intel.com
Cc: dev@dpdk.org; Shahaf Shuler <redacted>;
hemant.agrawal@nxp.com; Opher Reviv [off-list ref];

Alex

quoted

Rosenbaum [off-list ref]; Dovrat Zifroni

[off-list ref];

quoted

Prasun Kapoor [off-list ref]; nipun.gupta@nxp.com;
bruce.richardson@intel.com; yang.a.hong@intel.com;

harry.chang@intel.com;

quoted

gu.jian1@zte.com.cn; shanjiangh@chinatelecom.cn;
zhangy.yun@chinatelecom.cn; lixingfu@huachentel.com;

wushuai@inspur.com;

quoted

yuyingxia@yxlink.com; fanchenggang@sunyainfo.com;
davidfgao@tencent.com; liuzhong1@chinaunicom.cn;
zhaoyong11@huawei.com; oc@yunify.com; jim@netgate.com;
hongjun.ni@intel.com; j.bromhead@titan-ic.com; deri@ntop.org;
fc@napatech.com; arthur.su@lionic.com; Thomas Monjalon
[off-list ref]
Subject: Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce

regexdev

quoted

subsystem

quoted

Hi Ori,

quoted

Hi Pavan,
Thanks for the comments please see below.

quoted

-----Original Message-----
From: dev <redacted> On Behalf Of Pavan

Nikhilesh

quoted

Bhagavatula

quoted

Sent: Sunday, March 1, 2020 8:13 AM
To: Ori Kam <redacted>; Jerin Jacob Kollanukkaran
[off-list ref]; xiang.w.wang@intel.com
Cc: dev@dpdk.org; Shahaf Shuler <redacted>;
hemant.agrawal@nxp.com; Opher Reviv

[off-list ref];

quoted

Alex

quoted

Rosenbaum [off-list ref]; Dovrat Zifroni

[off-list ref];

quoted

Prasun Kapoor [off-list ref];

nipun.gupta@nxp.com;

quoted

bruce.richardson@intel.com; yang.a.hong@intel.com;

harry.chang@intel.com;

quoted

gu.jian1@zte.com.cn; shanjiangh@chinatelecom.cn;
zhangy.yun@chinatelecom.cn; lixingfu@huachentel.com;

wushuai@inspur.com;

quoted

yuyingxia@yxlink.com; fanchenggang@sunyainfo.com;
davidfgao@tencent.com; liuzhong1@chinaunicom.cn;
zhaoyong11@huawei.com; oc@yunify.com; jim@netgate.com;
hongjun.ni@intel.com; j.bromhead@titan-ic.com;

deri@ntop.org;

quoted

fc@napatech.com; arthur.su@lionic.com; Thomas Monjalon
[off-list ref]
Subject: Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce

regexdev

quoted

subsystem

quoted

Hi Ori,

Minor comments below.

<snip>

quoted

+/**
+ * The generic *rte_regex_ops* structure to hold the RegEx

attributes

quoted

+ * 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

quoted

Regex device.*/
+     uint16_t nb_matches;
+     /**< The total number of matches returned by the

RegEx

quoted

device for this
+      * scan. The size of *rte_regex_ops::matches* zero

length

quoted

array

quoted

will be
+      * this value.
+      *
+      * @see struct rte_regex_ops::matches, struct
rte_regex_match
+      */
+
+     /* W1 */
+     struct rte_mbuf mbuf; /**< source mbuf, to search in.

*/

quoted

This should be *mbuf.

Yes you are correct will fix.

quoted

+
+     /* 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

quoted

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.

quoted

Yes, you must have at least one group.

Makes sense, I think we need to update the comment a bit as it only

mentions

quoted

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)

quoted

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.
*/

I think, we can remove the limitation of group0 is always valid.
There are use cases like each group belongs certain functionality and
based on the packet type or
so application decides the group. In that case, group 0 may or may not valid.

IMO, By spec, we can dictate,

At minimum of one of the group should be valid and selected, Behaviour
is undefined if any of the group is not selected(This is to avoid fast
path check).

Thoughts?

quoted

/**< 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.

quoted

quoted
quoted
quoted
quoted
quoted
+     /**< First group_id to match the rule against. Minimum

one

quoted

group id
+      * must be provided by application.
+      * When RTE_REGEX_OPS_REQ_GROUP_ID1_VALID_F

set then

quoted

group_id1
+      * is valid, respectively similar flags for group_id2 and

group_id3.

quoted

+      * Upon the match, struct rte_regex_match::group_id

shall be

quoted

quoted
quoted
quoted
quoted
quoted
updated
+      * with matching group ID by the device. Group ID

scheme

quoted

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.

quoted

+     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

quoted

may use
+              * this field to hold application specific value to

share

quoted

+              * 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

quoted

number of
+      * elements in this array.
+      *
+      * @see struct rte_regex_ops::nb_matches
+      */
+};

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help