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 5FD78A0557; Sat, 22 Feb 2020 17:52:42 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3BEA71DBD; Sat, 22 Feb 2020 17:52:41 +0100 (CET) Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by dpdk.org (Postfix) with ESMTP id C4D63B62 for ; Sat, 22 Feb 2020 17:52:39 +0100 (CET) Received: by mail-io1-f65.google.com with SMTP id h8so5890996iob.2 for ; Sat, 22 Feb 2020 08:52:39 -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; bh=e80p93iJ1huqvQizPjiQUxf27b5luFrx8tkXGj8HaeY=; b=UTznioj09Xql7837HEgRSD+jP7WfwqUvy7OWJ6sNNEhmDp0rImANxCv6oszSm4hLWb 3IYkqkfBUqU08JkPUD2VcJs41E/4iHRN27JBj0ZY7L6GJgBLLWw2QX7qTK8YcrCRx1Xv l557Nyu0sOggv7LIspIuc0iynDA5pV35mbJEKA26Ck6kQW/swEu531xpibucUcywR+4E ne+7JhIgtPxldCK5zrEu2LweUB+UicvLZRpk4SO37D7OX/l3yLYNqEx8kpbAjUUGJ78v LjKW8i0RfLewrpTgt+3LFK5nHsh3/3kT2PcMWpw1hPydeGDjDi9YD91n5zhjTGW26utl pPlQ== 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; bh=e80p93iJ1huqvQizPjiQUxf27b5luFrx8tkXGj8HaeY=; b=YS49TJKwH6upmS9+mlMJIcJPjtmtz6l1oo6Ybofmy/18b309IaUuef8398sC9rWP1m GEjFfZXVSQ80XeSuk7UWOyK6gYPI6NNEDaSYvagITYWvX5MBVmR+Kh1R0IX9VwIzTb4n FiEomD4dmmofkSMWZ17di4+m3/vS8VJTk/eEZGu6qebcICTTtPMCpMNm8tOW7/fuw0t5 Ju1pe+YMVOGgX6ob8QTK5c279YNx7AVsr5ZyWHfNYFWluUkyRvp6k0SyLRPCXCNx7CZp fjZtCLX/rgaM6fddmqdz+RvZol0een7V375iSCW2e4inE66PCpw+PhhhZYjuHAqFx7CG rbrw== X-Gm-Message-State: APjAAAXfTwQZIAM0/K8kmGffW7Gkc70DLMKZkW/TNRMvBzmoV9chKGDy Ylx6Qc+sIEkOqd+YyVlhcOwl+N/cv8qehA14xIE= X-Google-Smtp-Source: APXvYqzlofheXVvq1dONJsTKsen6HAfwYNdG02X/OERtL7QBF8mR5lT7TRH7p/M+t98RoLePqvI638YP7vxdxrIm4E8= X-Received: by 2002:a6b:c742:: with SMTP id x63mr40212023iof.162.1582390358737; Sat, 22 Feb 2020 08:52:38 -0800 (PST) MIME-Version: 1.0 References: <20190627155036.56940-1-jerinj@marvell.com> <1580202029-37096-1-git-send-email-orika@mellanox.com> In-Reply-To: <1580202029-37096-1-git-send-email-orika@mellanox.com> From: Jerin Jacob Date: Sat, 22 Feb 2020 22:22:21 +0530 Message-ID: To: Ori Kam Cc: Jerin Jacob , xiang.w.wang@intel.com, dpdk-dev , Pavan Nikhilesh , Shahaf Shuler , Hemant Agrawal , opher@mellanox.com, alexr@mellanox.com, dovrat@marvell.com, Prasun Kapoor , Nipun Gupta , "Richardson, Bruce" , 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" Subject: Re: [dpdk-dev] [PATCH v3] 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" > diff --git a/lib/librte_regexdev/rte_regexdev.h b/lib/librte_regexdev/rte_regexdev.h > new file mode 100644 > index 0000000..c42128b > --- /dev/null > +++ b/lib/librte_regexdev/rte_regexdev.h > @@ -0,0 +1,1411 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(C) 2019 Marvell International Ltd. > + * Copyright(C) 2020 Mellanox International Ltd. There are a few comments from Xiang as well. So let's add Intel also to the list. > + */ > + > +#ifndef _RTE_REGEXDEV_H_ > +#define _RTE_REGEXDEV_H_ > + > +/** > + * RegEx device information > + */ > +struct rte_regex_dev_info { > + const char *driver_name; /**< RegEx driver name. */ > + struct rte_device *dev; /**< Device information. */ > + uint16_t max_matches; > + /**< Maximum matches per scan supported by this device. */ > + uint16_t max_queue_pairs; > + /**< Maximum queue pairs supported by this device. */ > + uint16_t max_payload_size; > + /**< Maximum payload size for a pattern match request or scan. > + * @see RTE_REGEX_DEV_CFG_CROSS_BUFFER_SCAN_F > + */ > + uint32_t max_rules_per_group; > + /**< Maximum rules supported per group by this device. > + * This number can't be larger then 20 bits. s/then/than I think, we don't need to say this " This number can't be larger than 20 bits." It may help SW drivers. > + */ > + uint16_t max_groups; > + /**< Maximum group supported by this device. > + * This number can't be larger then 12 bits. s/then/than I think, we don't need to say this " This number can't be larger than 12 bits." It may help SW drivers. > + */ > + uint32_t regex_dev_capa; > + /**< RegEx device capabilities. @see RTE_REGEX_DEV_CAPA_* */ > + uint64_t rule_flags; > + /**< Supported compiler rule flags. > + * @see RTE_REGEX_PCRE_RULE_*, struct rte_regex_rule::rule_flags > + */ > + uint8_t max_scatter_gather; > + /**< The max supported number of buffers that can > + * be used in a single ops. The total size of all elements > + * must be less then max_payload_size. > + */ > +}; > +int > +rte_regex_rule_db_compile(uint8_t dev_id); > + I think your "rte_regex_rule_db_compile_activate() - compile and activate the new rule set" API name looks good. I am for rte_regex_rule_db_compile_activate(). > +/* Fast path APIs */ > + > +/** > + * The generic *rte_regex_match* structure to hold the RegEx match attributes. > + * @see struct rte_regex_ops::matches > + */ > +struct rte_regex_match { > + RTE_STD_C11 > + union { > + uint64_t u64; > + struct { > + uint32_t rule_id:20; > + /**< Rule identifier to which the pattern matched. > + * @see struct rte_regex_rule::rule_id > + */ > + uint32_t group_id:12; > + /**< Group identifier of the rule which the pattern > + * matched. @see struct rte_regex_rule::group_id > + */ > + uint16_t offset; Since we have end_offset now, IMO, it is better to change this offset to "start_offset". > + /**< Starting Byte Position for matched rule. */ > + RTE_STD_C11 > + union { > + uint16_t len; > + /**< Length of match in bytes */ > + uint16_t end_offset; > + /**< The end offset of the match. In case > + * MATCH_AS_START configuration is disabled. > + * @see RTE_REGEX_DEV_CFG_MATCH_AS_START > + */ We have not concluded on this scheme. Have one field which has different meaning will be difficult for application. i.e fast path we need to have a check for this. I think, Based on the majority of HW/SW implementation, we need to either go with len or end_offset. What Mellanox HW returns? len or end_offset? or We can keep it as len or end_offset based on which drivers upstream first, other drivers when it comes, we can see how to abstract it? > + }; > + }; > + }; > +}; > +/** > + * 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 */ > + uint16_t num_of_bufs; > + /**< The number of bufs that are part of this ops. The total size of > + * the length of all the buffer must be smaller then the max buffer > + * len. > + */ > + uint16_t resv1; > + uint32_t resv2; One of the point came up in our implementation is that. HW can return an error due to various reasons. One option could be to make nb_matches as zero? and update some flag? What are your thoughts? updating the flag may be overkill. > + > + /* W2 */ > + struct rte_regex_iov *(*bufs)[]; > + /**< Holds a pointer to the buffers list.*/ This memory gets submitted to HW so it can not be from the heap. Cryptodev had a similar dilemma to use the container format for multi-segment case, Finally they choose to with mbuf. The following elements are in mbuf. Considering to avoid duplication and avoid overhead most common usecase DPI(Assume if it is rte_regex_iov, one need to copy all the elements from mbuf on fastpath). I propose to have mbuf here instead of rte_regex_iov. struct rte_regex_iov { RTE_STD_C11 union { uint64_t u64; /**< Allow 8-byte reserved on 32-bit system */ void *buf_addr; /**< Virtual address of the pattern to be matched. */ }; rte_iova_t buf_iova; /**< IOVA address of the pattern to be matched. */ uint16_t buf_size; /**< The buf size. */ }; > + > + /* W5 */ > + RTE_STD_C11 > + union { > + uint64_t cross_buf_id; > + /**< ID used by the RegEx device in order to handle cross > + * buffer detection. > + * This ID is given by the RegEx device on dequeue, and > + * the application must send it on the following enque. > + */ > + void *cross_buf_ptr; > + /**< Pointer representation of *cross_buf_id* */ Could you have some example of how to use cross_buf_id? Marvell HW does not support cross_buf_id, so we need to add this feature as capability. Regarding the rule attributes, We think, following needs to be added to control the rule compilation behavior. If it not converging in first look, I think, we can make below as separate patch once we have basic things. diff --git a/lib/librte_regexdev/rte_regexdev.h b/lib/librte_regexdev/rte_regexdev.h index 765da4aaa..1fa6a7135 100644 --- a/lib/librte_regexdev/rte_regexdev.h +++ b/lib/librte_regexdev/rte_regexdev.h @@ -767,6 +767,10 @@ enum rte_regex_rule_op { struct rte_regex_rule { enum rte_regex_rule_op op; /**< OP type of the rule either a OP_ADD or OP_DELETE */ + uint32_t nb_prefix; + /**< Number of prefix entries */ + char **prefixes; + /**< Rule prefix list */ uint16_t group_id; /**< Group identifier to which the rule belongs to. */ uint32_t rule_id; @@ -784,6 +788,169 @@ struct rte_regex_rule { */ }; +/** Enumerates RegEx rule prefix control type */ +enum rte_regex_prefix_control_entry_type +{ + RTE_REGEX_PREFIX_CONTROL_ENTRY_BLACKLIST = 0, + /**< The RegEx rule compiler will not use any prefixes that are + * specified. If there are no options but to use a blacklisted prefix, + * the rule will not be compiled. + */ + RTE_REGEX_PREFIX_CONTROL_ENTRY_GRAYLIST = 1, + /**< The RegEx rule compiler will try not to use any prefixes that + * are specified. + * It will settle for these if there are no other options. + */ + RTE_REGEX_PREFIX_CONTROL_ENTRY_WHITELIST = 2 + /**< The RegEx rule compiler will try to use any prefixes that are + * specified. If it can't it will use other prefixes. + */ + +}; + +/** Structure to hold a RegEx rule prefix control entry */ +struct rte_regex_prefix_control_entry +{ + enum rte_regex_prefix_control_entry_type type; + /**< Indicates the type of the prefix selection control list entry */ + char *value; + /**< The value associated with the prefix selection control list entry. + * This could be a string such as ABCD. + * Characters can also be represented in hex format + * such as \x00\x01\x02\x03. + */ +}; + +/** Structure to hold a RegEx rule rule prefix control entry list */ +struct rte_regex_prefix_control +{ + uint32_t num; + /**< The number of prefix selection control list entries */ + struct rte_regex_prefix_control_entry *prefixes; + /**< The list of prefix selection control list entries */ +}; + +/** Enumerates RegEx rule compilation capabilities */ + +#define RTE_REGEX_COMPILE_CAP_AUTO_RULE_ID_F (1ULL << 0) +/**< RegEx device compilation supports auto rule id + * Assign an automatic incrementing ID to each rule. + */ + +#define RTE_REGEX_COMPILE_CAP_CHECKSUM_F (1ULL << 1) +/**< RegEx device compilation supports checksum calculation. + * When performing an incremental compile use the end checksum of the base rule + * db as the start checksum for the new rule db. + * Must be used in conjunction with an incremental compile. + */ + +/* @warning might not be needed */ +#define RTE_REGEX_COMPILE_CAP_EM_F (1ULL << 2) +/**< RegEx device compilation supports external memory + * Store rule db in external memory only. + */ + +/* @warning might not be needed */ +#define RTE_REGEX_COMPILE_CAP_DISABLE_DISABLE_AIC_F (1ULL << 3) +/**< RegEx device compilation supports disable of atomic incremental compile + * Disable the atomic incremental compile system. + * The incremental compilation will be performed but the output + * will not be atomized. + */ + +#define RTE_REGEX_COMPILE_CAP_FORCE_F (1ULL << 4) +/**< RegEx device compilation supports force compiliation + * Force the compilation to complete even if errors are found. + */ + +#define RTE_REGEX_COMPILE_CAP_INCREMENTAL_F (1ULL << 5) +/**< RegEx device compilation supports incremental compilation + * Perform an incremental compile using pre build rule db as the base. + */ + +#define RTE_REGEX_COMPILE_CAP_NO_INC_COMPILE_PADDING_F (1ULL << 6) +/**< RegEx device compilation supports padding removal + * Remove the padding that is normally introduced to assist with + * incremental compile. This will allow for a more compact rule db footprint + * but is not recommended if you intend to perform an incremental + * compile with the resulting rule db. + */ + +#define RTE_REGEX_COMPILE_CAP_PCRE_PRE_8_36_F (1ULL << 7) +/**< RegEx device compilation supports change of space class + * Set the space class to not include vertical tab (VT) as was used in + * PCRE before v8.36. + */ + +#define RTE_REGEX_COMPILE_CAP_STRICT_QUANTIFIERS_F (1ULL << 8) +/**< RegEx device compilation supports strict quantifiers + * To help with performance, by default the REGEX Compiler treats non-fixed + * bounded quantifiers as unbounded e.g. .{0,2048} will be the same as .*. This + * has the caveat of false positives being possible. + * This capability will ensure that the original construct is used in all + * cases meaning performance will be worse but no false positives will occur. + */ + +#define RTE_REGEX_COMPILE_CAP_QUICK_REMOVE_RULES_F (1ULL << 9) +/**< RegEx device compilation supports quick removal of rules + * Quick incremental compile to remove rules. + * Specify rule_ids_to_remove from rule DB. + * Must be used in conjunction with the incremental option. + */ + +#define RTE_REGEX_COMPILE_CAP_QUICK_ADD_RULES_F (1ULL << 10) +/**< RegEx device compilation supports quick addition of rules + * Quick incremental compile to add rules. + * Specify rule_ids_to_add to rule DB. + * Must be used in conjunction with the incremental option. + */ + +/* @warning might not be needed */ +#define RTE_REGEX_COMPILE_CAP_UTF_8_F (1ULL << 11) +/**< RegEx device compilation supports UTF-8 mode + * Switch on UTF-8 mode. + */ + +#define RTE_REGEX_COMPILE_CAP_DISABLE_BIDIRECTIONAL_F (1ULL << 12) +/**< RegEx device compilation supports disabling of bidirectional + * This disables the bidirectional rule compilation. + */ + +#define RTE_REGEX_COMPILE_CAP_UTF_16_STRING_F (1ULL << 13) +/**< RegEx device compilation supports UTF-16 strings efficiency + * Deal with UTF-16 strings more efficiently. + * The Compiler will assume RegEx constructs like the following are intended + * to match UTF-16 strings:A\x00?B\x00?C\x00? + * with this switch enabled the Compiler will interpret the + * above RegEx as:ABCD|A\x00B\x00C\x00 + */ + +#define RTE_REGEX_COMPILE_CAP_SWITCH_OFF_LOCALE_SUPPORT_F (1ULL << 14) +/**< RegEx device compilation supports disabling locale + * Disable locale support. + */ + +#define RTE_REGEX_COMPILE_CAP_SPLIT_ALTERNATIONS_F (1ULL << 15) +/**< RegEx device compilation supports automatic alternation splitting + * Enable the automatic alternation splitting. + */ + +/** Structure to hold a RegEx control list data sample */ +struct rte_regex_data_sample +{ + float threshold; + /**< A % threshold used to blacklist strings in the sample data. + * This % value represents the % of bytes in the sample data that + * represent the start of a 1 - 4-byte string. + * If the string occurs at > the % threshold number of bytes it will + * then be blacklisted. + */ + size_t length; + /**< The number of bytes in the data sample to be analyzed */ + char *data; + /**< The data sample to be analyzed */ +}; + /** * Update the rule database of a RegEx device. * @@ -793,6 +960,12 @@ struct rte_regex_rule { * which contain the regex rules attributes to be updated in rule database. * @param nb_rules * The number of PCRE rules to update the rule database. + * @param prefixes + * The prefix selection control list. + * @param compiler_options + * The compiler options flags @see RTE_REGEX_COMPILE_CAP*. + * @param data_sample + * The data sample for auto control list generation. * * @return * The number of regex rules actually updated on the regex device's rule @@ -811,7 +984,10 @@ struct rte_regex_rule { */ uint16_t rte_regex_rule_db_update(uint8_t dev_id, const struct rte_regex_rule *rules, - uint16_t nb_rules); + uint16_t nb_rules, + struct rte_regex_prefix_control *prefix_control, + uint32_t compiler_options + struct rte_regex_data_sample *data_sample); /** * Import a prebuilt rule database from a buffer to a RegEx device.