[dpdk-dev] [PATCH] lpm: remove redundant check when adding lpm rule

[dpdk-dev] [PATCH] lpm: remove redundant check when adding lpm rule

[Wei Dai]

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 | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 24fec4b..5bdc23f 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -1071,14 +1071,9 @@ add_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth,
 
 		/* 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;
 		}
 
 		/*
-- 
2.5.5

[dpdk-dev] [PATCH v2] lpm: remove redundant check when adding lpm rule

[Wei Dai]

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(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 24fec4b..ec67765 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -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,
-- 
2.5.5

Re: [dpdk-dev] [PATCH v2] lpm: remove redundant check when adding lpm rule

[Bruce Richardson]

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

Re: [dpdk-dev] [PATCH v2] lpm: remove redundant check when adding lpm rule

[Thomas Monjalon]

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.

[dpdk-dev] [PATCH v3 3/3] lpm: remove redundant check when adding lpm rule

[Wei Dai]

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 valid of
each entry in this tbl8 is always true and depth of each
entry is always <= 24 before adding the 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(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 24fec4b..ec67765 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -940,14 +940,9 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t ip_masked, uint8_t depth,
 
 		/* 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;
 		}
 
 		/*
@@ -1071,14 +1066,9 @@ add_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth,
 
 		/* 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;
 		}
 
 		/*
-- 
2.5.5

Re: [dpdk-dev] [PATCH v3 3/3] lpm: remove redundant check when adding lpm rule

[Dai, Wei]

This patch is same as patch 15064 which has been acked by Bruce Richardson.
Resending it as 2/3 of a series of patches for same rte_lpm.c .

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, August 3, 2016 3:04 PM
> To: dev@dpdk.org
> Cc: Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v3 3/3] lpm: remove redundant check when adding lpm rule
> 
> 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
> valid of each entry in this tbl8 is always true and depth of each entry is always
> <= 24 before adding the 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(-)
> 
> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index
> 24fec4b..ec67765 100644
> --- a/lib/librte_lpm/rte_lpm.c
> +++ b/lib/librte_lpm/rte_lpm.c
> @@ -940,14 +940,9 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm,
> uint32_t ip_masked, uint8_t depth,
> 
>  		/* 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;
>  		}
> 
>  		/*
> @@ -1071,14 +1066,9 @@ add_depth_big_v1604(struct rte_lpm *lpm,
> uint32_t ip_masked, uint8_t depth,
> 
>  		/* 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;
>  		}
> 
>  		/*
> --
> 2.5.5

Re: [dpdk-dev] [PATCH v3 3/3] lpm: remove redundant check when adding lpm rule

[Dai, Wei]

This patch is same as patch 15094 which has been acked by Bruce Richardson.
Only minor typo is corrected according to Bruce's feedback.

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, August 3, 2016 3:04 PM
> To: dev@dpdk.org
> Cc: Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v3 3/3] lpm: remove redundant check when adding lpm rule
> 
> 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
> valid of each entry in this tbl8 is always true and depth of each entry is always
> <= 24 before adding the 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(-)
> 
> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index
> 24fec4b..ec67765 100644
> --- a/lib/librte_lpm/rte_lpm.c
> +++ b/lib/librte_lpm/rte_lpm.c
> @@ -940,14 +940,9 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm,
> uint32_t ip_masked, uint8_t depth,
> 
>  		/* 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;
>  		}
> 
>  		/*
> @@ -1071,14 +1066,9 @@ add_depth_big_v1604(struct rte_lpm *lpm,
> uint32_t ip_masked, uint8_t depth,
> 
>  		/* 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;
>  		}
> 
>  		/*
> --
> 2.5.5

Re: [dpdk-dev] [PATCH v2] lpm: remove redundant check when adding lpm rule

[Bruce Richardson]

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

[dpdk-dev] [PATCH v4 3/3] lpm: remove redundant check when adding lpm rule

[Wei Dai]

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 valid of
each entry in this tbl8 is always true and depth of each
entry is always <= 24 before adding the new rule with depth > 24.

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_lpm/rte_lpm.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 24fec4b..ec67765 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -940,14 +940,9 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t ip_masked, uint8_t depth,
 
 		/* 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;
 		}
 
 		/*
@@ -1071,14 +1066,9 @@ add_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth,
 
 		/* 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;
 		}
 
 		/*
-- 
2.5.5