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 D55E942FAA; Mon, 31 Jul 2023 22:40:44 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6256042670; Mon, 31 Jul 2023 22:40:44 +0200 (CEST) Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by mails.dpdk.org (Postfix) with ESMTP id ADAB54161A for ; Mon, 31 Jul 2023 22:40:43 +0200 (CEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 2145C20002; Mon, 31 Jul 2023 20:40:39 +0000 (UTC) Message-ID: <820c8ce1-091a-cd9b-2dd2-c830ff0a503c@ovn.org> Date: Mon, 31 Jul 2023 22:41:22 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Cc: i.maximets@ovn.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 Content-Language: en-US To: David Marchand , dev@dpdk.org References: <20230505103102.2912297-1-david.marchand@redhat.com> <20230621144327.2202591-1-david.marchand@redhat.com> From: Ilya Maximets Subject: Re: [PATCH v4] ethdev: advertise flow restore in mbuf In-Reply-To: <20230621144327.2202591-1-david.marchand@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 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. Alternatively, the _register() could also be called right from the rte_flow_restore_info_dynflag() at a slight performance cost. It shouldn't be important though, since drivers do not seem to call it from a performance-sensitive code. Thoughts? Best regards, Ilya Maximets.