[v5,6/6] lib/lpm: data update optimization for v20

Message ID 20190712030923.37832-7-ruifeng.wang@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series LPM4 memory ordering changes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Ruifeng Wang July 12, 2019, 3:09 a.m. UTC
  The table entries were updated field by field. There were two issues:
1. bitwise operations are read-modify-write sequences and not atomic,
   nor efficient.
2. the above non-atomic operations causes entries out of synchronization
   and inconsistency.
This patch combines the fields into a one-go 32bit entry update to avoid
inconsistency and as a bonus save CPU cycles.

Suggested-by: Gavin Hu <gavin.hu@arm.com>
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
---
 lib/librte_lpm/rte_lpm.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)
  

Comments

Honnappa Nagarahalli July 12, 2019, 8:09 p.m. UTC | #1
Similar to 5/6, this patch can also be merged into 4/6.

> -----Original Message-----
> From: Ruifeng Wang <ruifeng.wang@arm.com>
> Sent: Thursday, July 11, 2019 10:09 PM
> To: vladimir.medvedkin@intel.com; bruce.richardson@intel.com
> Cc: dev@dpdk.org; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>; nd <nd@arm.com>;
> Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>
> Subject: [PATCH v5 6/6] lib/lpm: data update optimization for v20
> 
> The table entries were updated field by field. There were two issues:
> 1. bitwise operations are read-modify-write sequences and not atomic,
>    nor efficient.
> 2. the above non-atomic operations causes entries out of synchronization
>    and inconsistency.
> This patch combines the fields into a one-go 32bit entry update to avoid
> inconsistency and as a bonus save CPU cycles.
> 
> Suggested-by: Gavin Hu <gavin.hu@arm.com>
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Gavin Hu <gavin.hu@arm.com>
> ---
>  lib/librte_lpm/rte_lpm.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index
> d86248713..95e2f75aa 100644
> --- a/lib/librte_lpm/rte_lpm.c
> +++ b/lib/librte_lpm/rte_lpm.c
> @@ -906,9 +906,14 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm,
> uint32_t ip_masked, uint8_t depth,
> 
>  		/* Set tbl8 entry. */
>  		for (i = tbl8_index; i < (tbl8_index + tbl8_range); i++) {
> -			lpm->tbl8[i].depth = depth;
> -			lpm->tbl8[i].next_hop = next_hop;
> -			lpm->tbl8[i].valid = VALID;
> +			struct rte_lpm_tbl_entry_v20 new_tbl8_entry = {
> +				.valid = VALID,
> +				.depth = depth,
> +				.valid_group = lpm->tbl8[i].valid_group,
> +			};
> +			new_tbl8_entry.next_hop = next_hop;
> +			__atomic_store(&lpm->tbl8[i], &new_tbl8_entry,
> +					__ATOMIC_RELAXED);
>  		}
> 
>  		/*
> @@ -943,19 +948,29 @@ 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;
> +			struct rte_lpm_tbl_entry_v20 new_tbl8_entry = {
> +				.valid = VALID,
> +				.depth = lpm->tbl24[tbl24_index].depth,
> +				.valid_group = lpm->tbl8[i].valid_group,
> +			};
> +			new_tbl8_entry.next_hop =
> +				lpm->tbl24[tbl24_index].next_hop;
> +			__atomic_store(&lpm->tbl8[i], &new_tbl8_entry,
> +					__ATOMIC_RELAXED);
>  		}
> 
>  		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++) {
> -			lpm->tbl8[i].valid = VALID;
> -			lpm->tbl8[i].depth = depth;
> -			lpm->tbl8[i].next_hop = next_hop;
> +			struct rte_lpm_tbl_entry_v20 new_tbl8_entry = {
> +				.valid = VALID,
> +				.depth = depth,
> +				.valid_group = lpm->tbl8[i].valid_group,
> +			};
> +			new_tbl8_entry.next_hop = next_hop;
> +			__atomic_store(&lpm->tbl8[i], &new_tbl8_entry,
> +					__ATOMIC_RELAXED);
>  		}
> 
>  		/*
> --
> 2.17.1
  

Patch

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index d86248713..95e2f75aa 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -906,9 +906,14 @@  add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t ip_masked, uint8_t depth,
 
 		/* Set tbl8 entry. */
 		for (i = tbl8_index; i < (tbl8_index + tbl8_range); i++) {
-			lpm->tbl8[i].depth = depth;
-			lpm->tbl8[i].next_hop = next_hop;
-			lpm->tbl8[i].valid = VALID;
+			struct rte_lpm_tbl_entry_v20 new_tbl8_entry = {
+				.valid = VALID,
+				.depth = depth,
+				.valid_group = lpm->tbl8[i].valid_group,
+			};
+			new_tbl8_entry.next_hop = next_hop;
+			__atomic_store(&lpm->tbl8[i], &new_tbl8_entry,
+					__ATOMIC_RELAXED);
 		}
 
 		/*
@@ -943,19 +948,29 @@  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;
+			struct rte_lpm_tbl_entry_v20 new_tbl8_entry = {
+				.valid = VALID,
+				.depth = lpm->tbl24[tbl24_index].depth,
+				.valid_group = lpm->tbl8[i].valid_group,
+			};
+			new_tbl8_entry.next_hop =
+				lpm->tbl24[tbl24_index].next_hop;
+			__atomic_store(&lpm->tbl8[i], &new_tbl8_entry,
+					__ATOMIC_RELAXED);
 		}
 
 		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++) {
-			lpm->tbl8[i].valid = VALID;
-			lpm->tbl8[i].depth = depth;
-			lpm->tbl8[i].next_hop = next_hop;
+			struct rte_lpm_tbl_entry_v20 new_tbl8_entry = {
+				.valid = VALID,
+				.depth = depth,
+				.valid_group = lpm->tbl8[i].valid_group,
+			};
+			new_tbl8_entry.next_hop = next_hop;
+			__atomic_store(&lpm->tbl8[i], &new_tbl8_entry,
+					__ATOMIC_RELAXED);
 		}
 
 		/*