From mboxrd@z Thu Jan 1 00:00:00 1970 From: Declan Doherty Subject: Re: [RFC][PATCH 0/5] cryptodev: Adding support for inline crypto processing of IPsec flows Date: Wed, 24 May 2017 11:06:25 +0100 Message-ID: <34ab35be-17ca-3e23-2042-43e276f88042@intel.com> References: <1494341879-18718-1-git-send-email-radu.nicolau@intel.com> <4071892.busQYGyi5c@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, olivier.matz@6wind.com, jerin.jacob@caviumnetworks.com, Boris Pismenny To: Thomas Monjalon , Radu Nicolau Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 1A4325424 for ; Wed, 24 May 2017 12:06:44 +0200 (CEST) In-Reply-To: <4071892.busQYGyi5c@xps> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 16/05/2017 10:46 PM, Thomas Monjalon wrote: > 09/05/2017 16:57, Radu Nicolau: >> In this RFC we introduce a mechanism to support inline hardware >> acceleration of symmetric crypto processing of IPsec flows >> on Ethernet adapters within the cryptodev framework, >> specifically this RFC includes the initial enablement work >> for the Intel® 82599 10 GbE Controller (IXGBE). > > We must stop after this first introduction and think about what > inline crypto processing is. > > At the beginning are two types of processing: > - networking Rx/Tx > - crypto > Then we want to combine them in the same device. > We could also try to combine more processing: > - compression > - pattern matching > - etc > We will also probably have in future some devices able to combine > processing or do them separately (inline crypto or simple crypto). > > Is there a good way to specify these combinations? > I'm dreaming of a pipeline model with a JIT compiler... > Indeed flexible pipeline device are going to be an interesting challenge to support within DPDK. I think inline offloading of symmetric crypto processing is an interesting offload it doesn't really effect the logical pipeline from an application point of view, it just delays a single element (crypto) of a pipeline stage (IPsec in this case) for performance, as it is always processed in the context of a higher level protocol such as IPsec, SSL, DTLS etc. This is one of the main reasons I proposed the cryptodev model instead of rte_flow type model, as the processing being done inline is exactly the same as provided by a cryptodev PMD be it executed on the host or on a lookaside accelerator, and a full IPsec protocol stack is still required. Also using rte_flow would essentially be creating a second crypto control plane API in DPDK for programming the same functionality. In the future when inline accelerators which can offload the all of processing for a particular protocol, including encap/decap then I rte_flow is a much more appropriate approach. > Here we are adding one more layer to the combination of Rx/Tx + crypto: > it is a specific API for IPsec. > > One more thing in this landscape: > How the eventdev model propose to combine such processing? > > [...] >> 3. The definition of new tx/rx mbuf offload flags to indicate that a packet requires inline crypto processing on to the NIC PMD on transmit and to indicate that a packet has been processed by the inline crypto hardware on ingress. >> >> /** >> * Inline IPSec Rx processed packet >> */ >> #define PKT_RX_IPSEC_INLINE_CRYPTO (1ULL << 17) >> >> /** >> * Inline IPSec Rx packet authentication failed >> */ >> #define PKT_RX_IPSEC_INLINE_CRYPTO_AUTH_FAILED (1ULL << 18) >> >> /** >> * Inline IPSec Tx process packet >> */ >> #define PKT_TX_IPSEC_INLINE_CRYPTO (1ULL << 43) > > We won't be able to add an offload flag for every protocols. > Can we define a more generic flag for Rx crypto failure? > The type of Rx crypto can be defined as a packet type. > IPsec is exactly the same thing as VLAN to this regard. > Olivier, what do you plan for VLAN flags and packet types? > How about: #define PKT_RX_INLINE_CRYPTO (1ULL << 17) #define PKT_RX_INLINE_CRYPTO_PROCESSING_FAILED (1ULL << 18) #define PKT_TX_INLINE_CRYPTO (1ULL << 43) that way it's protocol independent and these flags could be used with any accelerator providing inline crypto functionality for any protocol. > Where is the item 4? :) Lost in space :) just a typo. > >> 5. The addition of inline crypto metadata into the rte_mbuf structure toallow the required egress metadata to be given to the NIC PMD to build thenecessary transmit descriptors in tx_burst processing when the PKT_TX_IPSEC_INLINE_CRYPTO is set. We are looking for feedback on a better approach tohandling the passing of this metadata to the NIC as it is understood that different hardware accelerators which support this offload may have different requirements for metadata depending on implementation and other capabilities in the device. One possibility we have consider is that the last 16 bytes of mbuf is reserved for device specific metadata, which layout is flexible depending on the hardware being used. >> >> struct rte_mbuf { >> ... >> /** Inline IPSec metadata*/ >> struct { >> uint16_t sa_idx; /**< SA index */ >> uint8_t pad_len; /**< Padding length */ >> uint8_t enc; >> } inline_ipsec; >> } __rte_cache_aligned; > > I really think we should stop adding such things in the mbuf. > It is convenient for performance, but have we looked at other options? > I've consider making the use of private data a requirement for using this offload but it seemed a bit restrictive and onerous to application developers, but if no other alternative is available then perhaps this would be the best path forward. Another approach I had considered was managing private cookie's on a per packet basis, but the performance impact of requiring independent freeing of the cookie for each packet meant I ruled that out. In general I think a generic security flow identification field (uint32_t securtiy_flow_id) in the mbuf may work better as an alternative to the inline ipsec struct. This would allow for generic security flow identifier which could be used on both egress and ingress. This would also be applicable to more than just inline crypto IPsec offloads. It could also be used for a full inline IPsec offload and also for offloading of other protocols such as SSL/DTLS in the future. In our case we could also use this to fulfill our requirement to pass metadata with each packet, the security flow id could be used to reference this metadata internally within the pmd. I though it might be possible to extend the hash field for this purpose also but I think it would make sense to have an independent flow id for security protocols as rss/flow director functionality may be performed on the inner decrypted payloads and need to be reported separately. > We cannot reserve a metadata block and share it with other layers, because > it would prevent us from combining offloads of different layers. > And we won't have enough space for every layers. > > There was the same discussion when introducing cryptodev. And the > conclusion was to not directly link any crypto metadata to the mbuf. >