From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4C570A0527; Mon, 9 Nov 2020 11:21:33 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2E4EC5A62; Mon, 9 Nov 2020 11:21:32 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id BCFEC5A62 for ; Mon, 9 Nov 2020 11:21:30 +0100 (CET) IronPort-SDR: 6OjzoK7HOwvJHeFvm0vGyrIKODFcl1dcssg4LqtpRaVfOC5l2xiuEywdlNT6A2eDxoXjjkDWag /Vi7e3x58oYg== X-IronPort-AV: E=McAfee;i="6000,8403,9799"; a="167193330" X-IronPort-AV: E=Sophos;i="5.77,463,1596524400"; d="scan'208";a="167193330" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Nov 2020 02:21:27 -0800 IronPort-SDR: t9FZHggaM6M+gbcYvEXEnGgcElYrwFEZIo3PY0g5K58lUWXYe5iXKTdVSaxoin8y1MhyiIvi+X Ns9PbaKp+/LQ== X-IronPort-AV: E=Sophos;i="5.77,463,1596524400"; d="scan'208";a="530677913" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.193.177]) ([10.213.193.177]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Nov 2020 02:21:25 -0800 To: Matan Azrad , Wenzhuo Lu , Beilei Xing , Bernard Iremonger , Ori Kam Cc: "dev@dpdk.org" References: <1604252927-213452-1-git-send-email-matan@nvidia.com> <1604611973-64970-1-git-send-email-matan@nvidia.com> <970eeb8a-b18b-fc89-7659-02fa6020d19b@intel.com> From: Ferruh Yigit Message-ID: Date: Mon, 9 Nov 2020 10:21:22 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: support age shared action context X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11/7/2020 5:30 PM, Matan Azrad wrote: > Hi Ferruh > > From: Ferruh Yigit >> On 11/5/2020 9:32 PM, Matan Azrad wrote: >>> When an age action becomes aged-out the rte_flow_get_aged_flows should >>> return the action context supplied by the configuration structure. >>> >>> In case the age action created by the shared action API, the shared >>> action context of the Testpmd application was not set. >>> >>> In addition, the application handler of the contexts returned by the >>> rte_flow_get_aged_flows API didn't consider the fact that the action >>> could be set by the shared action API and considered it as regular >>> flow context. >>> >>> This caused a crash in Testpmd when the context is parsed. >>> >>> This patch set context type in the flow and shared action context and >>> uses it to parse the aged-out contexts correctly. >>> >>> Signed-off-by: Matan Azrad >>> Acked-by: Dekel Peled >>> --- >>> app/test-pmd/config.c | 119 ++++++++++++++++++++++++++++++++++------- >> -------- >>> app/test-pmd/testpmd.h | 5 +++ >>> 2 files changed, 87 insertions(+), 37 deletions(-) >>> >>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index >>> 755d1df..00a7dd1 100644 >>> --- a/app/test-pmd/config.c >>> +++ b/app/test-pmd/config.c >>> @@ -1763,6 +1763,33 @@ void port_flow_tunnel_create(portid_t port_id, >> const struct tunnel_ops *ops) >>> } >>> } >>> >>> +#define AGE_ACTION_TYPE_MASK 0x3u >>> + >>> +static void >>> +set_age_action_context(void **ctx, enum action_age_context_type type, >>> +void *obj) { >>> + uintptr_t value = (uintptr_t)obj; >>> + >>> + /* >>> + * obj is allocated by malloc\calloc which must return an address >>> + * aligned to 8. >>> + * Use the last 2 bits for the age context type. >>> + */ >>> + value |= (uintptr_t)type & AGE_ACTION_TYPE_MASK; >>> + *ctx = (void *)value; >> >> Thanks Matan, I think this is much clear. But I though the 'id' will be used, not >> the pointer itself, like "uintptr_t value = id | (type * MASK)" >> OR the address pointer and type seems error prone, although you comment >> you rely on the alignment. > > I understand your concern, that's why the context value management is wrapped well by dedicated functions for set and parse. > Also it's very optimized way for memory and time especially when we are talking about big scale(see below). > >> The testpmd usage also kind of sample usage for the applications, I am for not >> suggesting this for the user applications. > > >> Reserving the two bit of the 'id' reduces the usable 'id' to 30 bits, but it looks >> still big enough, what do you think? > > > Yes, it is big enough. > The problem with the id is the latency to get the pointer from it. > Since both the flows and the shared actions are saved in a list we need to traverse all the list in order to get the pointer and the needed information. > Using 'id' was your idea. OK, what about back to previous suggestion, adding a new data struct for both pointers and type? Your concern there was the memory consumption, yes although it will require more memory the amount is not unreasonable.