From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 722E7A00C2; Wed, 19 Jan 2022 16:16:05 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0B7154115D; Wed, 19 Jan 2022 16:16:05 +0100 (CET) Received: from sheloe.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 26B4C41147 for ; Wed, 19 Jan 2022 16:16:04 +0100 (CET) Received: by sheloe.oktetlabs.ru (Postfix, from userid 115) id 9AD8442; Wed, 19 Jan 2022 18:16:03 +0300 (MSK) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mail1.oktetlabs.ru X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=ALL_TRUSTED, DKIM_ADSP_DISCARD autolearn=no autolearn_force=no version=3.4.6 Received: from bree.oktetlabs.ru (bree.oktetlabs.ru [192.168.34.5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by sheloe.oktetlabs.ru (Postfix) with ESMTPS id 2ACF738 for ; Wed, 19 Jan 2022 18:16:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 sheloe.oktetlabs.ru 2ACF738 Authentication-Results: sheloe.oktetlabs.ru/2ACF738; dkim=none; dkim-atps=neutral Date: Wed, 19 Jan 2022 18:16:02 +0300 (MSK) From: Ivan Malov To: dev@dpdk.org Subject: Re: [PATCH v2 02/10] ethdev: add flow item/action templates Message-ID: MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset=US-ASCII X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi, > +Oftentimes in an application, many flow rules share a common structure > +(the same pattern and/or action list) so they can be grouped and classified > +together. This knowledge may be used as a source of optimization by a PMD/HW. > +The flow rule creation is done by selecting a table, an item template > +and an action template (which are bound to the table), and setting unique > +values for the items and actions. This API is not thread-safe. Consider: +Typically, flow rules generated by a given application conform to a small +group of "shapes". What defines a "shape" is a set of specific item masks +and action types. This knowledge facilitates optimisations in PMDs / HW. + +With such "shapes" (templates) being grouped in tables, a flow rule can +be created by selecting a template (pattern, action list) within a given +table and filling out specific match / action properties. > + struct rte_flow_item_template * > + rte_flow_item_template_create(uint16_t port_id, > + const struct rte_flow_item_template_attr *it_attr, > + const struct rte_flow_item items[], > + struct rte_flow_error *error); I'm afraid "it_attr" is hardly readable. Also, the API name can trick users into thinking that it's all about creating a single item template rather than a flow pattern template. Perhaps rename to "rte_flow_pattern_template_create()"? Use "tmpl" instead of "template"? Or "shape" maybe? For sure, "const struct rte_flow_item items[]" would look better when renamed to "const struct rte_flow_item pattern[]". The same goes for "rte_flow_action_template_create()" and "at_attr". Perhaps, "rte_flow_action_list_shape_create()" then? > +A table combines a number of item and action templates along with shared flow > +rule attributes (group ID, priority and traffic direction). This way a PMD/HW Please consider: +A template table consists of multiple pattern templates and action list +templates associated with a single set of rule attributes (group ID, +priority, etc). Perhaps rename "item_templates[]" and "action_templates[]" to "pattern_templates[]" and "action_list_templates[]". Maybe make use of the term "shape" here as well... > + /** > + * Relaxed matching policy, PMD may match only on items > + * with mask member set and skip matching on protocol > + * layers specified without any masks. > + * If not set, PMD will match on protocol layers > + * specified without any masks as well. > + * Packet data must be stacked in the same order as the > + * protocol layers to match inside packets, > + * starting from the lowest. > + */ > + uint32_t relaxed_matching:1; Consider rewording this to a bullet-formatted set of statements. For brevity. For improved clarity. > + * Flow attributes that will be used in the table. Perhaps: "Flow attributes to be used in each rule generated from this table". Something like that. > + struct rte_flow_item_template *item_templates[], Perhaps, "const struct"? The name could be "pattern_templates". > + uint8_t nb_item_templates, Why not "unsigned int"? The name could be "nb_pattern_templates". > + struct rte_flow_action_template *action_templates[], > + uint8_t nb_action_templates, Same questions here. -- Ivan M.