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 B723FA04B7; Tue, 13 Oct 2020 23:04:23 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 746371DB18; Tue, 13 Oct 2020 23:04:22 +0200 (CEST) Received: from mta114.f1.k8.com.br (mta114.f1.k8.com.br [187.73.32.186]) by dpdk.org (Postfix) with ESMTP id 3E8811DB0C for ; Tue, 13 Oct 2020 16:58:49 +0200 (CEST) Received: from [192.168.86.43] (pool-108-49-43-207.bstnma.fios.verizon.net [108.49.43.207]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by smtpz.f1.k8.com.br (Postfix) with ESMTPSA id 7BD8365; Tue, 13 Oct 2020 14:58:42 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtpz.f1.k8.com.br 7BD8365 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digirati.com.br; s=default; t=1602601125; bh=iNBMebithVRgkY7Ud+nJT0qMiEYdub/12jOBrMEZ2TM=; h=Subject:To:From:Date:Feedback-ID; b=p6NBaidaiwlE5cxN1S16D5jkJSGFUsxscMNAuGYKGt4mAyXOUcOLbxSD1nu9KKL2n dE3V06FGOL97DF4aWGv98Wr3/UYMQMaE5E9585KP7meTdGr7H9iGEp4QcLdR3oVT7k ZT+Nieq84g4Iic0z+i0p6BYAR3VHsl6LjlyefGlc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=k8.com.br; s=default; t=1602601125; bh=iNBMebithVRgkY7Ud+nJT0qMiEYdub/12jOBrMEZ2TM=; h=Subject:To:From:Date:Feedback-ID; b=VFP36NLKlanWZWTcVmDbR6BOG7LA/oe55s2YOcv2dW/TbUi2UVJKEE7FV+7VSILiW WFm1DLXwaiHeXeuPRYfU0xYwdaOEht9GgWKcPyYxVpSzv5ljIW3WSFLWzgAKAClPoB w3oCCgTCQaUES2q5w+UjkXoeBvFXGbUdQNL9TNqk= To: Kevin Traynor , Ruifeng Wang , "Medvedkin, Vladimir" , Bruce Richardson , Cody Doucette , Andre Nathan , Qiaobin Fu Cc: "dev@dpdk.org" , Honnappa Nagarahalli , nd References: <20200907081518.46350-1-ruifeng.wang@arm.com> <20200907081518.46350-3-ruifeng.wang@arm.com> <20200915160224.GA825@bricha3-MOBL.ger.corp.intel.com> <81eb8fde-cd4f-1df9-0ebb-05c902b30fe3@intel.com> <48834549-00e9-b762-4915-9a2dd0e5fe1d@redhat.com> From: Michel Machado Organization: Digirati Internet LTDA. Message-ID: <6497770e-9d1c-97c3-3834-84bd96186836@digirati.com.br> Date: Tue, 13 Oct 2020 10:58:39 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <48834549-00e9-b762-4915-9a2dd0e5fe1d@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-HN-S: bWljaGVsQGRpZ2lyYXRpLmNvbS5icg== X-HN-R: YW5kcmVAZGlnaXJhdGkuY29tLmJy a3RyYXlub3JAcmVkaGF0LmNvbQ== bWVuZ3hpYW5nMDgxMUBnbWFpbC5jb20= ZG91Y2V0dGVAYnUuZWR1 YnJ1Y2UucmljaGFyZHNvbkBpbnRlbC5jb20= dmxhZGltaXIubWVkdmVka2luQGludGVsLmNvbQ== UnVpZmVuZy5XYW5nQGFybS5jb20= Feedback-ID: MjAyMDEwMTM=:bWljaGVsQGRpZ2lyYXRpLmNvbS5icg==:ZGlnaXJhdGkuY29tLmJy:k8networks X-Mailman-Approved-At: Tue, 13 Oct 2020 23:04:21 +0200 Subject: Re: [dpdk-dev] [PATCH 2/2] lpm: hide internal data 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" Hi Kevin, We do need fields max_rules and number_tbl8s of struct rte_lpm, so the removal would force us to have another patch to our local copy of DPDK. We'd rather avoid this new local patch because we wish to eventually be in sync with the stock DPDK. Those fields are needed in Gatekeeper because we found a condition in an ongoing deployment in which the entries of some LPM tables may suddenly change a lot to reflect policy changes. To avoid getting into a state in which the LPM table is inconsistent because it cannot fit all the new entries, we compute the needed parameters to support the new entries, and compare with the current parameters. If the current table doesn't fit everything, we have to replace it with a new LPM table. If there were a way to obtain the struct rte_lpm_config of a given LPM table, it would cleanly address our need. We have the same need in IPv6 and have a local patch to work around it (see https://github.com/cjdoucette/dpdk/commit/3eaf124a781349b8ec8cd880db26a78115cb8c8f). Thus, an IPv4 and IPv6 solution would be best. PS: I've added Qiaobin Fu, another Gatekeeper maintainer, to this disscussion. [ ]'s Michel Machado On 10/13/20 9:53 AM, Kevin Traynor wrote: > Hi Gatekeeper maintainers (I think), > > fyi - there is a proposal to remove some members of a struct in DPDK LPM > API that Gatekeeper is using [1]. It would be only from DPDK 20.11 but > as it's an LTS I guess it would probably hit Debian in a few months. > > The full thread is here: > http://inbox.dpdk.org/dev/20200907081518.46350-1-ruifeng.wang@arm.com/ > > Maybe you can take a look and tell us if they are needed in Gatekeeper > or you can workaround it? > > thanks, > Kevin. > > [1] > https://github.com/AltraMayor/gatekeeper/blob/master/gt/lua_lpm.c#L235-L248 > > On 09/10/2020 07:54, Ruifeng Wang wrote: >> >>> -----Original Message----- >>> From: Kevin Traynor >>> Sent: Wednesday, September 30, 2020 4:46 PM >>> To: Ruifeng Wang ; Medvedkin, Vladimir >>> ; Bruce Richardson >>> >>> Cc: dev@dpdk.org; Honnappa Nagarahalli >>> ; nd >>> Subject: Re: [dpdk-dev] [PATCH 2/2] lpm: hide internal data >>> >>> On 16/09/2020 04:17, Ruifeng Wang wrote: >>>> >>>>> -----Original Message----- >>>>> From: Medvedkin, Vladimir >>>>> Sent: Wednesday, September 16, 2020 12:28 AM >>>>> To: Bruce Richardson ; Ruifeng Wang >>>>> >>>>> Cc: dev@dpdk.org; Honnappa Nagarahalli >>>>> ; nd >>>>> Subject: Re: [PATCH 2/2] lpm: hide internal data >>>>> >>>>> Hi Ruifeng, >>>>> >>>>> On 15/09/2020 17:02, Bruce Richardson wrote: >>>>>> On Mon, Sep 07, 2020 at 04:15:17PM +0800, Ruifeng Wang wrote: >>>>>>> Fields except tbl24 and tbl8 in rte_lpm structure have no need to >>>>>>> be exposed to the user. >>>>>>> Hide the unneeded exposure of structure fields for better ABI >>>>>>> maintainability. >>>>>>> >>>>>>> Suggested-by: David Marchand >>>>>>> Signed-off-by: Ruifeng Wang >>>>>>> Reviewed-by: Phil Yang >>>>>>> --- >>>>>>> lib/librte_lpm/rte_lpm.c | 152 >>>>>>> +++++++++++++++++++++++--------------- >>>>> - >>>>>>> lib/librte_lpm/rte_lpm.h | 7 -- >>>>>>> 2 files changed, 91 insertions(+), 68 deletions(-) >>>>>>> >>>>>> >>>>>>> diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h >>>>>>> index 03da2d37e..112d96f37 100644 >>>>>>> --- a/lib/librte_lpm/rte_lpm.h >>>>>>> +++ b/lib/librte_lpm/rte_lpm.h >>>>>>> @@ -132,17 +132,10 @@ struct rte_lpm_rule_info { >>>>>>> >>>>>>> /** @internal LPM structure. */ >>>>>>> struct rte_lpm { >>>>>>> - /* LPM metadata. */ >>>>>>> - char name[RTE_LPM_NAMESIZE]; /**< Name of the lpm. */ >>>>>>> - uint32_t max_rules; /**< Max. balanced rules per lpm. */ >>>>>>> - uint32_t number_tbl8s; /**< Number of tbl8s. */ >>>>>>> - struct rte_lpm_rule_info rule_info[RTE_LPM_MAX_DEPTH]; /**< >>>>> Rule info table. */ >>>>>>> - >>>>>>> /* LPM Tables. */ >>>>>>> struct rte_lpm_tbl_entry tbl24[RTE_LPM_TBL24_NUM_ENTRIES] >>>>>>> __rte_cache_aligned; /**< LPM tbl24 table. */ >>>>>>> struct rte_lpm_tbl_entry *tbl8; /**< LPM tbl8 table. */ >>>>>>> - struct rte_lpm_rule *rules_tbl; /**< LPM rules. */ >>>>>>> }; >>>>>>> >>>>>> >>>>>> Since this changes the ABI, does it not need advance notice? >>>>>> >>>>>> [Basically the return value point from rte_lpm_create() will be >>>>>> different, and that return value could be used by rte_lpm_lookup() >>>>>> which as a static inline function will be in the binary and using >>>>>> the old structure offsets.] >>>>>> >>>>> >>>>> Agree with Bruce, this patch breaks ABI, so it can't be accepted >>>>> without prior notice. >>>>> >>>> So if the change wants to happen in 20.11, a deprecation notice should >>>> have been added in 20.08. >>>> I should have added a deprecation notice. This change will have to wait for >>> next ABI update window. >>>> >>> >>> Do you plan to extend? or is this just speculative? >> It is speculative. >> >>> >>> A quick scan and there seems to be several projects using some of these >>> members that you are proposing to hide. e.g. BESS, NFF-Go, DPVS, >>> gatekeeper. I didn't look at the details to see if they are really needed. >>> >>> Not sure how much notice they'd need or if they update DPDK much, but I >>> think it's worth having a closer look as to how they use lpm and what the >>> impact to them is. >> Checked the projects listed above. BESS, NFF-Go and DPVS don't access the members to be hided. >> They will not be impacted by this patch. >> But Gatekeeper accesses the rte_lpm internal members that to be hided. Its compilation will be broken with this patch. >> >>> >>>> Thanks. >>>> Ruifeng >>>>>>> /** LPM RCU QSBR configuration structure. */ >>>>>>> -- >>>>>>> 2.17.1 >>>>>>> >>>>> >>>>> -- >>>>> Regards, >>>>> Vladimir >> >