Re: [dpdk-dev] [PATCH] fix lpm bugs

[mablexidana <mablexidana@163.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GBK, Size: 4098 bytes --]

hi:
    Fixes: 25e4f515fe63 ("fix lpm bugs")




   the random test of lpm , multiple delete and add ip address, it do not recover the last right ip address.
  eg1: 
   add a lot of routes:
 rule id : 1, ip : 16.32.0.0/19, next_hop : 62, 
rule id : 2, ip : 16.32.28.0/22, next_hop : 97,
rule id :  28, ip:16.32.0.0/21, next_hop :36
.....
when you delete rule id 3, then lookup 16.32.0.150, the return is 16.32.28.0/22,but not 16.32.0.0/19. this is because in delete_depth_small function, when lpm->tbl24[i].ext_entry == 0 and lpm->tbl24[i].depth > depth, it will run into the  tbl8 process.then the next_hop will be doing as tbl8_gindex, and the lpm->tbl8[j] data is being wrong processed.
fix: + else if (lpm->tbl24[i].ext_entry == 1) {


eg2:
when add ,delete and add again, it will also has problem.
in delete_depth_small function, the valid_group of new struct rte_lpm_tbl8_entry is INVALID, so when process  lpm->tbl8[j] = new_tbl8_entry, the valid_group is covered. and when just add a route depth > 24,  and  alloc a tbl8 index, then the tbl8_alloc will return it as new index, then the data is being wrong rewrite.
fix:+ .valid_group = VALID,




thanks.    I will provide the testing program later .




regards


yuerxin
   













At 2015-10-21 19:07:49, "Bruce Richardson" <bruce.richardson@intel.com> wrote:
>On Wed, Oct 21, 2015 at 05:54:13PM +0800, mablexidana wrote:
>> hi:
>>     We test some lpm cases and find some bugs, below is how to fix it. thanks :)
>
>Hi,
>
>thanks for the patch. Could you perhaps provide a description of how to reproduce
>the bug (or bugs you are fixing), so that we can reproduce them and verify the
>fix. (A unit test added to the existing lpm unit tests for this would be the 
>best solution.)
>For the patch itself, the commit message should also describe the bug, and
>how the patch fixes it. It's also good to include a one-line "Fixes:" line
>in the comment - generated by using the git alias "fixline" added as:
>	fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'
>
>Regards,
>/Bruce
>
>> ---
>>  lib/librte_lpm/rte_lpm.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> 
>> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
>> index 163ba3c..b5199ff 100644
>> --- a/lib/librte_lpm/rte_lpm.c
>> +++ b/lib/librte_lpm/rte_lpm.c
>> @@ -735,7 +735,7 @@ delete_depth_small(struct rte_lpm *lpm, uint32_t ip_masked,
>>                                         lpm->tbl24[i].depth <= depth ) {
>>                                 lpm->tbl24[i].valid = INVALID;
>>                         }
>> -                       else {
>> +                       else if (lpm->tbl24[i].ext_entry == 1){
>>                                 /*
>>                                  * If TBL24 entry is extended, then there has
>>                                  * to be a rule with depth >= 25 in the
>> @@ -770,6 +770,7 @@ delete_depth_small(struct rte_lpm *lpm, uint32_t ip_masked,
>> 
>> 
>>                 struct rte_lpm_tbl8_entry new_tbl8_entry = {
>>                         .valid = VALID,
>> +                       .valid_group = VALID,
>>                         .depth = sub_rule_depth,
>>                         .next_hop = lpm->rules_tbl
>>                         [sub_rule_index].next_hop,
>> @@ -781,7 +782,7 @@ delete_depth_small(struct rte_lpm *lpm, uint32_t ip_masked,
>>                                         lpm->tbl24[i].depth <= depth ) {
>>                                 lpm->tbl24[i] = new_tbl24_entry;
>>                         }
>> -                       else {
>> +                       else  if (lpm->tbl24[i].ext_entry == 1) {
>>                                 /*
>>                                  * If TBL24 entry is extended, then there has
>>                                  * to be a rule with depth >= 25 in the
>> --
>> 1.8.5.2 (Apple Git-48)
\x16º&Ž&§}é൩âž×¥r‰“†ãœ·m´ãn÷Óm5å\x17­º¹ÏjØc‰©ßzx-jx§µé\¢d^qè¯y×ë¢i k]bž×¥r‰¦­uŠ{^•Ê&×ݹ睽ݼ¥Ù(®\x03è²×âÇ\b­„DŒLø÷ÑB×Ýúéú+uëÝ¥Ù(®\x04á»mŽrݴם8Û½½ûM´Ð!\x12M\x17œz+Þuúè™ù¬š\x06´ÓgæŠ{^•Ê&×M¹ßn6鼟šÉ k]6~h§µé\¢l"¶\x11\x1213öÔç-ÛMy×Ý»ßM;ÓEÄÆÒ袝u\Šèœú+´\x05D