From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8CD1CA00E6 for ; Fri, 12 Jul 2019 05:10:14 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E9BFE1B954; Fri, 12 Jul 2019 05:10:13 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id B89EA1B952 for ; Fri, 12 Jul 2019 05:10:12 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 500192B; Thu, 11 Jul 2019 20:10:12 -0700 (PDT) Received: from net-arm-c2400-02.shanghai.arm.com (net-arm-c2400-02.shanghai.arm.com [10.169.40.42]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id BB19B3F246; Thu, 11 Jul 2019 20:10:10 -0700 (PDT) From: Ruifeng Wang To: vladimir.medvedkin@intel.com, bruce.richardson@intel.com Cc: dev@dpdk.org, honnappa.nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com, Ruifeng Wang Date: Fri, 12 Jul 2019 11:09:19 +0800 Message-Id: <20190712030923.37832-3-ruifeng.wang@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190712030923.37832-1-ruifeng.wang@arm.com> References: <20190605055451.30473-1-ruifeng.wang@arm.com> <20190712030923.37832-1-ruifeng.wang@arm.com> Subject: [dpdk-dev] [PATCH v5 2/6] lib/lpm: memory orderings to avoid race conditions for v1604 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" When a tbl8 group is getting attached to a tbl24 entry, lookup might fail even though the entry is configured in the table. For ex: consider a LPM table configured with 10.10.10.1/24. When a new entry 10.10.10.32/28 is being added, a new tbl8 group is allocated and tbl24 entry is changed to point to the tbl8 group. If the tbl24 entry is written without the tbl8 group entries updated, a lookup on 10.10.10.9 will return failure. Correct memory orderings are required to ensure that the store to tbl24 does not happen before the stores to tbl8 group entries complete. Signed-off-by: Honnappa Nagarahalli Signed-off-by: Ruifeng Wang Reviewed-by: Gavin Hu --- lib/librte_lpm/rte_lpm.c | 32 +++++++++++++++++++++++++------- lib/librte_lpm/rte_lpm.h | 4 ++++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index 18efa496c..396ad94e2 100644 --- a/lib/librte_lpm/rte_lpm.c +++ b/lib/librte_lpm/rte_lpm.c @@ -806,7 +806,8 @@ add_depth_small_v1604(struct rte_lpm *lpm, uint32_t ip, uint8_t depth, /* Setting tbl24 entry in one go to avoid race * conditions */ - lpm->tbl24[i] = new_tbl24_entry; + __atomic_store(&lpm->tbl24[i], &new_tbl24_entry, + __ATOMIC_RELEASE); continue; } @@ -1017,7 +1018,11 @@ add_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth, .depth = 0, }; - lpm->tbl24[tbl24_index] = new_tbl24_entry; + /* The tbl24 entry must be written only after the + * tbl8 entries are written. + */ + __atomic_store(&lpm->tbl24[tbl24_index], &new_tbl24_entry, + __ATOMIC_RELEASE); } /* If valid entry but not extended calculate the index into Table8. */ else if (lpm->tbl24[tbl24_index].valid_group == 0) { @@ -1063,7 +1068,11 @@ add_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, uint8_t depth, .depth = 0, }; - lpm->tbl24[tbl24_index] = new_tbl24_entry; + /* The tbl24 entry must be written only after the + * tbl8 entries are written. + */ + __atomic_store(&lpm->tbl24[tbl24_index], &new_tbl24_entry, + __ATOMIC_RELEASE); } else { /* * If it is valid, extended entry calculate the index into tbl8. @@ -1391,6 +1400,7 @@ delete_depth_small_v1604(struct rte_lpm *lpm, uint32_t ip_masked, /* Calculate the range and index into Table24. */ tbl24_range = depth_to_range(depth); tbl24_index = (ip_masked >> 8); + struct rte_lpm_tbl_entry zero_tbl24_entry = {0}; /* * Firstly check the sub_rule_index. A -1 indicates no replacement rule @@ -1405,7 +1415,8 @@ delete_depth_small_v1604(struct rte_lpm *lpm, uint32_t ip_masked, if (lpm->tbl24[i].valid_group == 0 && lpm->tbl24[i].depth <= depth) { - lpm->tbl24[i].valid = INVALID; + __atomic_store(&lpm->tbl24[i], + &zero_tbl24_entry, __ATOMIC_RELEASE); } else if (lpm->tbl24[i].valid_group == 1) { /* * If TBL24 entry is extended, then there has @@ -1450,7 +1461,8 @@ delete_depth_small_v1604(struct rte_lpm *lpm, uint32_t ip_masked, if (lpm->tbl24[i].valid_group == 0 && lpm->tbl24[i].depth <= depth) { - lpm->tbl24[i] = new_tbl24_entry; + __atomic_store(&lpm->tbl24[i], &new_tbl24_entry, + __ATOMIC_RELEASE); } else if (lpm->tbl24[i].valid_group == 1) { /* * If TBL24 entry is extended, then there has @@ -1713,8 +1725,11 @@ delete_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, tbl8_recycle_index = tbl8_recycle_check_v1604(lpm->tbl8, tbl8_group_start); if (tbl8_recycle_index == -EINVAL) { - /* Set tbl24 before freeing tbl8 to avoid race condition. */ + /* Set tbl24 before freeing tbl8 to avoid race condition. + * Prevent the free of the tbl8 group from hoisting. + */ lpm->tbl24[tbl24_index].valid = 0; + __atomic_thread_fence(__ATOMIC_RELEASE); tbl8_free_v1604(lpm->tbl8, tbl8_group_start); } else if (tbl8_recycle_index > -1) { /* Update tbl24 entry. */ @@ -1725,8 +1740,11 @@ delete_depth_big_v1604(struct rte_lpm *lpm, uint32_t ip_masked, .depth = lpm->tbl8[tbl8_recycle_index].depth, }; - /* Set tbl24 before freeing tbl8 to avoid race condition. */ + /* Set tbl24 before freeing tbl8 to avoid race condition. + * Prevent the free of the tbl8 group from hoisting. + */ lpm->tbl24[tbl24_index] = new_tbl24_entry; + __atomic_thread_fence(__ATOMIC_RELEASE); tbl8_free_v1604(lpm->tbl8, tbl8_group_start); } #undef group_idx diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index b886f54b4..6f5704c5c 100644 --- a/lib/librte_lpm/rte_lpm.h +++ b/lib/librte_lpm/rte_lpm.h @@ -354,6 +354,10 @@ rte_lpm_lookup(struct rte_lpm *lpm, uint32_t ip, uint32_t *next_hop) ptbl = (const uint32_t *)(&lpm->tbl24[tbl24_index]); tbl_entry = *ptbl; + /* Memory ordering is not required in lookup. Because dataflow + * dependency exists, compiler or HW won't be able to re-order + * the operations. + */ /* Copy tbl8 entry (only if needed) */ if (unlikely((tbl_entry & RTE_LPM_VALID_EXT_ENTRY_BITMASK) == RTE_LPM_VALID_EXT_ENTRY_BITMASK)) { -- 2.17.1