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

Medvedkin, Vladimir vladimir.medvedkin at intel.com
Fri Oct 23 18:08:21 CEST 2020


Hello,

On 23/10/2020 07:13, Ruifeng Wang wrote:
> 
>> -----Original Message-----
>> From: David Marchand <david.marchand at redhat.com>
>> Sent: Thursday, October 22, 2020 11:14 PM
>> To: Ruifeng Wang <Ruifeng.Wang at arm.com>
>> Cc: Bruce Richardson <bruce.richardson at intel.com>; Vladimir Medvedkin
>> <vladimir.medvedkin at intel.com>; dev <dev at dpdk.org>; Honnappa
>> Nagarahalli <Honnappa.Nagarahalli at arm.com>; nd <nd at arm.com>; Kevin
>> Traynor <ktraynor at redhat.com>; thomas at 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 at 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.

Agree, these structs are control plane related and could be moved from 
public struct.

> 
>> - 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.
> 

Also checked with testfib app with "-c -a" args, FIB and LPM lookup 
returns same values.

> Thanks.
> /Ruifeng
>>
>> --
>> David Marchand
> 

-- 
Regards,
Vladimir


More information about the dev mailing list