From mboxrd@z Thu Jan 1 00:00:00 1970 From: Narayana Prasad Athreya Subject: Re: [RFC PATCH v2 0/4] IPSec Inline and look aside crypto offload Date: Mon, 28 Aug 2017 19:56:05 +0530 Message-ID: References: <1b68515b-bb11-5da7-6a94-b30c04294478@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: "Kulkarni, Sunil" , "Chandran, Suheil" , "Murthy, Nidadavolu" To: dev@dpdk.org, "Berestovskyy, Andriy" , "Manoharan, Balasubramanian" Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0078.outbound.protection.outlook.com [104.47.34.78]) by dpdk.org (Postfix) with ESMTP id 319341B53 for ; Mon, 28 Aug 2017 16:26:08 +0200 (CEST) In-Reply-To: <1b68515b-bb11-5da7-6a94-b30c04294478@caviumnetworks.com> 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 Monday 28 August 2017 07:53 PM, Narayana Prasad Athreya wrote: >> This patchet showcases the definition and usage of the rte_security >> APIs described in the RFC v1 sent earlier. >> >> The data path and configuration path is similar to what was proposed in >> version 1. However, rte_security_configure API is removed, as it looked >> redundant. >> >> Also the rte_security.x files are placed inside the lib/librte_cryptodev/ >> as the APIs are defined with the help of crypto APIs and it makes more sense >> to extend the cryptodev library instead of a separate library which perform >> similar actions. >> >> Some of the parameters of the APIs are also modified for better usability. >> The parameter ``dev_name`` is removed as the appropriate device(crypto/eth) >> can be obtained by using the action type. >> >> The patchset is still in work in progress state and there may be some changes >> and cleanup in the next version. This is just to enable others to work >> in parallel on the crypto offloading using ethernet devices. >> >> This patchset include the definition of rte_security APIs in patch 1, >> changes required in cryptodev in patch 2, sample driver implementation >> in patch 3 and ipsec-secgw application changes in patch 4. >> >> Akhil Goyal (4): >> RFC2: rte_security: API definitions >> cryptodev: entend cryptodev to support security APIs >> crypto/dpaa2_sec: add support for protocol offload ipsec >> example/ipsec-secgw: add support for offloading crypto op >> >> drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 368 ++++++++++++++++++++++++- >> drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h | 33 +++ >> examples/ipsec-secgw/ipsec.c | 125 ++++++--- >> examples/ipsec-secgw/ipsec.h | 13 +- >> examples/ipsec-secgw/sa.c | 142 +++++++--- >> lib/librte_cryptodev/Makefile | 3 +- >> lib/librte_cryptodev/rte_crypto_sym.h | 15 + >> lib/librte_cryptodev/rte_cryptodev.h | 20 +- >> lib/librte_cryptodev/rte_cryptodev_pmd.h | 35 +++ >> lib/librte_cryptodev/rte_security.c | 171 ++++++++++++ >> lib/librte_cryptodev/rte_security.h | 409 ++++++++++++++++++++++++++++ >> 11 files changed, 1243 insertions(+), 91 deletions(-) >> create mode 100644 lib/librte_cryptodev/rte_security.c >> create mode 100644 lib/librte_cryptodev/rte_security.h >> >> -- >> 2.9.3 > I have a few questions/comments on the v1 and v2 versions of this > patch. I accumulated these from a few different cavium stakeholders. > > 1. conf_ipsec_sa::sa_dir and ipsec_xform::op seem to have same purpose. > 2. Its unclear how the Crypto Device will be configured to use a > specific Network device and vice-versa. The situation is when the same > network port must process IPsec and regular traffic. Should regular > traffic also use the singular device? > 3. The spec seems to assume PMD Network device. Event driven model is > also needed. > 4. SA Options for expiry(byte/time) are lacking. > 5. Error handling and Status notifications are not specified. These > can be tricky in the inline mode of operation, particularly inbound. > 6. SA expiry handling is another key aspect which hasn’t been > accounted for. > 7. No anti-replay window size SA param. > 8. ESP TFC padding not addressed. > 9. Incremental checksum computation in transport mode ESP doesnt > appear to be addressed > 10. Didnt spot details for tunnel mode header preservation > 11. Selector checking, especially for the inner packet in tunnel mode > appears to be missing > 12. Dynamic offloading - selectively offload some packets in hardware > is a feature we would like to support. > 13. Destination queue for IPSEC events: Operations in asynchronous or > inline mode enqueue resulting events into this queue. This helps with > our 93xx inline ipsec design. > 14. If event model (ASYNC) and inline are supported, there should be a > “pipeline” classifier option for inbound SAs. > 15. Maximum number of destination CoSes is not supported. The same CoS > may be used for many SAs. > 16. Per protocol header parsing capability after inbound processing > is missing. Preferred options : None/L2/L3/L4/ALL protocols. > 17. Per protocol outer header retention in inbound processing is > missing. Preferred options : None/L2/L3/L4/ALL protocols. > > Thanks > Prasad cc'ed the cavium stakeholders.