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 9A6A8A04DD; Fri, 23 Oct 2020 18:09:40 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8B351A939; Fri, 23 Oct 2020 18:09:35 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id A4F9CA938 for ; Fri, 23 Oct 2020 18:09:33 +0200 (CEST) IronPort-SDR: /HQ0P15lV4SAi6zqd9nUcc9DB6l9jV1sDldY6JxCtAXvCVg49uKgRYkUxP6S2yUGGFfPVe5uff 4CDsh1necE5Q== X-IronPort-AV: E=McAfee;i="6000,8403,9782"; a="185398621" X-IronPort-AV: E=Sophos;i="5.77,409,1596524400"; d="scan'208";a="185398621" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Oct 2020 09:08:26 -0700 IronPort-SDR: Kg+lvzzmkj8fBi6du6XHxIeVohrRC24XDiLINesWoD37OG9nY1F49y0XtwetPmJ7+uAph0gJts D6SMdyltJCyw== X-IronPort-AV: E=Sophos;i="5.77,409,1596524400"; d="scan'208";a="534443878" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.251.87.157]) ([10.251.87.157]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Oct 2020 09:08:23 -0700 To: Ruifeng Wang , David Marchand Cc: Bruce Richardson , dev , Honnappa Nagarahalli , nd , Kevin Traynor , "thomas@monjalon.net" References: <20200907081518.46350-1-ruifeng.wang@arm.com> <20201021030211.36381-1-ruifeng.wang@arm.com> <20201021030211.36381-3-ruifeng.wang@arm.com> From: "Medvedkin, Vladimir" Message-ID: <5b558902-f7c1-7da4-3ac8-830fdeaeb6e6@intel.com> Date: Fri, 23 Oct 2020 17:08:21 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.3.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 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" Hello, On 23/10/2020 07:13, Ruifeng Wang wrote: > >> -----Original Message----- >> From: David Marchand >> Sent: Thursday, October 22, 2020 11:14 PM >> To: Ruifeng Wang >> Cc: Bruce Richardson ; Vladimir Medvedkin >> ; dev ; Honnappa >> Nagarahalli ; nd ; Kevin >> Traynor ; thomas@monjalon.net >> Subject: Re: [PATCH v2 2/2] lpm: hide internal data >> >> On Wed, Oct 21, 2020 at 5:02 AM Ruifeng Wang >> wrote: >>> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index >>> 51a0ae578..88d31df6d 100644 >>> --- a/lib/librte_lpm/rte_lpm.c >>> +++ b/lib/librte_lpm/rte_lpm.c >>> @@ -42,9 +42,17 @@ enum valid_flag { >>> >>> /** @internal LPM structure. */ >>> struct __rte_lpm { >>> - /* LPM metadata. */ >>> + /* Exposed LPM data. */ >>> struct rte_lpm 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. */ >>> + /**< Rule info table. */ >>> + struct rte_lpm_rule_info rule_info[RTE_LPM_MAX_DEPTH]; >>> + struct rte_lpm_rule *rules_tbl; /**< LPM rules. */ >> >> - We hide the rules, is there a reason to keep struct rte_lpm_rule_info and >> struct rte_lpm_rule exposed? >> > These structs can be moved into rte_lpm.c and made internal too. Agree, these structs are control plane related and could be moved from public struct. > >> - Rather than have translations lpm -> i_lpm, in many places of this library, we >> should translate only in the functions exposed to the user. >> Besides, it is a bit hard to read between internal_lpm and i_lpm, I would >> adopt a single i_lpm convention for the whole file. >> I went and tried to do it (big search and replace + build tests, no runtime >> check though). >> This results in: >> https://github.com/david- >> marchand/dpdk/commit/4e61f0ce7cf2ac472565d3c6aa5bb78ffb8f70c9 >> >> What do you think? >> > I'm fine with the change. It looks good. > I checked unit test is passing. > Also checked with testfib app with "-c -a" args, FIB and LPM lookup returns same values. > Thanks. > /Ruifeng >> >> -- >> David Marchand > -- Regards, Vladimir