> +To: David Hunt, Distributor maintainer
> +To: Radu Nicolau and Akhil Goyal, IPsec security gateway example maintainers
> 
> > From: Tyler Retzlaff [mailto:roret...@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 <roret...@linux.microsoft.com>
> > ---
> 
> Reviewed-by: Morten Brørup <m...@smartsharesystems.com>
> 
> A few comments for the above mentioned maintainers inline below.
> 
> > 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.

Agreed, these can be removed.

> 
> >  };
> >
> >  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];

Reply via email to