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 E76B54240D; Wed, 18 Jan 2023 14:57:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 893CA400D6; Wed, 18 Jan 2023 14:57:03 +0100 (CET) Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by mails.dpdk.org (Postfix) with ESMTP id 7C1C54003F for ; Wed, 18 Jan 2023 14:57:02 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id E1A1D3200312; Wed, 18 Jan 2023 08:57:00 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Wed, 18 Jan 2023 08:57:01 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm3; t=1674050220; x= 1674136620; bh=Y+1rohJpuOqEZuiqLIX0+t67JhRAx6dARtW4uh6p01g=; b=g W23apXqSaZ9zL5rnDV10a27aHDVcrDb+kmUL/bsCFxis1/pN9KhHCkDSSwjArrpc y3CFhodyH2ivlFavl81x0QOptZgnEObte71hDbKL7EwXDbT/svR0ZYTQh6G3KnY7 JiU+bsZkChF+tCcYQWhuI3PZ3d/mpcuTgcpAUczaU829YYt43V32qQgN0yvcBcYf XF5wot6GyrJqUS6xa4/b4XteybozmlGUYIiZddofhZAfNSY1Oglv809D9Jgknp1h hUrRNfAPhWZseGHB2LGXuOqWKi63Y8c3OnQsIxtZ6Mk748IwYl6+lt3s8K7sQc1o vVIeIWrbYq8K87muahbrQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1674050220; x= 1674136620; bh=Y+1rohJpuOqEZuiqLIX0+t67JhRAx6dARtW4uh6p01g=; b=M 6ZaF6+8qZZNe9Yycg78gB63CMGRWQmERkuo1DZDSIdTFxfJa3zE7R6H67ix+keIz VB3EvYOctO4I1E//G8uwcLKrP9CBTjY7vQ+807fzClLRXSdqUoEpPmwu7eEpsXBp XAV/bTYoUBr9DUChMzhW4Pge8rOTKjhuBP8dnZcrklDkN0O/OlxyviW8KgZ8ZuZA sMf1wrBjcuo5vO5lDDeHit+oX1zfrELjYMwlWaGOk3rOLmu2k/exR4xeFyazK2V/ ofiTAk87xd/HPVMfre1rth8eSVTgc3gFRSOz4+FSH2bvtD3+UFP1sAn95mly9JF6 c3c1E0hx3yabR0d6oOuUg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedruddtkedgheekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedtjeeiieefhedtfffgvdelteeufeefheeujefgueetfedttdei kefgkeduhedtgfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 18 Jan 2023 08:56:58 -0500 (EST) From: Thomas Monjalon To: Gregory Etelson Cc: dev@dpdk.org, matan@nvidia.com, rasland@nvidia.com, Ori Kam , Ferruh Yigit , Andrew Rybchenko , ivan.malov@oktetlabs.ru Subject: Re: [PATCH v4 1/2] ethdev: add query_update sync and async function calls Date: Wed, 18 Jan 2023 14:56:56 +0100 Message-ID: <4220871.e99z0qppnp@thomas> In-Reply-To: <20230118103109.4052-1-getelson@nvidia.com> References: <20221221073547.988-1-getelson@nvidia.com> <20230118103109.4052-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 18/01/2023 11:31, Gregory Etelson: > Current API allows either query or update indirect flow action. > If port hardware allows both update and query in a single operation, > application still has to issue 2 separate hardware requests. > > The patch adds `rte_flow_action_handle_query_update` function call, > and it's async version `rte_flow_async_action_handle_query_update` > to atomically query and update flow action. What is the benefit? Is it a performance optimization? How much? > int > rte_flow_action_handle_query_update > (uint16_t port_id, struct rte_flow_action_handle *handle, > const void *update, void *query, > enum rte_flow_query_update_mode mode, > struct rte_flow_error *error); > > int > rte_flow_async_action_handle_query_update > (uint16_t port_id, uint32_t queue_id, > const struct rte_flow_op_attr *op_attr, > struct rte_flow_action_handle *action_handle, > const void *update, void *query, > enum rte_flow_query_update_mode mode, > void *user_data, struct rte_flow_error *error); No need to copy the functions in the commit log. > Application can control query and update order, if that is supported > by port hardware, by setting qu_mode parameter to > RTE_FLOW_QU_QUERY_FIRST or RTE_FLOW_QU_UPDATE_FIRST. > > RTE_FLOW_QU_QUERY and RTE_FLOW_QU_UPDATE parameter values provide > query only and update only functionality for backward compatibility > with existing API. Why do we need such compatibility? The existing functions will stay, isn't it? [...] > + * RTE_FLOW_QU_QUERY_FIRST > + * Force port to query action before update. > + * RTE_FLOW_QU_UPDATE_FIRST > + * Force port to update action before update. You mean "before query". Anyway you don't need to repeat the enum in the comment. > + * > + * @see rte_flow_action_handle_query_update() > + * @see rte_flow_async_action_handle_query_update() > + */ > +enum rte_flow_query_update_mode { > + RTE_FLOW_QU_QUERY_FIRST, /* query before update */ > + RTE_FLOW_QU_UPDATE_FIRST, /* query after update */ > +}; > + > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Query and/or update indirect flow action. > + * If update parameter is NULL the function queries indirect action. > + * If query parameter is NULL the function updates indirect action. > + * If both query and update not NULL, the function atomically > + * queries and updates indirect action. Query and update carried in order "are" carried? > + * specified in the mode parameter. > + * > + * @param port_id > + * Port identifier of Ethernet device. > + * @param handle > + * Handle for the indirect action object to be updated. > + * @param update > + * Update profile specification used to modify the action pointed by handle. > + * *update* could be with the same type of the immediate action corresponding could be "of" the same type > + * to the *handle* argument when creating, or a wrapper structure includes > + * action configuration to be updated and bit fields to indicate the member > + * of fields inside the action to update. > + * @param query > + * Pointer to storage for the associated query data type. > + * @param mode > + * Operational mode. > + * @param error > + * Perform verbose error reporting if not NULL. > + * PMDs initialize this structure in case of error only. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_errno is set. > + * - (ENOTSUP) if underlying device does not support this functionality. > + * - (EINVAL) - if *handle* invalid > + */ > +__rte_experimental > +int > +rte_flow_action_handle_query_update(uint16_t port_id, > + struct rte_flow_action_handle *handle, > + const void *update, void *query, > + enum rte_flow_query_update_mode mode, > + struct rte_flow_error *error); > + > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Enqueue async indirect flow action query and/or update > + * > + * @param port_id > + * Port identifier of Ethernet device. > + * @param queue_id > + * Flow queue which is used to update the rule. > + * @param attr > + * Indirect action update operation attributes. > + * @param handle > + * Handle for the indirect action object to be updated. > + * @param update > + * Update profile specification used to modify the action pointed by handle. > + * *update* could be with the same type of the immediate action corresponding > + * to the *handle* argument when creating, or a wrapper structure includes includes -> including > + * action configuration to be updated and bit fields to indicate the member > + * of fields inside the action to update. Where can we find how such wrapper should look like? > + * @param query > + * Pointer to storage for the associated query data type. > + * Query result returned on async completion event. > + * @param mode > + * Operational mode. > + * @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 on success, a negative errno value otherwise and rte_errno is set. > + * - (ENOTSUP) if underlying device does not support this functionality. > + * - (EINVAL) - if *handle* invalid > + */ > +__rte_experimental > +int > +rte_flow_async_action_handle_query_update(uint16_t port_id, uint32_t queue_id, > + const struct rte_flow_op_attr *attr, > + struct rte_flow_action_handle *handle, > + const void *update, void *query, > + enum rte_flow_query_update_mode mode, > + void *user_data, > + struct rte_flow_error *error); > --- a/lib/ethdev/version.map > +++ b/lib/ethdev/version.map > @@ -298,6 +298,11 @@ EXPERIMENTAL { > rte_flow_get_q_aged_flows; > rte_mtr_meter_policy_get; > rte_mtr_meter_profile_get; > + > + # future It should be "# added in 23.03" > + rte_flow_action_handle_query_update; > + rte_flow_async_action_handle_query_update;