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 8B880A04DB; Thu, 15 Oct 2020 21:30:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7CA411DDE1; Thu, 15 Oct 2020 21:30:32 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id CA7EE1DDE1 for ; Thu, 15 Oct 2020 21:30:29 +0200 (CEST) IronPort-SDR: NeYpEIBBTM8XUnCgrxxrFAzpDwedM7sEYsYBhWdO4M3xMRH0jxyP7uJ3LnrFyOR9v5DipBHtuT RISKvNdK9Bvw== X-IronPort-AV: E=McAfee;i="6000,8403,9775"; a="162971455" X-IronPort-AV: E=Sophos;i="5.77,380,1596524400"; d="scan'208";a="162971455" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Oct 2020 12:30:28 -0700 IronPort-SDR: tN5LebtKKuC1kemiXEmBsOJFo3KTIhBZwMXgW1/EhmGfIW3bYh+iTMj1QWnsFyzZnhtat4tG03 NG3KW6vlUwwg== X-IronPort-AV: E=Sophos;i="5.77,380,1596524400"; d="scan'208";a="531385571" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.252.20.243]) ([10.252.20.243]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Oct 2020 12:30:25 -0700 To: Honnappa Nagarahalli , Michel Machado , Kevin Traynor , Ruifeng Wang , Bruce Richardson , Cody Doucette , Andre Nathan , Qiaobin Fu Cc: "dev@dpdk.org" , nd References: <20200907081518.46350-1-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> <6497770e-9d1c-97c3-3834-84bd96186836@digirati.com.br> <18c44f31-abc0-c0b5-c2fb-76d6166d5237@digirati.com.br> <9197371c-5e03-4852-d62a-6456f0b762f0@intel.com> <9647f80d-53c3-33aa-b6d0-301aef34ca0a@intel.com> <781ddbaf-cfed-bc90-cf6c-2b88bfda1202@digirati.com.br> From: "Medvedkin, Vladimir" Message-ID: <505d3e00-717b-25f8-ffea-d6108165a12a@intel.com> Date: Thu, 15 Oct 2020 20:30:23 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 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 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 15/10/2020 18:38, Honnappa Nagarahalli wrote: > >> >> On 10/14/20 7:57 PM, Honnappa Nagarahalli wrote: >>>>>> On 13/10/2020 18:46, Michel Machado wrote: >>>>>>> On 10/13/20 11:41 AM, Medvedkin, Vladimir wrote: >>>>>>>> Hi Michel, >>>>>>>> >>>>>>>> Could you please describe a condition when LPM gets inconsistent? >>>>>>>> As I can see if there is no free tbl8 it will return -ENOSPC. >>>>>>> >>>>>>>     Consider this simple example, we need to add the following >>>>>>> two prefixes with different next hops: 10.99.0.0/16, >>>>>>> 18.99.99.128/25. If the LPM table is out of tbl8s, the second >>>>>>> prefix is not added and Gatekeeper will make decisions in >>>>>>> violation of the policy. The data structure of the LPM table is >>>>>>> consistent, but its content inconsistent with the policy. >>> max_rules and number_tbl8s in 'struct rte_lpm' contain the config >> information. These 2 fields do not change based on the routes added and do >> not indicate the amount of space left. So, you cannot use this information to >> decide if there is enough space to add more routes. Thanks Honnappa, agree, these two fields are read only after LPM initialization, I confused them with rte_fib's "rsvd_tbl8s" and "cur_tbl8s", so there is no need to read them directly from LPM after initialization. I'd suggest just keeping them as external variables outside of the LPM library (in kind of a global configuration I suppose?). >> >> We are aware that those fields hold the config information not a status of >> the LPM table. >> >> Before updating a LPM table that holds network prefixes derived from >> threat intelligence, we compute the minimum values for max_rules and >> number_tbl8s. Here is an example of how we do it: >> https://github.com/AltraMayor/gatekeeper/blob/95d1d6e8201861a0d0c698 >> bfd06ad606674f1e07/lua/examples/policy.lua#L135-L166 >> >> Once these minimum values are available, we get the parameters of the >> LPM table to be updated and check if we can update it, or have to recreate it. >> >>>>>> Aha, thanks. So do I understand correctly that you need to add a >>>>>> set of routes atomically (either the entire set is installed or nothing)? >>>>> >>>>>    Yes. >>>>> >>>>>> If so, then I would suggest having 2 lpm and switching them >>>>>> atomically after a successful addition. As for now, even if you >>>>>> have enough tbl8's, routes are installed non atomically, i.e. there >>>>>> will be a time gap between adding two routes, so in this time >>>>>> interval the table will be inconsistent with the policy. >>>>>> Also, if new lpm algorithms are added to the DPDK, they won't have >>>>>> such a thing as tbl8. >>>>> >>>>>    Our code already deals with synchronization. >>> If the application code already deals with synchronization, is it possible to >> revert back (i.e. delete the routes that got added so far) when the addition >> of the route-set fails? >> >> The way the code is structured, this would require a significant rewrite >> because the code assumes that it will succeed since the capacity of the LPM >> tables was already checked. >> >>>>>>>> On 13/10/2020 15:58, Michel Machado wrote: >>>>>>>>> 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/3eaf124a781349b8ec8cd880db >>>> 26a78115cb8c8f). >>> I do not see why such an API is not possible, we could add one API that >> returns max_rules and number_tbl8s (essentially, the config that was passed >> to rte_lpm_create API). >>> But, is there a possibility to store that info in the application as that data >> was passed to rte_lpm from the application? >> >> A suggestion for what this API could look like: >> >> void rte_lpm_get_config(const struct rte_lpm *lpm, struct rte_lpm_config >> *config); void rte_lpm6_get_config(const struct rte_lpm6 *lpm, struct >> rte_lpm6_config *config); >> >> If the final choice is for not supporting a way to retrieve the config >> information on the API, we'll look for a place to keep a copy of the >> parameters in our code. > IMO, this is not a performance critical path and it is not a difficult solution to store these values in the application. My suggestion is to skip adding the API and store the values in the application. > Vladimir, what's your opinion? Agree. Global vars or part of a global configuration could be used here. > -- Regards, Vladimir