From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 889A8A0565; Mon, 2 Mar 2020 08:19:18 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3B8981C012; Mon, 2 Mar 2020 08:19:14 +0100 (CET) Received: from mail-il1-f194.google.com (mail-il1-f194.google.com [209.85.166.194]) by dpdk.org (Postfix) with ESMTP id 6875D1C012 for ; Mon, 2 Mar 2020 08:19:12 +0100 (CET) Received: by mail-il1-f194.google.com with SMTP id x2so8422034ila.9 for ; Sun, 01 Mar 2020 23:19:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=MOYcPprWb368gyGr8wGsyBHpk+3pFRbSiQ31IyUEuTw=; b=gUR+1+zLbQS497OXk/vh8hd5nz1XzkpC7nKiaSbEwVkvpBbKwJmN6iy6NKiKjc9K/e 1Ig3FFxG++d2Jh6ePo/RJ8e+c3MHnHQRdxfcf9TAX/kWWU4ni1rwEe2JDEzzxWatqCmG jiF+Zp9pb9yVw9FEyMCz+EBaozklIOrKlCQNJgLRj3SinZguR3mrla22H1ij9eTVJjsa HD6m8hIyVkhhnykV3LAEKWHkZvQdz+o/WH2rG7mxn9EX+nbLqqOO90Oz3RaXQpCeoMSC NKlqVRfIkOaGHwgY0/JtGXGvSUXdeG7O933K/OD1PNHtSm6MeHNMy2WcXpVFO4uZdOUL jeOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=MOYcPprWb368gyGr8wGsyBHpk+3pFRbSiQ31IyUEuTw=; b=RW+ePLppTxaf+cxW76vV0mGRznlUGpjrBOtBtckz3iM1fAuMBU8vZDJXwT4VeDmsuU 4dQqFT49XZDrcnL40yy3rC+DO6sjgbCsfPLsOUHKXvEJlXFZCfoXajb7HBqd69BESY7k I2tPR1LnMjdJD7N7l12shpDOhz8FyJnDAdmDbW2oMmUdso/fOdt5B/1esxMqtpIwhvAs 051qiDzMQC4raq5Jg4KzYtuRHEBlKvCV49NqnALQ5FG2zaApe5EkWPemyJ/cRc1vfR3L xj3Ui1zvwRHsn/PJhshFBM/3s3hOJ48WcAcVAVo4Xfx/x9tZLL/FVdwOZhfZKIm4V/60 bacA== X-Gm-Message-State: APjAAAWaBetdKyTRMwWofrJfHwAi+H/XGtxwt4a3AU8EadFeE6OnQWln Qx60fr3McEk2atRqpQIABJ8A47IWwXHOu8QGHgo= X-Google-Smtp-Source: APXvYqwFzVTMQ+agJJSbgfU4PoteOpqB1FFRxGa3W/gPJZVi/Wtuyi7mzZ8gWaaj9ry29ijJjBWzuPaRDvQrRg1a3eo= X-Received: by 2002:a05:6e02:f48:: with SMTP id y8mr9346101ilj.271.1583133551657; Sun, 01 Mar 2020 23:19:11 -0800 (PST) MIME-Version: 1.0 References: <20190627155036.56940-1-jerinj@marvell.com> <1582816115-95871-1-git-send-email-orika@mellanox.com> In-Reply-To: From: Jerin Jacob Date: Mon, 2 Mar 2020 12:48:55 +0530 Message-ID: To: Pavan Nikhilesh Bhagavatula Cc: Ori Kam , Jerin Jacob Kollanukkaran , "xiang.w.wang@intel.com" , "dev@dpdk.org" , Shahaf Shuler , "hemant.agrawal@nxp.com" , Opher Reviv , Alex Rosenbaum , Dovrat Zifroni , Prasun Kapoor , "nipun.gupta@nxp.com" , "bruce.richardson@intel.com" , "yang.a.hong@intel.com" , "harry.chang@intel.com" , "gu.jian1@zte.com.cn" , "shanjiangh@chinatelecom.cn" , "zhangy.yun@chinatelecom.cn" , "lixingfu@huachentel.com" , "wushuai@inspur.com" , "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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce regexdev subsystem X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Sun, Mar 1, 2020 at 9:28 PM Pavan Nikhilesh Bhagavatula wrote: > > Hi OrI, > > > > >Hi Pavan, > > > > > >> -----Original Message----- > >> From: dev On Behalf Of Pavan Nikhilesh > >Bhagavatula > >> Sent: Sunday, March 1, 2020 4:38 PM > >> To: Ori Kam ; Jerin Jacob Kollanukkaran > >> ; xiang.w.wang@intel.com > >> Cc: dev@dpdk.org; Shahaf Shuler ; > >> hemant.agrawal@nxp.com; Opher Reviv ; > >Alex > >> Rosenbaum ; Dovrat Zifroni > >; > >> Prasun Kapoor ; nipun.gupta@nxp.com; > >> bruce.richardson@intel.com; yang.a.hong@intel.com; > >harry.chang@intel.com; > >> gu.jian1@zte.com.cn; shanjiangh@chinatelecom.cn; > >> zhangy.yun@chinatelecom.cn; lixingfu@huachentel.com; > >wushuai@inspur.com; > >> 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 > >> > >> Subject: Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce regexdev > >subsystem > >> > >> Hi Ori, > >> > >> > > >> >Hi Pavan, > >> > > >> >> -----Original Message----- > >> >> From: dev On Behalf Of Pavan > >Nikhilesh > >> >Bhagavatula > >> >> Sent: Sunday, March 1, 2020 3:23 PM > >> >> To: Ori Kam ; Jerin Jacob Kollanukkaran > >> >> ; xiang.w.wang@intel.com > >> >> Cc: dev@dpdk.org; Shahaf Shuler ; > >> >> hemant.agrawal@nxp.com; Opher Reviv ; > >> >Alex > >> >> Rosenbaum ; Dovrat Zifroni > >> >; > >> >> Prasun Kapoor ; nipun.gupta@nxp.com; > >> >> bruce.richardson@intel.com; yang.a.hong@intel.com; > >> >harry.chang@intel.com; > >> >> gu.jian1@zte.com.cn; shanjiangh@chinatelecom.cn; > >> >> zhangy.yun@chinatelecom.cn; lixingfu@huachentel.com; > >> >wushuai@inspur.com; > >> >> 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 > >> >> > >> >> 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 On Behalf Of Pavan > >> >Nikhilesh > >> >> >Bhagavatula > >> >> >> Sent: Sunday, March 1, 2020 8:13 AM > >> >> >> To: Ori Kam ; Jerin Jacob Kollanukkaran > >> >> >> ; xiang.w.wang@intel.com > >> >> >> Cc: dev@dpdk.org; Shahaf Shuler ; > >> >> >> hemant.agrawal@nxp.com; Opher Reviv > >; > >> >> >Alex > >> >> >> Rosenbaum ; Dovrat Zifroni > >> >> >; > >> >> >> Prasun Kapoor ; > >nipun.gupta@nxp.com; > >> >> >> bruce.richardson@intel.com; yang.a.hong@intel.com; > >> >> >harry.chang@intel.com; > >> >> >> gu.jian1@zte.com.cn; shanjiangh@chinatelecom.cn; > >> >> >> zhangy.yun@chinatelecom.cn; lixingfu@huachentel.com; > >> >> >wushuai@inspur.com; > >> >> >> 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 > >> >> >> > >> >> >> Subject: Re: [dpdk-dev] [EXT] [RFC v5] regexdev: introduce > >> >regexdev > >> >> >subsystem > >> >> >> > >> >> >> Hi Ori, > >> >> >> > >> >> >> Minor comments below. > >> >> >> > >> >> >> > >> >> >> > >> >> >> >+/** > >> >> >> >+ * 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=E2=80=99t 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>_VALI= D_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 vali= d. 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? > > > > >/**< 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 > >> >> >> >+ */ > >> >> >> >+};