> -----Original Message----- > From: David Marchand <[email protected]> > Sent: Thursday, October 22, 2020 11:14 PM > To: Ruifeng Wang <[email protected]> > Cc: Bruce Richardson <[email protected]>; Vladimir Medvedkin > <[email protected]>; dev <[email protected]>; Honnappa > Nagarahalli <[email protected]>; nd <[email protected]>; Kevin > Traynor <[email protected]>; [email protected] > Subject: Re: [PATCH v2 2/2] lpm: hide internal data > > On Wed, Oct 21, 2020 at 5:02 AM Ruifeng Wang <[email protected]> > 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.
> - 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. Thanks. /Ruifeng > > -- > David Marchand
