> -----Original Message-----
> From: David Marchand <david.march...@redhat.com>
> Sent: Thursday, October 22, 2020 11:14 PM
> To: Ruifeng Wang <ruifeng.w...@arm.com>
> Cc: Bruce Richardson <bruce.richard...@intel.com>; Vladimir Medvedkin
> <vladimir.medved...@intel.com>; dev <dev@dpdk.org>; Honnappa
> Nagarahalli <honnappa.nagaraha...@arm.com>; nd <n...@arm.com>; Kevin
> Traynor <ktray...@redhat.com>; tho...@monjalon.net
> Subject: Re: [PATCH v2 2/2] lpm: hide internal data
>
> On Wed, Oct 21, 2020 at 5:02 AM Ruifeng Wang <ruifeng.w...@arm.com>
> 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
