[dpdk-dev] [RFC][PATCH 0/5] cryptodev: Adding support for inline crypto processing of IPsec flows

[Declan Doherty]

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.
>