[dpdk-dev,v2] lpm: remove redundant check when adding lpm rule
Commit Message
When a rule with depth > 24 is added into an existing
rule with depth <=24, a new tbl8 is allocated, the existing
rule first fulfill whole new tbl8, so the filed vaild of
each entry in this tbl8 is always true and depth of each
entry is always < 24 before adding new rule with depth > 24.
Signed-off-by: Wei Dai <wei.dai@intel.com>
---
lib/librte_lpm/rte_lpm.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
Comments
On Tue, Aug 02, 2016 at 10:09:25AM +0800, Wei Dai wrote:
> When a rule with depth > 24 is added into an existing
> rule with depth <=24, a new tbl8 is allocated, the existing
> rule first fulfill whole new tbl8, so the filed vaild of
typo "valid"
> each entry in this tbl8 is always true and depth of each
> entry is always < 24 before adding new rule with depth > 24.
>
> Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Having to make this change twice shows up the fact that we are still carrying
around some version changes for older releases. Given that we are now past the
16.07 release, the old code can probably be removed. Any volunteers to maybe
do up a patch for that.
Regards,
/Bruce
2016-08-02 17:04, Bruce Richardson:
> Having to make this change twice shows up the fact that we are still carrying
> around some version changes for older releases. Given that we are now past the
> 16.07 release, the old code can probably be removed. Any volunteers to maybe
> do up a patch for that.
The first step would be to announce an ABI breakage.
Do you plan to do other breaking changes? We may try to group them.
On Tue, Aug 02, 2016 at 11:36:41PM +0200, Thomas Monjalon wrote:
> 2016-08-02 17:04, Bruce Richardson:
> > Having to make this change twice shows up the fact that we are still carrying
> > around some version changes for older releases. Given that we are now past the
> > 16.07 release, the old code can probably be removed. Any volunteers to maybe
> > do up a patch for that.
>
> The first step would be to announce an ABI breakage.
> Do you plan to do other breaking changes? We may try to group them.
>
Why does an ABI breakage need to be announced? The code has been in place for
some time, and was called out as an API change in the release notes for 16.04.
Any app compiled against either 16.04 or 16.07 release will work fine once the
code is removed. Any app compiled against an earlier version:
a) is not guaranteed to work, because we only guarantee 1-version
compatibility right now
b) in practice almost certainly won't work with 16.11 anyway, due to
ABI changes in other areas.
Therefore, I would view an ABI announcement as rather pointless.
/Bruce
@@ -931,32 +931,27 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t ip_masked, uint8_t depth,
/* Populate new tbl8 with tbl24 value. */
for (i = tbl8_group_start; i < tbl8_group_end; i++) {
lpm->tbl8[i].valid = VALID;
lpm->tbl8[i].depth = lpm->tbl24[tbl24_index].depth;
lpm->tbl8[i].next_hop =
lpm->tbl24[tbl24_index].next_hop;
}
tbl8_index = tbl8_group_start + (ip_masked & 0xFF);
/* Insert new rule into the tbl8 entry. */
for (i = tbl8_index; i < tbl8_index + tbl8_range; i++) {
- if (!lpm->tbl8[i].valid ||
- lpm->tbl8[i].depth <= depth) {
- lpm->tbl8[i].valid = VALID;
- lpm->tbl8[i].depth = depth;
- lpm->tbl8[i].next_hop = next_hop;
-
- continue;
- }
+ lpm->tbl8[i].valid = VALID;
+ lpm->tbl8[i].depth = depth;
+ lpm->tbl8[i].next_hop = next_hop;
}
/*
* Update tbl24 entry to point to new tbl8 entry. Note: The
* ext_flag and tbl8_index need to be updated simultaneously,
* so assign whole structure in one go.
*/
struct rte_lpm_tbl_entry_v20 new_tbl24_entry = {
{ .group_idx = (uint8_t)tbl8_group_index, },
.valid = VALID,
.valid_group = 1,
@@ -1062,32 +1057,27 @@ add_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth,
/* Populate new tbl8 with tbl24 value. */
for (i = tbl8_group_start; i < tbl8_group_end; i++) {
lpm->tbl8[i].valid = VALID;
lpm->tbl8[i].depth = lpm->tbl24[tbl24_index].depth;
lpm->tbl8[i].next_hop =
lpm->tbl24[tbl24_index].next_hop;
}
tbl8_index = tbl8_group_start + (ip_masked & 0xFF);
/* Insert new rule into the tbl8 entry. */
for (i = tbl8_index; i < tbl8_index + tbl8_range; i++) {
- if (!lpm->tbl8[i].valid ||
- lpm->tbl8[i].depth <= depth) {
- lpm->tbl8[i].valid = VALID;
- lpm->tbl8[i].depth = depth;
- lpm->tbl8[i].next_hop = next_hop;
-
- continue;
- }
+ lpm->tbl8[i].valid = VALID;
+ lpm->tbl8[i].depth = depth;
+ lpm->tbl8[i].next_hop = next_hop;
}
/*
* Update tbl24 entry to point to new tbl8 entry. Note: The
* ext_flag and tbl8_index need to be updated simultaneously,
* so assign whole structure in one go.
*/
struct rte_lpm_tbl_entry new_tbl24_entry = {
.group_idx = (uint8_t)tbl8_group_index,
.valid = VALID,
.valid_group = 1,