[v4] lpm: skip table entries update if rules found

Message ID 20200420024850.21481-1-zhouyates@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v4] lpm: skip table entries update if rules found |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS

Commit Message

Matt April 20, 2020, 2:48 a.m. UTC
  Table entries do not need to be updated if the same rules can be found.

Signed-off-by: Yangchao Zhou <zhouyates@gmail.com>
---
v1-v2: Skip updating when the next hop is the same as the current one
v2-v4: Coding style fix
---
 lib/librte_lpm/rte_lpm.c | 9 +++++++++
 1 file changed, 9 insertions(+)
  

Comments

Vladimir Medvedkin April 21, 2020, 3:44 p.m. UTC | #1
Hi Yangchao,

Thanks for the patch.
This might be useful for control plane implementations that don't track 
inserted routes.
I have just a one nit inlined below. Also, could you do the same for lpm6?

P.S. Please have a look at rte_fib library, there are more optimizations 
compared to lpm library.

Apart from that,
Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>

On 20/04/2020 03:48, Yangchao Zhou wrote:
> Table entries do not need to be updated if the same rules can be found.
>
> Signed-off-by: Yangchao Zhou <zhouyates@gmail.com>
> ---
> v1-v2: Skip updating when the next hop is the same as the current one
> v2-v4: Coding style fix
> ---
>   lib/librte_lpm/rte_lpm.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
> index 268756419..b8a90e385 100644
> --- a/lib/librte_lpm/rte_lpm.c
> +++ b/lib/librte_lpm/rte_lpm.c
> @@ -285,6 +285,9 @@ rule_add(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth,
>   
>   			/* If rule already exists update its next_hop and return. */
>   			if (lpm->rules_tbl[rule_index].ip == ip_masked) {
> +
> +				if (lpm->rules_tbl[rule_index].next_hop == next_hop)


Line over 80 characters, please split it by two.

> +					return -EEXIST;
>   				lpm->rules_tbl[rule_index].next_hop = next_hop;
>   
>   				return rule_index;
> @@ -674,6 +677,12 @@ rte_lpm_add(struct rte_lpm *lpm, uint32_t ip, uint8_t depth,
>   	/* Add the rule to the rule table. */
>   	rule_index = rule_add(lpm, ip_masked, depth, next_hop);
>   
> +	/* Skip table entries update if The rule is the same as
> +	 * the rule in the rules table.
> +	 */
> +	if (rule_index == -EEXIST)
> +		return 0;
> +
>   	/* If the is no space available for new rule return error. */
>   	if (rule_index < 0) {
>   		return rule_index;
  
Thomas Monjalon April 24, 2020, 5:17 p.m. UTC | #2
21/04/2020 17:44, Medvedkin, Vladimir:
> Hi Yangchao,
> 
> Thanks for the patch.
> This might be useful for control plane implementations that don't track 
> inserted routes.
> I have just a one nit inlined below. Also, could you do the same for lpm6?
> 
> P.S. Please have a look at rte_fib library, there are more optimizations 
> compared to lpm library.
> 
> Apart from that,
> Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
[...]
> >   			/* If rule already exists update its next_hop and return. */

Reduced length of above comment,

> >   			if (lpm->rules_tbl[rule_index].ip == ip_masked) {
> > +
> > +				if (lpm->rules_tbl[rule_index].next_hop == next_hop)
> 
> 
> Line over 80 characters, please split it by two.

and split above line.

Applied with requested changes, thanks.
  

Patch

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 268756419..b8a90e385 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -285,6 +285,9 @@  rule_add(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth,
 
 			/* If rule already exists update its next_hop and return. */
 			if (lpm->rules_tbl[rule_index].ip == ip_masked) {
+
+				if (lpm->rules_tbl[rule_index].next_hop == next_hop)
+					return -EEXIST;
 				lpm->rules_tbl[rule_index].next_hop = next_hop;
 
 				return rule_index;
@@ -674,6 +677,12 @@  rte_lpm_add(struct rte_lpm *lpm, uint32_t ip, uint8_t depth,
 	/* Add the rule to the rule table. */
 	rule_index = rule_add(lpm, ip_masked, depth, next_hop);
 
+	/* Skip table entries update if The rule is the same as
+	 * the rule in the rules table.
+	 */
+	if (rule_index == -EEXIST)
+		return 0;
+
 	/* If the is no space available for new rule return error. */
 	if (rule_index < 0) {
 		return rule_index;