The problem with priv_data in crypto and security API's

[Stephen Hemminger]

In the patchset, that removes the GNU style zero length arrays,
there is a problem caused by the use of these in the cryptodev
header files.

For both rte_session and rte_cryptodev, the current convention
is to do:

struct rte_security_session {
	RTE_MARKER cacheline0;
	uint64_t opaque_data;
	/**< Opaque user defined data */
	uint64_t fast_mdata;
	/**< Fast metadata to be used for inline path */
	rte_iova_t driver_priv_data_iova;
	/**< session private data IOVA address */

	RTE_MARKER cacheline1 __rte_cache_min_aligned;
	uint8_t driver_priv_data[0];
	/**< Private session material, variable size (depends on driver) */
};

This can not just be replaced with a C90 flex array because then
clang correctly complains of using flex array not at the end
of the structure.

struct cn9k_sec_session {
	struct rte_security_session rte_sess;

	/** PMD private space */

	/** ESN */
	union {
		uint64_t esn;
		struct {
			uint32_t seq_lo;
			uint32_t seq_hi;
		};
	};
	/** IPsec SA direction */
	uint8_t is_outbound;
	/* ESN enable flag */
	uint8_t esn_en;
	/** Pre-populated CPT inst words */
	struct cnxk_cpt_inst_tmpl inst;
	/** Response length calculation data */
	struct cnxk_ipsec_outb_rlens rlens;
	/** Anti replay window size */
	uint32_t replay_win_sz;
	/** Cipher IV offset in bytes */
	uint16_t cipher_iv_off;
	/** Cipher IV length in bytes */
	uint8_t cipher_iv_len;
	/** Outbound custom header length */
	uint8_t custom_hdr_len;
	/** Anti replay */
	struct cnxk_on_ipsec_ar ar;
	/** Queue pair */
	struct cnxk_cpt_qp *qp;

	struct cn9k_ipsec_sa sa;
} __rte_cache_aligned;



There are a couple of ways to fix this, and both are non-trivial.
The first is to get rid of the priv_data element and modify the
management to do the cache alignment.

Something like:

diff --git a/lib/security/rte_security_driver.h b/lib/security/rte_security_driver.h
index faa4074f1965..ffc5b5d7257b 100644
--- a/lib/security/rte_security_driver.h
+++ b/lib/security/rte_security_driver.h
@@ -30,11 +30,6 @@ struct rte_security_session {
        uint64_t fast_mdata;
        /**< Fast metadata to be used for inline path */
        rte_iova_t driver_priv_data_iova;
-       /**< session private data IOVA address */
-
-       RTE_MARKER cacheline1 __rte_cache_min_aligned;
-       uint8_t driver_priv_data[];
-       /**< Private session material, variable size (depends on driver) */
 };
 
 /**
@@ -61,13 +56,33 @@ struct rte_security_ctx {
        /**< Number of MACsec SA attached to this context */
 };
 
+/**
+ * Helper to acces rte_session private data
+ * @param      sess
+ *    Pointer to Security private session structure
+ *
+ * @return
+ *    Pointer to area reserved for private data
+ */
+static inline void *
+rte_security_priv(const struct rte_security_session *sess)
+{
+       return (void *)((uintptr_t)sess + RTE_CACHE_LINE_ROUNDUP(sizeof(*sess)));
+}
+

diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
index b082a290296b..f3202e3df6cd 100644
--- a/lib/security/rte_security.c
+++ b/lib/security/rte_security.c
@@ -66,6 +66,7 @@ rte_security_session_create(void *ctx,
 {
        struct rte_security_session *sess = NULL;
        struct rte_security_ctx *instance = ctx;
+       const size_t sess_size = RTE_CACHE_LINE_ROUNDUP(sizeof(*sess));
        uint32_t sess_priv_size;
 
        RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, session_create, NULL, NULL);
@@ -73,17 +74,16 @@ rte_security_session_create(void *ctx,
        RTE_PTR_OR_ERR_RET(mp, NULL);
 
        sess_priv_size = instance->ops->session_get_size(instance->device);
-       if (mp->elt_size < (sizeof(struct rte_security_session) + sess_priv_size))
+       if (mp->elt_size < sess_size + sess_priv_size)
                return NULL;
 
        if (rte_mempool_get(mp, (void **)&sess))
                return NULL;
 
        /* Clear session priv data */
-       memset(sess->driver_priv_data, 0, sess_priv_size);
+       memset(rte_security_priv(sess), 0, sess_priv_size);
 
-       sess->driver_priv_data_iova = rte_mempool_virt2iova(sess) +
-                       offsetof(struct rte_security_session, driver_priv_data);
+       sess->driver_priv_data_iova = rte_mempool_virt2iova(sess) + sess_size;
        if (instance->ops->session_create(instance->device, conf, sess)) {
                rte_mempool_put(mp, (v

This is what Linux kernel does with netdev_priv() helper.

There is also a lot of open coded versions of same thing
in Intel crypto drivers. Anything with that many casts looks
like a design mistake to me.

      struct ixgbe_crypto_session *ic_session = (void *)(uintptr_t)
                       ((const struct rte_security_session *)sess)->driver_priv_data;