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 B019E43DCA; Mon, 8 Apr 2024 17:44:06 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3A145402C9; Mon, 8 Apr 2024 17:44:06 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id F2E17402C0 for ; Mon, 8 Apr 2024 17:44:04 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id E9FB920EA435; Mon, 8 Apr 2024 08:44:03 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com E9FB920EA435 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1712591043; bh=tDT7HYc8/3fb9fOhJT8TDVt7IdXdBjhxwCPWPNBKaCk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=SD/pGcrFP2H+r8oMJYjDQghb2/8x/OdvVDRfXrHksRtIBv1xah/rNkjzbeALq5j5j MwPGQzpMc4J8/idcqzND3SGQQ/qZgHoNvJqsP5lIyYsC5SDqTulMHsbaodQ3TchDKZ X2V0XT4HZ7qNfKrnyhfgkRiWR1wNEN4mISx/+e2I= Date: Mon, 8 Apr 2024 08:44:03 -0700 From: Tyler Retzlaff To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: dev@dpdk.org, David Hunt , Radu Nicolau , Akhil Goyal , Mattias =?iso-8859-1?Q?R=F6nnblom?= , "Min Hu (Connor)" , Abdullah Sevincer , Ajit Khaparde , Alok Prasad , Amit Bernstein , Anatoly Burakov , Andrew Boyer , Andrew Rybchenko , Ankur Dwivedi , Anoob Joseph , Ashish Gupta , Ashwin Sekhar T K , Bruce Richardson , Byron Marohn , Chaoyong He , Chas Williams , Chenbo Xia , Chengwen Feng , Conor Walsh , Cristian Dumitrescu , Dariusz Sosnowski , Devendra Singh Rawat , Ed Czeck , Evgeny Schemeilin , Fan Zhang , Gagandeep Singh , Guoyang Zhou , Harman Kalra , Harry van Haaren , Hemant Agrawal , Honnappa Nagarahalli , Hyong Youb Kim , Jakub Grajciar , Jerin Jacob , Jian Wang , Jiawen Wu , Jie Hai , Jingjing Wu , John Daley , John Miller , Joyce Kong , Junfeng Guo , Kai Ji , Kevin Laatz , Kiran Kumar K , Konstantin Ananyev , Lee Daly , Liang Ma , Liron Himi , Long Li , Maciej Czekaj , Matan Azrad , Matt Peters , Maxime Coquelin , Michael Shamis , Nagadheeraj Rottela , Nicolas Chautru , Nithin Dabilpuram , Ori Kam , Pablo de Lara , Pavan Nikhilesh , Peter Mccarthy , Rahul Lakkireddy , Rakesh Kudurumalla , Raveendra Padasalagi , Reshma Pattan , Ron Beider , Ruifeng Wang , Sachin Saxena , Selwin Sebastian , Shai Brandes , Shepard Siegel , Shijith Thotton , Sivaprasad Tummala , Somnath Kotur , Srikanth Yalavarthi , Stephen Hemminger , Steven Webster , Suanming Mou , Sunil Kumar Kori , Sunil Uttarwar , Sunila Sahu , Tejasree Kondoj , Viacheslav Ovsiienko , Vikas Gupta , Volodymyr Fialko , Wajeeh Atrash , Wisam Jaddo , Xiaoyun Wang , Yipeng Wang , Yisen Zhuang , Yuying Zhang , Zhangfei Gao , Zhirun Yan , Ziyang Xuan Subject: Re: [PATCH 01/83] examples: move alignment attribute on types Message-ID: <20240408154403.GA30422@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1710949096-5786-1-git-send-email-roretzla@linux.microsoft.com> <1710949096-5786-2-git-send-email-roretzla@linux.microsoft.com> <98CBD80474FA8B44BF855DF32C47DC35E9F371@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F371@smartserver.smartshare.dk> User-Agent: Mutt/1.5.21 (2010-09-15) 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 On Sat, Apr 06, 2024 at 04:55:06PM +0200, Morten Brørup wrote: > +To: David Hunt, Distributor maintainer > +To: Radu Nicolau and Akhil Goyal, IPsec security gateway example maintainers thanks morten. if the maintainers reply to acknowledge i will update the series with your recommended feedback. i noticed a couple of these as well when making the conversion but resist the urge to change them to avoid confusion/review overhead. > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > > Sent: Wednesday, 20 March 2024 16.37 > > > > Move location of __rte_aligned(a) to new conventional location. The new > > placement between {struct,union} and the tag allows the desired > > alignment to be imparted on the type regardless of the toolchain being > > used for both C and C++. Additionally, it avoids confusion by Doxygen > > when generating documentation. > > > > Signed-off-by: Tyler Retzlaff > > --- > > Reviewed-by: Morten Brørup > > A few comments for the above mentioned maintainers inline below. > > > diff --git a/examples/distributor/main.c b/examples/distributor/main.c > > index 542f76c..ddbc387 100644 > > --- a/examples/distributor/main.c > > +++ b/examples/distributor/main.c > > @@ -44,39 +44,39 @@ > > unsigned int num_workers; > > > > static volatile struct app_stats { > > - struct { > > + alignas(RTE_CACHE_LINE_SIZE) struct { > > uint64_t rx_pkts; > > uint64_t returned_pkts; > > uint64_t enqueued_pkts; > > uint64_t enqdrop_pkts; > > - } rx __rte_cache_aligned; > > - int pad1 __rte_cache_aligned; > > + } rx; > > + alignas(RTE_CACHE_LINE_SIZE) int pad1; > > pad1/2/3/4/5 should be replaced by RTE_CACHE_GUARD, if that is their purpose. > > @David: You might want to change this in a future patch. > > > > > - struct { > > + alignas(RTE_CACHE_LINE_SIZE) struct { > > uint64_t in_pkts; > > uint64_t ret_pkts; > > uint64_t sent_pkts; > > uint64_t enqdrop_pkts; > > - } dist __rte_cache_aligned; > > - int pad2 __rte_cache_aligned; > > + } dist; > > + alignas(RTE_CACHE_LINE_SIZE) int pad2; > > > > - struct { > > + alignas(RTE_CACHE_LINE_SIZE) struct { > > uint64_t dequeue_pkts; > > uint64_t tx_pkts; > > uint64_t enqdrop_pkts; > > - } tx __rte_cache_aligned; > > - int pad3 __rte_cache_aligned; > > + } tx; > > + alignas(RTE_CACHE_LINE_SIZE) int pad3; > > > > - uint64_t worker_pkts[64] __rte_cache_aligned; > > + alignas(RTE_CACHE_LINE_SIZE) uint64_t worker_pkts[64]; > > > > - int pad4 __rte_cache_aligned; > > + alignas(RTE_CACHE_LINE_SIZE) int pad4; > > > > - uint64_t worker_bursts[64][8] __rte_cache_aligned; > > + alignas(RTE_CACHE_LINE_SIZE) uint64_t worker_bursts[64][8]; > > > > - int pad5 __rte_cache_aligned; > > + alignas(RTE_CACHE_LINE_SIZE) int pad5; > > > > - uint64_t port_rx_pkts[64] __rte_cache_aligned; > > - uint64_t port_tx_pkts[64] __rte_cache_aligned; > > + alignas(RTE_CACHE_LINE_SIZE) uint64_t port_rx_pkts[64]; > > + alignas(RTE_CACHE_LINE_SIZE) uint64_t port_tx_pkts[64]; > > } app_stats; > > > > struct app_stats prev_app_stats; > > [...] > > > diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h > > index bdcada1..4cf4c9d 100644 > > --- a/examples/ipsec-secgw/ipsec.h > > +++ b/examples/ipsec-secgw/ipsec.h > > @@ -112,7 +112,7 @@ enum { > > return (struct ipsec_sa *)i; > > } > > > > -struct ipsec_sa { > > +struct __rte_cache_aligned ipsec_sa { > > struct rte_ipsec_session sessions[IPSEC_SESSION_MAX]; > > uint32_t spi; > > struct cdev_qp *cqp[RTE_MAX_LCORE]; > > @@ -170,7 +170,7 @@ struct ipsec_sa { > > struct rte_flow_item_esp esp_spec; > > struct rte_flow *flow; > > struct rte_security_session_conf sess_conf; > > -} __rte_cache_aligned; > > +}; > > > > struct ipsec_xf { > > struct rte_crypto_sym_xform a; > > @@ -190,12 +190,12 @@ struct sa_ctx { > > struct ipsec_sa sa[]; > > }; > > > > -struct ipsec_mbuf_metadata { > > +struct __rte_cache_aligned ipsec_mbuf_metadata { > > struct ipsec_sa *sa; > > struct rte_crypto_op cop; > > struct rte_crypto_sym_op sym_cop; > > uint8_t buf[32]; > > -} __rte_cache_aligned; > > +}; > > > > #define IS_TRANSPORT(flags) ((flags) & TRANSPORT) > > > > @@ -224,7 +224,7 @@ struct cdev_qp { > > uint16_t qp; > > uint16_t in_flight; > > uint16_t len; > > - struct rte_crypto_op *buf[MAX_PKT_BURST] __rte_aligned(sizeof(void > > *)); > > + alignas(sizeof(void *)) struct rte_crypto_op *buf[MAX_PKT_BURST]; > > Aligning a pointer to the size of a pointer is superfluous, unless the structure is packed. > > @Radu, @Akhil: You might want to remove these in a future patch. > > > }; > > > > struct ipsec_ctx { > > @@ -235,7 +235,7 @@ struct ipsec_ctx { > > uint16_t nb_qps; > > uint16_t last_qp; > > struct cdev_qp tbl[MAX_QP_PER_LCORE]; > > - struct rte_mbuf *ol_pkts[MAX_PKT_BURST] __rte_aligned(sizeof(void > > *)); > > + alignas(sizeof(void *)) struct rte_mbuf *ol_pkts[MAX_PKT_BURST]; > > uint16_t ol_pkts_cnt; > > uint64_t ipv4_offloads; > > uint64_t ipv6_offloads; > > @@ -283,18 +283,18 @@ struct cnt_blk { > > uint32_t cnt; > > } __rte_packed; > > > > -struct lcore_rx_queue { > > +struct __rte_cache_aligned lcore_rx_queue { > > uint16_t port_id; > > uint8_t queue_id; > > void *sec_ctx; > > -} __rte_cache_aligned; > > +}; > > > > struct buffer { > > uint16_t len; > > - struct rte_mbuf *m_table[MAX_PKT_BURST] __rte_aligned(sizeof(void > > *)); > > + alignas(sizeof(void *)) struct rte_mbuf *m_table[MAX_PKT_BURST]; > > }; > > > > -struct lcore_conf { > > +struct __rte_cache_aligned lcore_conf { > > uint16_t nb_rx_queue; > > struct lcore_rx_queue rx_queue_list[MAX_RX_QUEUE_PER_LCORE]; > > uint16_t tx_queue_id[RTE_MAX_ETHPORTS];