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 9F99EA0487 for ; Fri, 5 Jul 2019 20:20:42 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9B7B51BE62; Fri, 5 Jul 2019 20:20:41 +0200 (CEST) Received: from mail-qk1-f193.google.com (mail-qk1-f193.google.com [209.85.222.193]) by dpdk.org (Postfix) with ESMTP id 6172E1BE5E for ; Fri, 5 Jul 2019 20:20:39 +0200 (CEST) Received: by mail-qk1-f193.google.com with SMTP id r21so8251343qke.2 for ; Fri, 05 Jul 2019 11:20:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=UoHIbALmyrJPOTL4BHwmVMvOg/f76XiG4i6IoSNdX3Y=; b=atUtAeeWw0SdWuXhAl3ylQljNhnaFEVEgguuRbeeiHVmG9wySVRY9qIE19Cwdc5sD2 MRULVOeKuUE55f+0P/vJFRwM3E9yxtBzdh/Ou71TntuWfoNNyQjuVrIgXEPz1WDGgJVK B5IRkYJJ70338qxyD3DAJuY7zl/Hg48/gQQMyvfeqTS4nMrigi1IjHsaPXcUECIx6iTl DPJIvMlsjP0yNxwoawy6/7pOOicUd3kn92/EwHaWNH33CG0HQ6ZJR4Zt8sudXPTfNj+1 0oqI/Dy5mbjOM58k2GKwrxD9XbnDkulUHBbebR+AkLCOgZi9STklRDuMoGGF57+r6hQI hEjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=UoHIbALmyrJPOTL4BHwmVMvOg/f76XiG4i6IoSNdX3Y=; b=qcho8IFc+Ie98LL/aGyhOBrrdKgu1ipRRdz6pyt75nouWl3fgRNr0tl6fjb9Yb6pye pu5TIVJCQtEuAG1Y5BYDRQtj21ea14LquIht2/y5TvPqdzhUcke7CSJf1eUjm1LrRqbT bSI60c/vcNZUIALATHFzeHuySP5qpkouqYKVxv7wcHu7hISCdf2brklYkI0jCABQvSzJ olVnVN07i3W8D/Fr6nfyrlFW2tlEcqtg+wDwUAGBVrnoftQ6mEAD6y65cCUbja4xcg6P ulb/3DJ9nGe1Pgh6cucWMQHiqL+s8e45uDG0V9CKtrKJNYdNZ9RnPjoV46n41wkbMk8e MalA== X-Gm-Message-State: APjAAAWZ38b+t9nv0StfJrlStR75+s2+uSSNf7tgFXqlSPeVj3vVlThx 9TI62g+AWyKZF2nd4oJcSZxEw+gyU2xujSSI99Q= X-Google-Smtp-Source: APXvYqyM+WvwapXYSsNAkKXMbwvRa3Pc4ldEZCDVJ8UJdjyJv6YXIsZL13T+1J2Z8tSTyFU5XKa2z11pNlycJHWcqwc= X-Received: by 2002:a37:c449:: with SMTP id h9mr4033235qkm.187.1562350838784; Fri, 05 Jul 2019 11:20:38 -0700 (PDT) MIME-Version: 1.0 References: <20190703054441.30162-1-ruifeng.wang@arm.com> <20190703054441.30162-2-ruifeng.wang@arm.com> In-Reply-To: From: Vladimir Medvedkin Date: Fri, 5 Jul 2019 19:20:25 +0100 Message-ID: To: "Medvedkin, Vladimir" Cc: Ruifeng Wang , Bruce Richardson , dev@dpdk.org, honnappa.nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v4 2/3] lib/lpm: memory orderings to avoid race conditions for v20 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" Hi all, =D0=BF=D1=82, 5 =D0=B8=D1=8E=D0=BB. 2019 =D0=B3. =D0=B2 17:52, Medvedkin, V= ladimir < vladimir.medvedkin@intel.com>: > Hi Wang, > > On 03/07/2019 06:44, Ruifeng Wang wrote: > > 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. > > > > Besides, explicit structure alignment is used to address atomic > > operation building issue with older version clang. > > > > Suggested-by: Honnappa Nagarahalli > > Signed-off-by: Ruifeng Wang > > Reviewed-by: Honnappa Nagarahalli > > Reviewed-by: Gavin Hu > > --- > > v4: changed alignment attribute parameter > > v3: no changes > > v2: fixed clang building issue by supplying alignment attribute. > > > > lib/librte_lpm/rte_lpm.c | 32 +++++++++++++++++++++++++------- > > lib/librte_lpm/rte_lpm.h | 4 ++-- > > 2 files changed, 27 insertions(+), 9 deletions(-) > > > > diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c > > index 6ec450a08..baa6e7460 100644 > > --- a/lib/librte_lpm/rte_lpm.c > > +++ b/lib/librte_lpm/rte_lpm.c > > @@ -737,7 +737,8 @@ add_depth_small_v20(struct rte_lpm_v20 *lpm, > uint32_t ip, uint8_t depth, > > /* Setting tbl24 entry in one go to avoid race > > * conditions > > */ > > - lpm->tbl24[i] =3D new_tbl24_entry; > > + __atomic_store(&lpm->tbl24[i], &new_tbl24_entry, > > + __ATOMIC_RELEASE); > > > > continue; > > } > > @@ -892,7 +893,8 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t > ip_masked, uint8_t depth, > > .depth =3D 0, > > }; > > > > - lpm->tbl24[tbl24_index] =3D new_tbl24_entry; > > + __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 =3D=3D 0) { > > @@ -938,7 +940,8 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t > ip_masked, uint8_t depth, > > .depth =3D 0, > > }; > > > > - lpm->tbl24[tbl24_index] =3D new_tbl24_entry; > > + __atomic_store(&lpm->tbl24[tbl24_index], &new_tbl24_entry= , > > + __ATOMIC_RELEASE); > > > > } else { /* > > * If it is valid, extended entry calculate the index into > tbl8. > > @@ -1320,7 +1323,15 @@ delete_depth_small_v20(struct rte_lpm_v20 *lpm, > uint32_t ip_masked, > > > > if (lpm->tbl24[i].valid_group =3D=3D 0 && > > lpm->tbl24[i].depth <=3D depth) { > > - lpm->tbl24[i].valid =3D INVALID; > > + struct rte_lpm_tbl_entry_v20 > > + zero_tbl24_entry =3D { > > + .valid =3D INVALID, > > + .depth =3D 0, > > + .valid_group =3D 0, > > + }; > > + zero_tbl24_entry.next_hop =3D 0; > Why don't you use just "struct rte_lpm_tbl_entry_v20 zero_tbl24_entry =3D > {0} " like you do for _v1604? > Ah, sorry, just found what you mentioned earlier, this will cause meson build to fail. > > + __atomic_store(&lpm->tbl24[i], > > + &zero_tbl24_entry, > __ATOMIC_RELEASE); > > } else if (lpm->tbl24[i].valid_group =3D=3D 1) { > > /* > > * If TBL24 entry is extended, then there > has > > @@ -1365,7 +1376,8 @@ delete_depth_small_v20(struct rte_lpm_v20 *lpm, > uint32_t ip_masked, > > > > if (lpm->tbl24[i].valid_group =3D=3D 0 && > > lpm->tbl24[i].depth <=3D depth) { > > - lpm->tbl24[i] =3D new_tbl24_entry; > > + __atomic_store(&lpm->tbl24[i], > &new_tbl24_entry, > > + __ATOMIC_RELEASE); > > } else if (lpm->tbl24[i].valid_group =3D=3D 1) { > > /* > > * If TBL24 entry is extended, then there > has > > @@ -1647,8 +1659,11 @@ delete_depth_big_v20(struct rte_lpm_v20 *lpm, > uint32_t ip_masked, > > tbl8_recycle_index =3D tbl8_recycle_check_v20(lpm->tbl8, > tbl8_group_start); > > > > if (tbl8_recycle_index =3D=3D -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 =3D 0; > > + __atomic_thread_fence(__ATOMIC_RELEASE); > > tbl8_free_v20(lpm->tbl8, tbl8_group_start); > > } else if (tbl8_recycle_index > -1) { > > /* Update tbl24 entry. */ > > @@ -1659,8 +1674,11 @@ delete_depth_big_v20(struct rte_lpm_v20 *lpm, > uint32_t ip_masked, > > .depth =3D 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] =3D new_tbl24_entry; > > + __atomic_thread_fence(__ATOMIC_RELEASE); > > tbl8_free_v20(lpm->tbl8, tbl8_group_start); > > } > > > > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h > > index 6f5704c5c..906ec4483 100644 > > --- a/lib/librte_lpm/rte_lpm.h > > +++ b/lib/librte_lpm/rte_lpm.h > > @@ -88,7 +88,7 @@ struct rte_lpm_tbl_entry_v20 { > > */ > > uint8_t valid_group :1; > > uint8_t depth :6; /**< Rule depth. */ > > -}; > > +} __rte_aligned(sizeof(uint16_t)); > > > > __extension__ > > struct rte_lpm_tbl_entry { > > @@ -121,7 +121,7 @@ struct rte_lpm_tbl_entry_v20 { > > uint8_t group_idx; > > uint8_t next_hop; > > }; > > -}; > > +} __rte_aligned(sizeof(uint16_t)); > > > > __extension__ > > struct rte_lpm_tbl_entry { > > -- > Regards, > Vladimir > > --=20 Regards, Vladimir