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 9BE0E43374; Sun, 19 Nov 2023 20:58:14 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2703A42D80; Sun, 19 Nov 2023 20:58:14 +0100 (CET) Received: from mail-pf1-f175.google.com (mail-pf1-f175.google.com [209.85.210.175]) by mails.dpdk.org (Postfix) with ESMTP id D641040285 for ; Sun, 19 Nov 2023 20:58:12 +0100 (CET) Received: by mail-pf1-f175.google.com with SMTP id d2e1a72fcca58-6c33ab26dddso3172802b3a.0 for ; Sun, 19 Nov 2023 11:58:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1700423892; x=1701028692; darn=dpdk.org; h=content-transfer-encoding:mime-version:message-id:subject:cc:to :from:date:from:to:cc:subject:date:message-id:reply-to; bh=Ck2taojojXbZQsGj8KtZoHDr9fOUhotxZ3FxwjpXEx8=; b=1cdtLpUdQ93ypVqjVVqfIHDAseqXSY6SuVZOcKs6gGwk901HYMfIhtEoXo4lRIMBI1 aJ3R9+Sly3DqCumbhp6+ap9efqSdmcdYpwQN7CO81Jicux2F82ERZZ2LM6Q3s1shvTn1 FuwntB9ZE8c8zlEl2b16NTcSjVHvcV0bz1bRz8uY0V6wI1IIJGGWN5ifbCQV9Ttm+Wf1 MYvUNSnpZKtFuMuAD3Exv1uMuZ5+V6QdFiiee+x+7tOw6E4UWew0t+8ZoXmwyr9OBhbO rZlA9NqcIcNviS5I3dmDRG5nwzF4XftxPt85PsgeATRttTH65Nty7nQddTp0al1Ry5W7 JdWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700423892; x=1701028692; h=content-transfer-encoding:mime-version:message-id:subject:cc:to :from:date:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Ck2taojojXbZQsGj8KtZoHDr9fOUhotxZ3FxwjpXEx8=; b=c/z9KViV4UvZpdq7vY/ygdaLXMh/DHU06eUZzFtsrFCKxPZPHez8hRCjKi5eV9R86Y pM22NsEY+dhxBZkDFMTJ6bNRFLfquRMmnisAo3znTcLkcE/Z9dT6PShRfxrXW9MVc2La 7xclvCYMPy9yzaDGQzrrhBMeIGmeRyrvWynahNjc4N2OJGTfGmx14Dy0/U0jjBmW7h2+ ewU9i6Xg2Brt3NhVLbqqOrkH77E6QitMr4auk0/88LCeuIeDj0GkT2bj2bzt07rFGCCh TSd8tE+6Hk2I6g4KCcbN8iVPaSoioy/baWts6HDbC/XGr4Jpj4d3PnkmtUeG9N1LrKDY 9+kQ== X-Gm-Message-State: AOJu0YwI5eryeYhwK54O0Ff9O5HBUBNMSswNJ/zd1A86whRZAvqEfgJt 97j1H84wOeNNTvcMqloL5ainxQ== X-Google-Smtp-Source: AGHT+IEJ+wCsx9SMbIp5ZGJp4LqgckFCcuWb1i/xt2tPOskuf82MdtureqIV+VhbWt47awJ+7KpVHg== X-Received: by 2002:a05:6a00:8d87:b0:6c6:b762:ad8c with SMTP id im7-20020a056a008d8700b006c6b762ad8cmr4584014pfb.0.1700423891651; Sun, 19 Nov 2023 11:58:11 -0800 (PST) Received: from hermes.local (204-195-123-141.wavecable.com. [204.195.123.141]) by smtp.gmail.com with ESMTPSA id p17-20020a056a000b5100b006889511ab14sm4666214pfo.37.2023.11.19.11.58.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 19 Nov 2023 11:58:11 -0800 (PST) Date: Sun, 19 Nov 2023 11:58:09 -0800 From: Stephen Hemminger To: Akhil Goyal , Fan Zhang Cc: dev@dpdk.org Subject: The problem with priv_data in crypto and security API's Message-ID: <20231119115809.6303bd89@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 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;