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 58AEF43B01; Mon, 12 Feb 2024 15:02:46 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 40EE5402D9; Mon, 12 Feb 2024 15:02:46 +0100 (CET) Received: from fout5-smtp.messagingengine.com (fout5-smtp.messagingengine.com [103.168.172.148]) by mails.dpdk.org (Postfix) with ESMTP id D134B402D0 for ; Mon, 12 Feb 2024 15:02:44 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfout.nyi.internal (Postfix) with ESMTP id 4297A1380058; Mon, 12 Feb 2024 09:02:44 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Mon, 12 Feb 2024 09:02:44 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm3; t=1707746564; x=1707832964; bh=0HdZcQzB5OU50F3JS1AVKlGft+XKgR5OFFXO+ZMlVc4=; b= J37Td7LCjwidBNTgIab8aQdXyaUTzTJJ9/JKZ9YyglGxU2bONpi630chTUsR2pT8 JsXKxRz/3amtAS0ud+PUuIK76IcBm76fAjip6zP2Jd8Zm+kLKfTiqZxxjVMXMZwK uYMQ/9YY9DMu4PNmHyrUKwM+NphCqGGqtqkiJFZ8At1eHrYsYcjDfYYpjjJ1oiqd HtV+shxI4dbA3j4EdNMKyhd8lLvo8WT1IB9Bg4msUT+KGXJQAopVXtDXuBpsvcdJ QN+NmNmqjqNBDP7mQY7B+RXcyEh1EPaFx5vF4ez4Ho6nx2Z8QDt6q7xTgEJNby8D GSFmB/o43QF2+OmF/V/tIw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1707746564; x= 1707832964; bh=0HdZcQzB5OU50F3JS1AVKlGft+XKgR5OFFXO+ZMlVc4=; b=m fnIfHNcromc0RZcmpdeYrLgBQGcnr7YP0VpXU0kMDqkwQo3fHG4KKohJllRFD11q Mv3k87ki5dbtBSe7hyhYnLZXtfD3BckKkE8g/ZNIWJL42DvYG7N1wB29SMdSMNi7 GTWSsiDsRBLGngB3zG1Vg3d66EI3lAgRv8nKcxjlbB3XIg3V1DdypRcwNbTqO7cm znU57IiWHIQpBkXTdDvL5d5+xvUDKjNmQqV7TdRp71KoCgY0/NaMnZc/JSwefRiY CsIUpz/5D1JJs7vtfhISqFrBHx2mw6r/Iv3BQ4eAEZ+Ebu55e+RqB3b+r6PWAy8b SNOu7YGbtFmdoqmfkpFjQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefgdehjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddtieek gfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 12 Feb 2024 09:02:41 -0500 (EST) From: Thomas Monjalon To: Gregory Etelson Cc: dev@dpdk.org, mkashani@nvidia.com, Ori Kam , Aman Singh , Yuying Zhang , Ferruh Yigit , Andrew Rybchenko Subject: Re: [PATCH v5] ethdev: add template table resize API Date: Mon, 12 Feb 2024 15:02:39 +0100 Message-ID: <2811388.XrmoMso0CX@thomas> In-Reply-To: <20240211093053.397469-1-getelson@nvidia.com> References: <20231217093205.321082-1-getelson@nvidia.com> <20240211093053.397469-1-getelson@nvidia.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; 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 11/02/2024 10:30, Gregory Etelson: > --- a/doc/guides/howto/rte_flow.rst > +++ b/doc/guides/howto/rte_flow.rst > +Template API resizable table > +---------------------------- > + > +Description > +~~~~~~~~~~~ > + > +A guide to the resizable template table API. This sentence is useless. Let's keep it as short as we can. > + > +The resizable template table API enables applications to dynamically adjust > +capacity of template tables without disrupting the existing flows operation. > +The resizable template table API allows applications to optimize the memory > +usage and performance of template tables according to the traffic conditions > +and requirements. > + > +A typical use case for the resizable template table API You may add a colon at the end of this sentence. The below you may use "#" for automatic list numbering. > + > + 1. Create a resizable table with the initial capacity. > + > + 2. Change the table flows capacity. > + > + 3. Update flows that were created before the table update. > + > + 4. Complete the table resize procedure. > + > +When application begins to resize the table, it enters the resizable state. I'm not sure to understand what means "enters the resizable state". > +When application finishes resizing the table, it returns to the normal state. > +Only a table in the normal state can be resized. This sentence looks redundant with the following one. > After a table is back to > +the normal state, application can start a new resize. > +Application can add, change or remove flow rules regardless of table state. > +Table performance may worsen in the resizable state. Table performance must > +recover after the table is back to the normal state. This last sentence looks redundant. > + > +Table resize procedure must not interfere with flows that existed before > +the table size changed. Who the "must" is for? I suppose you are talking about driver requirement? > +Flow handles must remain unchanged during table resize. Again, it is for driver behaviour? > +Application must be able to create new flows and modify or delete existing flows > +regardless of the table state. If it is by design we may say "Application is able". > + > +Application needs to set the `RTE_FLOW_TABLE_SPECIALIZE_RESIZABLE` bit in Please use double backquotes for code items. > +the table attributes when creating a template table that can be resized. > +The current API cannot make an existing table resizable if it was not created In the guide, there is no old or current API. > +with the `RTE_FLOW_TABLE_SPECIALIZE_RESIZABLE` bit. I think this sentence is redundant. > +Resizable template table starts in the normal state. It looks obvious. Remember a guide is doc a detailed API description. > + > +Application can trigger the table to resize by calling > +the `rte_flow_template_table_resize()` function. The resize process updates > +the PMD table settings and port hardware to fit the new flows capacity. > +The resize process must not affect the current flows functionality. > +The resize process must not change the current flows handles. Is it redundant with what is above? Try to be concise. > +Application can create new flows and modify or delete existing flows > +while the table is resizing, but the table performance might be > +slower than usual. It was already explained before. I prefer this one. > + > +Flows that existed before table resize are still functional after table resize. > +However, the PMD flow resources that existed before table resize may not be > +fully efficient after table resize. What is "not fully efficient" exactly? > In this case, application can combine > +the old flow resources from before the resize with the new flow resources > +from after the resize. > +Application uses the `rte_flow_async_update_resized()` function call to update > +flow resources. The flow update process does not interfere with or alter > +the existing flow object. It only updates the PMD resources associated with that > +flow. > +The post-resize flow update process may conflict with application flows > +operations, such as creation, removal or update. Therefore, performance-oriented > +applications need to choose the best time to call for post-resize flow update. Not perfectly clear. What must be done by the app exactly? > +When application selects flows for the post table resize update, it can iterate > +over all existing flows or it can keep track of the flows that need > +to be updated. > +Flows that were created after the `rte_flow_template_table_resize()` > +call finished do not require an update. > + > +To return table to the normal state, use the > +`rte_flow_template_table_resize_complete()`. If PMD does not require post-resize > +flows How do we know what the PMD requires? > update and application does not care about PMD resources optimization, > +application can avoid post-resize flows update and move resized table back to > +the normal state right after the `rte_flow_template_table_resize()`. > +Application can resize the table again when it is in the normal state. > + > +Testpmd commands:(wrapped for clarity):: You should replace the first colon with a space. > + > + # 1. Create resizable template table for 1 flow. > + testpmd> flow pattern_template 0 create ingress pattern_template_id 3 > + template eth / ipv4 / udp src mask 0xffff / end > + testpmd> flow actions_template 0 create ingress actions_template_id 7 > + template count / rss / end > + testpmd> flow template_table 0 create table_id 101 resizable ingress > + group 1 priority 0 rules_number 1 > + pattern_template 3 actions_template 7 > + > + # 2. Queue a flow rule. > + testpmd> flow queue 0 create 0 template_table 101 > + pattern_template 0 actions_template 0 postpone no > + pattern eth / ipv4 / udp src spec 1 / end actions count / rss / end > + > + # 3. Resize the template table > + # The new table capacity is 32 rules > + testpmd> flow template_table 0 resize table_resize_id 101 Why not just "resize table 101" ? > + table_resize_rules_num 32 > + > + # 4. Queue more flow rules. > + testpmd> flow queue 0 create 0 template_table 101 > + pattern_template 0 actions_template 0 postpone no > + pattern eth / ipv4 / udp src spec 2 / end actions count / rss / end > + testpmd> flow queue 0 create 0 template_table 101 > + pattern_template 0 actions_template 0 postpone no > + pattern eth / ipv4 / udp src spec 3 / end actions count / rss / end > + testpmd> flow queue 0 create 0 template_table 101 > + pattern_template 0 actions_template 0 postpone no > + pattern eth / ipv4 / udp src spec 4 / end actions count / rss / end > + > + # 5. Queue the initial flow update. > + testpmd> flow queue 0 update_resized 0 rule 0 > + > + # 6. Complete the table resize. > + testpmd> flow template_table 0 resize_complete table 101 > diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst > index 6f8ad27808..047664a079 100644 > --- a/doc/guides/rel_notes/release_24_03.rst > +++ b/doc/guides/rel_notes/release_24_03.rst > @@ -55,6 +55,8 @@ New Features > Also, make sure to start the actual text at the margin. > ======================================================= > > +* **Added support for template API table resize.** Would be better to give a quick description to say it is adding a new table creation flag and 4 new functions. [...] > --- a/lib/ethdev/rte_flow.h > +++ b/lib/ethdev/rte_flow.h > +/** > + * Specialize table for resize. > + */ > +#define RTE_FLOW_TABLE_SPECIALIZE_RESIZABLE RTE_BIT32(2) > /**@}*/ [...] > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Query whether a table can be resized. > + * > + * @param port_id > + * Port identifier of Ethernet device. > + * @param tbl_attr > + * Template table. > + * > + * @return > + * True if the table can be resized. This is not aligned, probably because of parameters above. > + */ > +__rte_experimental > +bool > +rte_flow_table_resizable(__rte_unused uint16_t port_id, > + const struct rte_flow_template_table_attr *tbl_attr); > + [...] > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Change template table flow rules capacity. > + * PMD implementation must support table change to the new size. I don't think it is the good place for this PMD comment. > + * > + * @param port_id > + * Port identifier of Ethernet device. > + * @param table > + * Template table to modify. > + * @param nb_rules > + * New flow rules capacity. > + * @param error > + * Perform verbose error reporting if not NULL. > + * PMDs initialize this structure in case of error only. > + * > + * @return > + * - (0) if success. > + * - (-ENODEV) if *port_id* invalid. > + * - (-ENOTSUP) if underlying device does not support this functionality. > + * - (-EINVAL) if *table* cannot be resized or resize to *nb_rules* > + * is not supported in PMD. > + */ > +__rte_experimental > +int > +rte_flow_template_table_resize(uint16_t port_id, > + struct rte_flow_template_table *table, > + uint32_t nb_rules, > + struct rte_flow_error *error); > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Following table resize, update flow resources in port. You need to give details here about why, when to do it, and what are the constraints. It is really better to have all details in doxygen. The RST is more to explain the global flow of the process. > + * > + * @param port_id > + * Port identifier of Ethernet device. > + * @param queue > + * Flow queue for async operation. > + * @param attr > + * Async operation attributes. > + * @param rule > + * Flow rule to update. > + * @param user_data > + * The user data that will be returned on async completion event. > + * @param error > + * Perform verbose error reporting if not NULL. > + * PMDs initialize this structure in case of error only. > + * > + * @return > + * - (0) if success. > + * - (-ENODEV) if *port_id* invalid. > + * - (-ENOTSUP) if underlying device does not support this functionality. > + * - (-EINVAL) if *rule* cannot be updated. > + */ > +__rte_experimental > +int > +rte_flow_async_update_resized(uint16_t port_id, uint32_t queue, > + const struct rte_flow_op_attr *attr, > + struct rte_flow *rule, void *user_data, > + struct rte_flow_error *error); > + > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Following table resize, notify port that all table flows were updated. You need to give details here about when to call it. > + * > + * @param port_id > + * Port identifier of Ethernet device. > + * @param table > + * Template table that undergoing resize operation. > + * @param error > + * Perform verbose error reporting if not NULL. > + * PMDs initialize this structure in case of error only. > + * > + * @return > + * - (0) if success. > + * - (-ENODEV) if *port_id* invalid. > + * - (-ENOTSUP) if underlying device does not support this functionality. > + * - (-EINVAL) PMD cannot complete table resize. > + */ > +__rte_experimental > +int > +rte_flow_template_table_resize_complete(uint16_t port_id, > + struct rte_flow_template_table *table, > + struct rte_flow_error *error); [...] > --- a/lib/ethdev/version.map > +++ b/lib/ethdev/version.map > + # added in 24.03 > + rte_flow_table_resizable; > + rte_flow_template_table_resize; > + rte_flow_async_update_resized; > + rte_flow_template_table_resize_complete; alphabetical ordering here please