Re: [dpdk-dev] [RFC v6] regexdev: introduce regexdev subsystem

[Ori Kam <orika@mellanox.com>]

Hi All,

If there are no more comments, I'm starting to implement the new class.

Thanks,
Ori

> -----Original Message-----
> From: Ori Kam
> Sent: Tuesday, March 10, 2020 7:00 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; xiang.w.wang@intel.com
> Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>;
> hemant.agrawal@nxp.com; Opher Reviv <opher@mellanox.com>; Alex
> Rosenbaum <Alexr@mellanox.com>; Dovrat Zifroni <dovrat@marvell.com>;
> Prasun Kapoor <pkapoor@marvell.com>; 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
> <thomas@monjalon.net>
> Subject: RE: [dpdk-dev] [RFC v6] regexdev: introduce regexdev subsystem
> 
> Hi Pavan,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Pavan Nikhilesh
> Bhagavatula
> > Sent: Tuesday, March 10, 2020 6:37 PM
> > To: Ori Kam <orika@mellanox.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; xiang.w.wang@intel.com
> > Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>;
> > hemant.agrawal@nxp.com; Opher Reviv <opher@mellanox.com>; Alex
> > Rosenbaum <alexr@mellanox.com>; Dovrat Zifroni <dovrat@marvell.com>;
> > Prasun Kapoor <pkapoor@marvell.com>; 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
> > <thomas@monjalon.net>
> > Subject: Re: [dpdk-dev] [RFC v6] regexdev: introduce regexdev subsystem
> >
> > Hi Ori,
> > >
> > >Hi Pavan,
> > >
> > >> -----Original Message-----
> > >> From: dev <dev-bounces@dpdk.org> On Behalf Of Pavan Nikhilesh
> > >Bhagavatula
> > >> Sent: Tuesday, March 10, 2020 3:42 PM
> > >> To: Ori Kam <orika@mellanox.com>; Jerin Jacob Kollanukkaran
> > >> <jerinj@marvell.com>; xiang.w.wang@intel.com
> > >> Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>;
> > >> hemant.agrawal@nxp.com; Opher Reviv <opher@mellanox.com>;
> > >Alex
> > >> Rosenbaum <alexr@mellanox.com>; Dovrat Zifroni
> > ><dovrat@marvell.com>;
> > >> Prasun Kapoor <pkapoor@marvell.com>; 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
> > >> <thomas@monjalon.net>
> > >> Subject: Re: [dpdk-dev] [RFC v6] regexdev: introduce regexdev
> > >subsystem
> > >>
> > >> Hi Ori,
> > >>
> > >> <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. */
> > >>
> > >> While implementing pcre2 SW driver I came across an oddity where
> > >having
> > >> mbuf alone
> > >> wouldn’t suffice, we need to have scan start offset and scan length as
> > >generally
> > >> we would skip the
> > >> L2/L3 header.
> > >>
> > >
> > >Yes you are correct, in most cases the application will need
> > >not the all mbuf or it will connect number of mbuf.
> > >This can be acchived by modifying the mbuf to point to the correct data
> > >start, and decrease the len.
> >
> > Wouldn’t that complicate Txing the packet later on after dequeue from regex
> if
> > the user decides to do so?.
> > Instead we can have two fields in rte_regex_ops for storing scan_start_offset
> > and
> > scan_size
> >
> The user will need to return the packet to the original state.  I agree that
> that it is a bit harder for the application (but not by much). But in any case the
> user knows
> the size he removed so when done he just need to return to the original value.
> on the other end it save the user the working with iov structs.
> 
> Regarding your idea about start_offset and scan_size. It is a nice idea,
> But I don't think it is worth it, since the start_offset is just what the user
> needs to keep in order to return the mbuf to original state.
> Also if the user wants to combine number of messages, he can't use this
> approach  since he will need to remove the header also from the second
> message and bind the two messages. So in any case the user must have some
> logic.
> 
> > >In one of the previous version we used buffer address and iov to solve
> > >this issue. But in order to keep the API the same as crypto we decided
> > >to go
> > >with mbuf.
> >
> > The general idea was to save cycles converting mbuf and chain of mbuf to iov
> > and back not
> > just to stay in line with crypto.
> >
> 
> I agree and this was also my main thinking but Jerin and other community
> members raised
> this approach.
> Each approach has advantages and disadvantages.
> If the user wants he can just give the all mbuf. Also since at least in some
> cases the regex will be done after crypto it make sense to use the same structs.
> There is also the advantage of sharing code between all the drivers.
> (net/crypto/regex)
> which can be done when using mbuf. (for example memory registration)
> 
> > >This API is experimental and based on the usage we might change it to
> > >iov.
> > >
> > >> >+
> > >> >+	/* W2 */
> > >> >+	uint16_t group_id0;
> > >> >+	/**< First group_id to match the rule against. At minimum one
> > >> >group
> > >> >+	 * should be valid. Behaviour is undefined non of the groups are
> > >> >valid.
> > >> >+	 *
> > >> >+	 * @see RTE_REGEX_OPS_REQ_GROUP_ID0_VALID_F
> > >> >+	 */
> > >> >+	uint16_t group_id1;
> > >> >+	/**< Second group_id to match the rule against.
> > >> >+	 *
> > >> >+	 * @see RTE_REGEX_OPS_REQ_GROUP_ID1_VALID_F
> > >> >+	 */
> > >> >+	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
> > >> >+	 */
> > >> >+};
> > >> >+
> > >>
> > >> Thanks,
> > >> Pavan.
> > >
> > >Thanks,
> > >Ori