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 <[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] [RFC v6] regexdev: introduce regexdev subsystem > > Hi Pavan, > > > -----Original Message----- > > From: dev <[email protected]> On Behalf Of Pavan Nikhilesh > Bhagavatula > > Sent: Tuesday, March 10, 2020 6:37 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] [RFC v6] regexdev: introduce regexdev subsystem > > > > Hi Ori, > > > > > >Hi Pavan, > > > > > >> -----Original Message----- > > >> From: dev <[email protected]> On Behalf Of Pavan Nikhilesh > > >Bhagavatula > > >> Sent: Tuesday, March 10, 2020 3:42 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] [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
