From: mablexidana <mablexidana@163.com>
To: "Bruce Richardson" <bruce.richardson@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] fix lpm bugs
Date: Thu, 22 Oct 2015 10:15:16 +0800 (CST) [thread overview]
Message-ID: <313921ca.4b27.1508d543038.Coremail.mablexidana@163.com> (raw)
In-Reply-To: <20151021110749.GD16140@bricha3-MOBL3>
[-- 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
next prev parent reply other threads:[~2015-10-22 2:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-21 9:54 mablexidana
2015-10-21 11:07 ` Bruce Richardson
2015-10-22 2:15 ` mablexidana [this message]
2015-10-23 4:40 ` mablexidana
[not found] ` <29ae0ca8.e6db.15093aafa12.Coremail.mablexidana@163.com>
2015-10-23 9:08 ` Mcnamara, John
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=313921ca.4b27.1508d543038.Coremail.mablexidana@163.com \
--to=mablexidana@163.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).