From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B15C0440F9; Wed, 29 May 2024 19:54:56 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 40AB340273; Wed, 29 May 2024 19:54:56 +0200 (CEST) Received: from mail-qt1-f171.google.com (mail-qt1-f171.google.com [209.85.160.171]) by mails.dpdk.org (Postfix) with ESMTP id 82C5B4026F for ; Wed, 29 May 2024 19:54:55 +0200 (CEST) Received: by mail-qt1-f171.google.com with SMTP id d75a77b69052e-43f8e0d8616so10747171cf.1 for ; Wed, 29 May 2024 10:54:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717005295; x=1717610095; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=PAl9GiCYbqVo4g4qtw1XCE1RTDJKLAE5n+cc3gbMzXs=; b=boImxIsZYBh28u5MDSPblzDFHdvl+GuZe6znReCiknGRlZ0JlWxC1e4gQn8JW/wUhy BkgKu7GAAXU1uf2ILG9iLUTPAS2sJvAyhKNCQ5/FDKbJzlTrM4CLz+od0vx7lQDoqiNW QWWjvDC2PD3UQ57vy7f1GMECEsTYq6dwmcqmHSq7Ds6kTTTxqDvwWhKDEGoIbx85l/57 8YBQ+Mmv3D492UtFDv+iuqDKOAEE2ivQqsaVvz7GKCN+ixnBDgTf1739wz1wSj0npgH1 f/feqlNBpCETYsq2dUw2Ki4GBAJ275xbmRMSG7o6dL37ohtzkjvA12iHKZ4POpytEdh6 6TXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717005295; x=1717610095; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PAl9GiCYbqVo4g4qtw1XCE1RTDJKLAE5n+cc3gbMzXs=; b=tS8GJgU2p1o0suRKcAgnh6kzGJpPLyfvB0KO5qVc6+144mc9RrjBXAKl5SR8tmTR5S CkJ3Qiu5iEWJSXz3ik1aEBuurkZkMZjVGBqHxXBTw9pwhQiXHC/khn+1gHjwMu1qfq3m vMJ40/JWNNfJlJtiO72aE6oiI9y35TZVwryU8/xPM+o7uC/9T42OV7BFT6616Kfx9kSE Tq9POoHWjr9QbcUah4CGf6MBYWc7xnLQiQUkjflNEO6Ajgc8Fp9L4M0CWRYAFB1Cis+7 4JkVRllIzTdhPAVcpesz/QSfZrLmT+IFQiDuv1pciHi0Tc1ts7/LSzd6gzecbjawLR5X efkQ== X-Gm-Message-State: AOJu0Yz2S4Iq2rCUzAJayEeaMUGFNXQ2qwtzzx212i9VKQl6Ugmh3s72 qQlsQ+WsVK8dVbzLMnks+6pd8eYHw3Ckyksn9qrM/gjIrRGDUAfdRNEsafz3Xr/TogFsxJ0bAC6 0N2Hg6OYvQMWHPEHqRs19TNE8Etk= X-Google-Smtp-Source: AGHT+IFloR5x5ADYRk9jBDEdDlwpLwDJjPm3/AUjOmI44jVNfK0N58TXTVr74ctu7IewRG5HLdZCtjiVVCXtS+XObbM= X-Received: by 2002:a05:622a:d2:b0:439:f5f0:ac86 with SMTP id d75a77b69052e-43fb0e85883mr188754721cf.17.1717005294593; Wed, 29 May 2024 10:54:54 -0700 (PDT) MIME-Version: 1.0 References: <20240325100500.694748-2-rjarry@redhat.com> <20240327091440.1166119-2-rjarry@redhat.com> In-Reply-To: <20240327091440.1166119-2-rjarry@redhat.com> From: Nithin Dabilpuram Date: Wed, 29 May 2024 23:24:42 +0530 Message-ID: Subject: Re: [PATCH v5] graph: expose node context as pointers To: Robin Jarry Cc: dev , Jerin Jacob , Kiran Kumar K , Nithin Dabilpuram , Zhirun Yan , Tyler Retzlaff Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Acked-by: Nithin Dabilpuram On Wed, Mar 27, 2024 at 2:47=E2=80=AFPM Robin Jarry wro= te: > > In some cases, the node context data is used to store two pointers > because the data is larger than the reserved 16 bytes. Having to define > intermediate structures just to be able to cast is tedious. And without > intermediate structures, casting to opaque pointers is hard without > violating strict aliasing rules. > > Add an unnamed union to allow storing opaque pointers in the node > context. Unfortunately, aligning an unnamed union that contains an array > produces inconsistent results between C and C++. To preserve ABI/API > compatibility in both C and C++, move all fast-path area fields into an > unnamed struct which is cache aligned. Use __rte_cache_min_aligned to > preserve existing alignment on architectures where cache lines are 128 > bytes. > > Add a static assert to ensure that the unnamed union is not larger than > the context array (RTE_NODE_CTX_SZ). > > Signed-off-by: Robin Jarry > --- > > Notes: > v5: > > * Helper functions to hide casting proved to be harder than expected. > Naive casting may even be impossible without breaking strict aliasi= ng > rules. The only other option would be to use explicit memcpy calls. > * Unnamed union tentative again. As suggested by Tyler (thank you!), > using an intermediate unnamed struct to carry the alignment produce= s > consistent ABI in C and C++. > * Also, Tyler (thank you!) suggested that the fast path area alignmen= t > size may be incorrect for architectures where the cache line is not= 64 > bytes. There will be a 64 bytes hole in the structure at the end of > the unnamed struct before the zero length next nodes array. Use > __rte_cache_min_aligned to preserve existing alignment. > > v4: > > * Replaced the unnamed union with helper inline functions. > > v3: > > * Added __extension__ to the unnamed struct inside the union. > * Fixed C++ header checks. > * Replaced alignas() with an explicit static_assert. > > lib/graph/rte_graph_worker_common.h | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/lib/graph/rte_graph_worker_common.h b/lib/graph/rte_graph_wo= rker_common.h > index 36d864e2c14e..84d4997bbbf6 100644 > --- a/lib/graph/rte_graph_worker_common.h > +++ b/lib/graph/rte_graph_worker_common.h > @@ -12,7 +12,9 @@ > * process, enqueue and move streams of objects to the next nodes. > */ > > +#include > #include > +#include > > #include > #include > @@ -111,14 +113,21 @@ struct __rte_cache_aligned rte_node { > } dispatch; > }; > /* Fast path area */ > + __extension__ struct __rte_cache_min_aligned { > #define RTE_NODE_CTX_SZ 16 > - alignas(RTE_CACHE_LINE_SIZE) uint8_t ctx[RTE_NODE_CTX_SZ]; /**< N= ode Context. */ > - uint16_t size; /**< Total number of objects available. *= / > - uint16_t idx; /**< Number of objects used. */ > - rte_graph_off_t off; /**< Offset of node in the graph reel. */ > - uint64_t total_cycles; /**< Cycles spent in this node. */ > - uint64_t total_calls; /**< Calls done to this node. */ > - uint64_t total_objs; /**< Objects processed by this node. */ > + union { > + uint8_t ctx[RTE_NODE_CTX_SZ]; > + __extension__ struct { > + void *ctx_ptr; > + void *ctx_ptr2; > + }; > + }; /**< Node Context. */ > + uint16_t size; /**< Total number of objects avai= lable. */ > + uint16_t idx; /**< Number of objects used. */ > + rte_graph_off_t off; /**< Offset of node in the graph = reel. */ > + uint64_t total_cycles; /**< Cycles spent in this node. *= / > + uint64_t total_calls; /**< Calls done to this node. */ > + uint64_t total_objs; /**< Objects processed by this no= de. */ > union { > void **objs; /**< Array of object pointers.= */ > uint64_t objs_u64; > @@ -127,9 +136,13 @@ struct __rte_cache_aligned rte_node { > rte_node_process_t process; /**< Process function= . */ > uint64_t process_u64; > }; > + }; > alignas(RTE_CACHE_LINE_MIN_SIZE) struct rte_node *nodes[]; /**< N= ext nodes. */ > }; > > +static_assert(offsetof(struct rte_node, size) - offsetof(struct rte_node= , ctx) =3D=3D RTE_NODE_CTX_SZ, > + "rte_node context must be RTE_NODE_CTX_SZ bytes exactly"); > + > /** > * @internal > * > -- > 2.44.0 >