DPDK patches and discussions
 help / color / Atom feed
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: "Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>,
	Michel Machado <michel@digirati.com.br>,
	Kevin Traynor <ktraynor@redhat.com>,
	Ruifeng Wang <Ruifeng.Wang@arm.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	Cody Doucette <doucette@bu.edu>,
	Andre Nathan <andre@digirati.com.br>,
	Qiaobin Fu <mengxiang0811@gmail.com>,
	"techboard@dpdk.org" <techboard@dpdk.org>,
	David Marchand <david.marchand@redhat.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>, nd <nd@arm.com>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH 2/2] lpm: hide internal data
Date: Thu, 15 Oct 2020 22:54:11 +0000
Message-ID: <DBAPR08MB581430BDB3D55AF6F611C0B498020@DBAPR08MB5814.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <505d3e00-717b-25f8-ffea-d6108165a12a@intel.com>

<snip>
> 
> Hello,
> 
> On 15/10/2020 18:38, Honnappa Nagarahalli wrote:
> > <snip>
> >>
> >> 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.
Thank you. I think we are fine to go ahead with merging this patch.

> 
> >
> 
> --
> Regards,
> Vladimir

  reply index

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-07  8:15 [dpdk-dev] [PATCH 0/2] LPM changes Ruifeng Wang
2020-09-07  8:15 ` [dpdk-dev] [PATCH 1/2] lpm: fix free of data structure Ruifeng Wang
2020-09-15 15:55   ` Bruce Richardson
2020-09-15 16:25   ` Medvedkin, Vladimir
2020-09-07  8:15 ` [dpdk-dev] [PATCH 2/2] lpm: hide internal data Ruifeng Wang
2020-09-15 16:02   ` Bruce Richardson
2020-09-15 16:28     ` Medvedkin, Vladimir
2020-09-16  3:17       ` Ruifeng Wang
2020-09-30  8:45         ` Kevin Traynor
2020-10-09  6:54           ` Ruifeng Wang
2020-10-13 13:53             ` Kevin Traynor
2020-10-13 14:58               ` Michel Machado
2020-10-13 15:41                 ` Medvedkin, Vladimir
2020-10-13 17:46                   ` Michel Machado
2020-10-13 19:06                     ` Medvedkin, Vladimir
2020-10-13 19:48                       ` Michel Machado
2020-10-14 13:10                         ` Medvedkin, Vladimir
2020-10-14 23:57                           ` Honnappa Nagarahalli
2020-10-15 13:39                             ` Michel Machado
2020-10-15 17:38                               ` Honnappa Nagarahalli
2020-10-15 19:30                                 ` Medvedkin, Vladimir
2020-10-15 22:54                                   ` Honnappa Nagarahalli [this message]
2020-10-16 11:39                                     ` Kevin Traynor
2020-10-16 13:55                                       ` Michel Machado
2020-10-19 14:53                                     ` David Marchand
2020-10-20 14:22                                       ` Thomas Monjalon
2020-10-20 14:32                                         ` Medvedkin, Vladimir
2020-10-19 17:53             ` Honnappa Nagarahalli
2020-09-15 14:41 ` [dpdk-dev] [PATCH 0/2] LPM changes David Marchand
2020-10-19 13:37   ` Kevin Traynor
2020-10-21  3:02 ` [dpdk-dev] [PATCH v2 " Ruifeng Wang
2020-10-21  3:02   ` [dpdk-dev] [PATCH v2 1/2] lpm: fix free of data structure Ruifeng Wang
2020-10-21  3:02   ` [dpdk-dev] [PATCH v2 2/2] lpm: hide internal data Ruifeng Wang
2020-10-21  7:58     ` Thomas Monjalon
2020-10-21  8:15       ` Ruifeng Wang
2020-10-22 15:14     ` David Marchand
2020-10-23  6:13       ` Ruifeng Wang
2020-10-23 16:08         ` Medvedkin, Vladimir
2020-10-23  9:38 ` [dpdk-dev] [PATCH v3 0/2] LPM changes David Marchand
2020-10-23  9:38   ` [dpdk-dev] [PATCH v3 1/2] lpm: fix free of data structure David Marchand
2020-10-23  9:38   ` [dpdk-dev] [PATCH v3 2/2] lpm: hide internal data David Marchand
2020-10-26  8:26   ` [dpdk-dev] [PATCH v3 0/2] LPM changes David Marchand

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBAPR08MB581430BDB3D55AF6F611C0B498020@DBAPR08MB5814.eurprd08.prod.outlook.com \
    --to=honnappa.nagarahalli@arm.com \
    --cc=Ruifeng.Wang@arm.com \
    --cc=andre@digirati.com.br \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=doucette@bu.edu \
    --cc=ktraynor@redhat.com \
    --cc=mengxiang0811@gmail.com \
    --cc=michel@digirati.com.br \
    --cc=nd@arm.com \
    --cc=techboard@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=vladimir.medvedkin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox