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 1B5F042648; Tue, 26 Sep 2023 21:48:43 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DDD9B40271; Tue, 26 Sep 2023 21:48:42 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by mails.dpdk.org (Postfix) with ESMTP id 2AA0140269 for ; Tue, 26 Sep 2023 21:48:41 +0200 (CEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id E3FB7240003; Tue, 26 Sep 2023 19:48:35 +0000 (UTC) Message-ID: <9213d474-779c-e4ec-845a-5ba43d64a95c@ovn.org> Date: Tue, 26 Sep 2023 21:49:27 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Cc: i.maximets@ovn.org, dev@dpdk.org, thomas@monjalon.net, alialnu@nvidia.com, Andrew Rybchenko , Viacheslav Ovsiienko , Ori Kam , Aman Singh , Yuying Zhang , Matan Azrad , Suanming Mou , David Christensen , Ruifeng Wang , Bruce Richardson , Konstantin Ananyev , Ferruh Yigit Subject: Re: [PATCH v4] ethdev: advertise flow restore in mbuf Content-Language: en-US To: David Marchand References: <20230505103102.2912297-1-david.marchand@redhat.com> <20230621144327.2202591-1-david.marchand@redhat.com> <820c8ce1-091a-cd9b-2dd2-c830ff0a503c@ovn.org> From: Ilya Maximets In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Sasl: i.maximets@ovn.org 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 On 9/26/23 11:17, David Marchand wrote: > Hello Ilya, > > On Mon, Jul 31, 2023 at 10:40 PM Ilya Maximets wrote: >> On 6/21/23 16:43, David Marchand wrote: >>> As reported by Ilya [1], unconditionally calling >>> rte_flow_get_restore_info() impacts an application performance for drivers >>> that do not provide this ops. >>> It could also impact processing of packets that require no call to >>> rte_flow_get_restore_info() at all. >>> >>> Register a dynamic mbuf flag when an application negotiates tunnel >>> metadata delivery (calling rte_eth_rx_metadata_negotiate() with >>> RTE_ETH_RX_METADATA_TUNNEL_ID). >>> >>> Drivers then advertise that metadata can be extracted by setting this >>> dynamic flag in each mbuf. >>> >>> The application then calls rte_flow_get_restore_info() only when required. >>> >>> Link: http://inbox.dpdk.org/dev/5248c2ca-f2a6-3fb0-38b8-7f659bfa40de@ovn.org/ >>> Signed-off-by: David Marchand >>> Acked-by: Andrew Rybchenko >>> Acked-by: Viacheslav Ovsiienko >>> Tested-by: Ali Alnubani >>> Acked-by: Ori Kam >>> --- >>> Changes since RFC v3: >>> - rebased on next-net, >>> - sending as non RFC for CIs that skip RFC patches, >>> >>> Changes since RFC v2: >>> - fixed crash introduced in v2 and removed unneeded argument to >>> rte_flow_restore_info_dynflag_register(), >>> >>> Changes since RFC v1: >>> - rebased, >>> - updated vectorized datapath functions for net/mlx5, >>> - moved dynamic flag register to rte_eth_rx_metadata_negotiate() and >>> hid rte_flow_restore_info_dynflag_register() into ethdev internals, >>> >>> --- >>> app/test-pmd/util.c | 9 +++-- >>> drivers/net/mlx5/mlx5.c | 2 + >>> drivers/net/mlx5/mlx5.h | 5 ++- >>> drivers/net/mlx5/mlx5_flow.c | 47 +++++++++++++++++++++--- >>> drivers/net/mlx5/mlx5_rx.c | 2 +- >>> drivers/net/mlx5/mlx5_rx.h | 1 + >>> drivers/net/mlx5/mlx5_rxtx_vec_altivec.h | 16 ++++---- >>> drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 6 +-- >>> drivers/net/mlx5/mlx5_rxtx_vec_sse.h | 6 +-- >>> drivers/net/mlx5/mlx5_trigger.c | 4 +- >>> drivers/net/sfc/sfc_dp.c | 14 +------ >>> lib/ethdev/rte_ethdev.c | 5 +++ >>> lib/ethdev/rte_flow.c | 27 ++++++++++++++ >>> lib/ethdev/rte_flow.h | 18 ++++++++- >>> lib/ethdev/rte_flow_driver.h | 6 +++ >>> lib/ethdev/version.map | 1 + >>> 16 files changed, 128 insertions(+), 41 deletions(-) >> >> >> >>> diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h >>> index 356b60f523..f9fb01b8a2 100644 >>> --- a/lib/ethdev/rte_flow_driver.h >>> +++ b/lib/ethdev/rte_flow_driver.h >>> @@ -376,6 +376,12 @@ struct rte_flow_ops { >>> const struct rte_flow_ops * >>> rte_flow_ops_get(uint16_t port_id, struct rte_flow_error *error); >>> >>> +/** >>> + * Register mbuf dynamic flag for rte_flow_get_restore_info. >>> + */ >>> +int >>> +rte_flow_restore_info_dynflag_register(void); >>> + >> >> Hi, David, others. >> >> Is there a reason to not expose this function to the application? >> >> The point is that application will likely want to know the value >> of the flag before creating any devices. I.e. request it once >> and use for all devices later without performing a call to an >> external library (DPDK). In current implementation, application >> will need to open some device first, and only then the result of >> rte_flow_restore_info_dynflag() will become meaningful. >> >> There is no need to require application to call this function, >> it can still be called from the rx negotiation API, but it would >> be nice if application could know it beforehand, i.e. had control >> over when the flag is actually becomes visible. > > DPDK tries to register flags only when needed, as there is not a lot > of space for dyn flags. > Some drivers take some space and applications want some share too. > > DPDK can export the _register function for applications to call it > regardless of what driver will be used later. > > Yet, I want to be sure why it matters in OVS context. > Is it not enough resolving the flag (by calling > rte_flow_restore_info_dynflag()) once rte_eth_rx_metadata_negotiate > for tunnel metadata is called? > Do you want to avoid an atomic store/load between OVS main thread and > PMD threads? Yeas, something like this. I do not have a solid implementation idea for that though. I replied to your OVS patch. Best regards, Ilya Maximets.