From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id A771A2E8B for ; Tue, 11 Jul 2017 19:06:18 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2017 10:06:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,347,1496127600"; d="scan'208";a="285567093" Received: from dwdohert-mobl1.ger.corp.intel.com (HELO [163.33.228.191]) ([163.33.228.191]) by fmsmga004.fm.intel.com with ESMTP; 11 Jul 2017 10:06:07 -0700 To: Boris Pismenny , dev@dpdk.org References: <1499672117-56728-1-git-send-email-borisp@mellanox.com> Cc: aviadye@mellanox.com, "Nicolau, Radu" From: Declan Doherty Message-ID: Date: Tue, 11 Jul 2017 18:06:06 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1499672117-56728-1-git-send-email-borisp@mellanox.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [RFC 0/7] ipsec inline X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Jul 2017 17:06:19 -0000 On 10/07/2017 8:35 AM, Boris Pismenny wrote: > In this RFC we introduce a infrastructure for IPSec inline hardware offloading. > This RFC introduces device capabilities, configuration API and data path > processing. We also provide a comparison with a previous RFC posted on the list > for this feature. > Hey Boris, we've been working on v2 of the RFC based on the feedback you and others gave on our original , but as what we were going to propose is largely inline with your proposal here, with one or 2 exceptions, mainly on the IPsec SA management elements, I'll just comment here instead of sending another RFC. We agree the rte_flow based approach as proposed here is the more flexible approach and should work better with futures devices which could offer support for other protocols as well as full protocol offload. The main difference to your proposal below and what we are considering is that we would like to introduce the idea of a port based rte_security API which would support a generic API for security protocol configuration, I can see MACsec, IPsec, DTLS all working easily under this approach. struct rte_security_session * rte_security_session_create(uint8_t port_id, struct rte_security_sess_conf *sess_conf); The session create function will return a opaque security session which would be used in the security flow action programming. The session configuration will contain the security protocol specific information, in IPsec case the SA parameter as well as the crypto xforms. /** IPsec Security Session Configuration */ struct rte_security_conf_ipsec_sa { unsigned int spi; /**< SA security parameter index */ enum rte_security_conf_ipsec_sa_dir { RTE_SECURITY_IPSEC_SA_DIR_INGRESS, RTE_SECURITY_IPSEC_SA_DIR_EGRESS } direction; /**< IPsec SA direction - ingress / egress */ enum rte_security_conf_ipsec_sa_mode { RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT, RTE_SECURITY_IPSEC_SA_MODE_TUNNEL } mode; /**< IPsec SA Mode - transport/tunnel */ enum rte_security_conf_ipsec_sa_protocol { RTE_SECURITY_IPSEC_SA_PROTO_AH, RTE_SECURITY_IPSEC_SA_PROTO_ESP } proto; /**< IPsec SA Protocol - AH/ESP */ struct ipaddr src_ip; /**< Source IP */ struct ipaddr dst_ip; /**< Destination IP */ }; /** * Security Session Configuration */ struct rte_security_sess_conf { enum { RTE_SECURITY_SESS_INLINE_CRYPTO, RTE_SECURITY_SESS_FULL_PROTO_OFFLOAD } action_type; enum rte_security_sess_conf_type { SEC_CONF_DTLS, SEC_CONF_IPSEC, SEC_CONF_MACSEC } type; /**< Type of security session to be configured */ struct { struct rte_security_conf_dtls dtls; struct rte_security_conf_ipsec_sa ipsec_sa; struct rte_security_conf_macsec macsec; }; /* Configuration parameters for security session */ struct rte_crypto_sym_xform *xform; /**< Symmetric Crypto Transform Chain */ }; The APIs would be introduced in the same manner as the flow and traffic management API as a plug-able component into a ethdev, and would provide the abstraction to configure security protocols state (IPsec SA, DTLS record etc.) and then the flow action would be a security flow action instead of the crypto flow action as proposed below. This gives a flexible approach to future extension to other protocols and modes (crypto vs full offload) and also addresses an issue raised on the our previous RFC regarding polluting the crypto namespace with security protocol specific information. One other issue with putting the protocol information into a crypto transform is that we won't have any crypto devices which support them. > 1. Inline crypto processing > 1.1. Device Capabilities: > o DEV_RX_OFFLOAD_IPSEC_CRYPTO - device support inline ipsec > decryption offload. > o DEV_TX_OFFLOAD_IPSEC_CRYPTO_HW_TRAILER - device support inline ipsec > encrypted offload, ipsec trailer is added by hardware. > o DEV_TX_OFFLOAD_IPSEC_CRYPTO_TSO - device support inline ipsec > encrypted offload within segment large packets, ipsec trailer is added by > hardware to each segment. > > 1.2. Configuration API: > We will modify steering API in order to add IPsec transform actions. > > o Definition of ESP header: > > struct esp_hdr { > int32_t spi; /**< Security Parameters Index */ > uint32_t seq; /**< packet sequence number */ > } __attribute__((__packed__)); > > o New flow item: > > enum rte_flow_item_type { > ... > > /** > * Matches a ESP header. > * > * See struct rte_flow_item_esp. > */ > RTE_FLOW_ITEM_TYPE_ESP, > }; > > struct rte_flow_item_esp { > struct esp_hdr hdr; /**< ESP header definition. */ > }; > > struct rte_flow_item_esp { > static const struct rte_flow_item_esp rte_flow_item_esp_mask = { > .hdr = { > .spi = 0xffffffff, > }, > }; > > o New ipsec transform: > struct rte_crypto_ipsec_xform { > enum rte_crypto_cipher_operation op; > enum rte_crypto_cipher_algorithm algo; > > struct { > uint8_t *data; /**< pointer to key data */ > size_t length; /**< key length in bytes */ > } key; > > uint32_t salt; /* salt for this security association */ > }; > > /** Crypto transformation types */ > enum rte_crypto_sym_xform_type { > ... > RTE_CRYPTO_SYM_XFORM_CIPHER, /**< Cipher xform */ > RTE_CRYPTO_SYM_XFORM_IPSEC, /**< IPsec xform */ > }; > > struct rte_crypto_sym_xform { > ... > struct rte_crypto_ipsec_xform ipsec; > /**< IPsec xform */ > }; > As mentioned above I think it would be better to keep the security protocol specific information separate to the crypto APIs > > o New flow action: > > enum rte_flow_action_type { > ... > > /** > * Encrypts or decrypts packets matching this flow. Must be either egress > * or ingress, but not both. > * > * See struct rte_flow_action_crypto. > */ > RTE_FLOW_ACTION_TYPE_CRYPTO, > }; > > struct rte_flow_action_crypto { > struct rte_crypto_sym_xform xform; /* applied crypto transform */ > }; > > > Configuration Path > | > +--------|--------+ > | Add/Remove | > | IPsec SA | <------ Build crypto flow action of ipsec transform > | | | > |--------|--------| > | > +--------V--------+ > | Flow API | > +--------|--------+ > | > +--------V--------+ > | | > | NIC PMD | <------ Add/Remove SA to/from hw context > | | > +--------|--------+ > | > +--------|--------+ > | HW ACCELERATED | > | NIC | > | | > +--------|--------+ > > o Add/Delete SA flow: > To add a new inline SA construct a rte_flow_item for Ethernet + IP + ESP > using the SA selectors and the rte_crypto_ipsec_xform as the rte_flow_action. > Note that any rte_flow_items may be empty, which means it is not checked. > > In its most basic form, IPsec flow specification is as follows: > +-------+ +----------+ +--------+ +-----+ > | Eth | -> | IP4/6 | -> | ESP | -> | END | > +-------+ +----------+ +--------+ +-----+ > > However, the API can represent, IPsec crypto offload with any encapsulation: > > +-------+ +--------+ +-----+ > | Eth | -> ... -> | ESP | -> | END | > +-------+ +--------+ +-----+ > For egress I think we need to support creating multiple flows using the same crypto action, i.e. multiple plaintext flows to a single IPsec SA. I think having the security_sess action would make this much easier than having to match crypto transform chains within the PMD to see if the action SA is the same as an existing one. Also it will be difficult to update the tunnel state if there isn't as separate API for configuration. > 1.3. Data Path Processing: > > 1.3.1. mbuf Changes > o New rx mbuf offload flags to indicate that a packet has gone through > inline crypto processing on to the NIC PMD and the result of this processing. > On failure, packets should be dropped. > /** > * Mask of bits used to determine the status of RX IPsec crypto. > * - PKT_RX_IPSEC_CRYPTO_UNKNOWN : no information about the RX IPsec crypto > * - PKT_RX_IPSEC_CRYPTO � : decryption and authentication were performed > * - PKT_RX_IPSEC_CRYPTO_FAILED : ipsec processing failed > */ > #define PKT_RX_IPSEC_CRYPTO_UNKNOWN 0 This doesn't make sense to me, it's implicit in the flag below not being set? > #define PKT_RX_IPSEC_CRYPTO (1ULL << 18) > #define PKT_RX_IPSEC_CRYPTO_FAILED (1ULL << 19) > > o New tx mbuf offload flags to indicate that a packet requires IPsec inline > crypto processing and trailer construction on the NIC PMD. > I know Thomas raised a concern of having to add multiple crypto flags for different protocol. If we go with the security session model, then PKT_RX_SECURITY & PKT_RX_SECURITY_FAILED flag would be sufficient to support all security protocols I think, especially if we added a generic field for security_session_id in the mbuf which could be populated in the rx_burst function. > /** > * Offload the IPsec encryption with software provided trailer. > * This flag must be set by the application to enable this > * offload feature for a packet to be transmitted. > */ > #define PKT_TX_IPSEC_CRYPTO (1ULL << 42) > > /** > * Offload the IPsec encryption and trailer construction. This flag must > * be set by the application to enable this offload feature for a packet > * to be transmitted. > */ > #define PKT_TX_IPSEC_CRYPTO_HW_TRAILER (1ULL << 43) Would the hw trailer not be implicit by the device you are using, or do you think that there may be a case when some flows would have this set and some wouldn't? > > > Egress Data Path > | > +--------|--------+ > | egress IPsec | > | | | > | +------V------+ | > | | SABD lookup | | > | +------|------+ | > | +------V------+ | > | | Tunnel | | <------ Add tunnel header to packet > | +------|------+ | > | +------V------+ | > | | ESP | | <------ Add ESP header without trailer to packet > | | | | <------ Mark packet to be offloaded, add trailer meta-data > | +------|------+ | to mbuf > +--------V--------+ > | > +--------V--------+ > | L2 Stack | > +--------|--------+ > | > +--------V--------+ > | | > | NIC PMD | <------ Set hw context for inline crypto offload > | | > +--------|--------+ > | > +--------|--------+ > | HW ACCELERATED | <------ Packet Encryption/Decryption and > | NIC | Authentication happens inline > | | > +--------|--------+ > > > > 2. IPsec Gateway Sample Application > 2.1. Add/Delete SA > SAs are configured as previously via the configuration file, the user could > specify which SAs require inline offload by adding "offload" for that SA. > We then store for each SA whether it is offloaded in the ipsec_sa structure. > Additionally, we extended the ipsec_sa structure with additional fields > related to rte_flow, which are initialized if offload is requested. > > struct ipsec_sa { > ... > #define MAX_RTE_FLOW_PATTERN (4) > // ETH + IP + ESP + END > union { > struct rte_flow_item_ipv4 ipv4; > struct rte_flow_item_ipv6 ipv6; > } ip_spec; > struct rte_flow_item_esp esp_spec; > struct rte_flow_item pattern[MAX_RTE_FLOW_PATTERN]; > #define MAX_RTE_FLOW_ACTIONS (3) > struct rte_flow_action_crypto crypto_action; > struct rte_flow_action action[MAX_RTE_FLOW_ACTIONS]; > struct rte_flow *flow; > ... > int offload; > #define OFFLOAD_ENABLED 1 > #define OFFLOAD_DISABLED 0 > ... > }; > > When a user attempts to crete a session it checks whether inline offload is > needed. If yes, then rte_flow_create is called with the parameters > initialized above. In any case, create_session_cryptodev is called. The > cryptodev session is used to handle software fallback for packets that are > not processed by hardware. > > 3. Comparison with previous RFC > In the section we compare the previous RFC > (http://dpdk.org/ml/archives/dev/2017-May/065513.html) > with this RFC. > > The main difference is in NIC capabilities. Mellanox hardware > is capable of applying IPsec inline based on packet pattern, while > intel use mbuf metadata. This API allows application to save cycles > by not storing metadata on each packet mbuf, but instead store metadata > once per flow. > > Another difference is the use of Crypto PMD for inline IPsec vs. > NIC PMD. Using the NIC PMD has several advantages. First, configuration > is simpler - crypto is performed by the same device to which packets > are routed, and there is no chance for configuring a crypto context on > one device and routing packets to another device and transmitting > plaintext as a result. Second, Crypto PMD semantics imply that data has > been processed after dequeue. This is not true for inline IPsec, where > data is processed only when it goes through the NIC. Finally, using the > NIC PMD directly has better performance, because the Crypto PMD code is > not involved. > > Aviad Yehezkel (2): > mbuf: Added next_esp_proto field > example/ipsec_gw: Support SA offload in datapath > > Boris Pismenny (5): > ethdev: add device ipsec encrypt/decrypt capability flags > ethdev: Add ESP header to generic flow steering > ethdev: add rte flow action for crypto > cryptodev: add ipsec xform > mbuf: Add IPsec crypto flags > > examples/ipsec-secgw/esp.c | 68 ++++++++++++---- > examples/ipsec-secgw/esp.h | 13 +--- > examples/ipsec-secgw/ipsec.c | 142 +++++++++++++++++++++++++++++----- > examples/ipsec-secgw/ipsec.h | 30 +++++++ > examples/ipsec-secgw/sa.c | 120 ++++++++++++++++++++++++---- > lib/Makefile | 1 + > lib/librte_cryptodev/rte_crypto_sym.h | 42 +++++++++- > lib/librte_ether/rte_ethdev.h | 4 + > lib/librte_ether/rte_flow.h | 50 ++++++++++++ > lib/librte_mbuf/rte_mbuf.c | 16 ++++ > lib/librte_mbuf/rte_mbuf.h | 38 ++++++++- > lib/librte_net/Makefile | 2 +- > 12 files changed, 466 insertions(+), 60 deletions(-) >