Re: [dpdk-dev] [PATCH v2 2/2] lpm: hide internal data

[Ruifeng Wang <Ruifeng.Wang@arm.com>]

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