From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DD39C49ED6 for ; Wed, 11 Sep 2019 13:01:41 +0000 (UTC) Received: from dpdk.org (dpdk.org [92.243.14.124]) by mail.kernel.org (Postfix) with ESMTP id EA9C7206A1 for ; Wed, 11 Sep 2019 13:01:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EA9C7206A1 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B748D1E545; Wed, 11 Sep 2019 15:01:39 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id B21711D451 for ; Wed, 11 Sep 2019 15:01:37 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Sep 2019 06:01:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,493,1559545200"; d="scan'208";a="214675302" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga002.fm.intel.com with ESMTP; 11 Sep 2019 06:01:35 -0700 Received: from irsmsx111.ger.corp.intel.com (10.108.20.4) by IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 11 Sep 2019 14:01:34 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.164]) by irsmsx111.ger.corp.intel.com ([169.254.2.112]) with mapi id 14.03.0439.000; Wed, 11 Sep 2019 14:01:34 +0100 From: "Ananyev, Konstantin" To: Akhil Goyal , "Zhang, Roy Fan" , "dev@dpdk.org" CC: "Doherty, Declan" , "De Lara Guarch, Pablo" Thread-Topic: [RFC PATCH 1/9] security: introduce CPU Crypto action type and API Thread-Index: AQHVYm4Y+AedzaNgY0qWMAmu5GwNVqcbQpEAgAArCICAAuADAIAARjqAgAYrbACAAbWwQA== Date: Wed, 11 Sep 2019 13:01:33 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580191962D34@irsmsx105.ger.corp.intel.com> References: <20190903154046.55992-1-roy.fan.zhang@intel.com> <20190903154046.55992-2-roy.fan.zhang@intel.com> <9F7182E3F746AB4EA17801C148F3C6043369D686@IRSMSX101.ger.corp.intel.com> <9F7182E3F746AB4EA17801C148F3C604336AF46E@IRSMSX101.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzVhNmQ2NDQtYTQ1ZS00NWI4LWI2MDYtMmY5MmIyZDM5NGIwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiSll2YU85bHVqQXM0T1dwK0FWaysxNzc4WkRRbHFWZXB3V08wNTVwUFM2cEowUHdxM0NrclRcL0YrSDZwTlFQUzYifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC PATCH 1/9] security: introduce CPU Crypto action type and API 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi lads, > > > > You are right, the new API will process the crypto workload, no heavy e= nqueue > > Dequeue operations required. > > > > Cryptodev tends to support multiple crypto devices, including HW and SW= . > > The 3-cache line access, iova address computation and assignment, simul= ation > > of async enqueue/dequeue operations, allocate and free crypto ops, even= the > > mbuf linked-list for scatter-gather buffers are too heavy for SW crypto= PMDs. >=20 > Why cant we have a cryptodev synchronous API which work on plain bufs as = your suggested > API and use the same crypto sym_session creation logic as it was before? = It will perform > same as it is doing in this series. I tried to summarize our reasons in another mail in that thread. >=20 > > > > To create this new synchronous API in cryptodev cannot avoid the proble= m > > listed above: first the API shall not serve only to part of the crypto= (SW) PMDs - > > as you know, it is Cryptodev. The users can expect some PMD only suppor= t part > > of the overall algorithms, but not the workload processing API. >=20 > Why cant we have an optional data path in cryptodev for synchronous behav= ior if the > underlying PMD support it. It depends on the PMD to decide whether it can= have it supported or not. > Only a feature flag will be needed to decide that. > One more option could be a PMD API which the application can directly cal= l if the > mode is only supported in very few PMDs. This could be a backup if there = is a > requirement of deprecation notice etc. >=20 > > > > Another reason is, there is assumption made, first when creating a cryp= to op > > we have to allocate the memory to hold crypto op + sym op + iv, - we ca= nnot > > simply declare an array of crypto ops in the run-time and discard it wh= en > > processing > > is done. Also we need to fill aad and digest HW address, which is not r= equired for > > SW at all. >=20 > We are defining a new API which may have its own parameters and requireme= nts which > Need to be fulfilled. In case it was a rte_security API, then also you ar= e defining a new way > Of packet execution and API params. So it would be same. > You can reduce the cache line accesses as you need in the new API. > The session logic need not be changed from crypto session to security ses= sion. > Only the data patch need to be altered as per the new API. >=20 > > > > Bottom line: using crypto op will still have 3 cache-line access perfor= mance > > problem. > > > > So if we to create the new API in Cryptodev instead of rte_security, we= need to > > create new crypto op structure only for the SW PMDs, carefully document= them > > to not confuse with existing cryptodev APIs, make new device feature fl= ags to > > indicate the API is not supported by some PMDs, and again carefully doc= ument > > them of these device feature flags. >=20 > The explanation of the new API will also happen in case it is a security = API. Instead you need > to add more explanation for session also which is already there in crypto= dev. >=20 > > > > So, to push these changes to rte_security instead the above problem can= be > > resolved, > > and the performance improvement because of this change is big for small= er > > packets > > - I attached a performance test app in the patchset. >=20 > I believe there wont be any perf gap in case the optimized new cryptodev = API is used. >=20 > > > > For rte_security, we already have inline-crypto type that works quite c= lose to the > > this > > new API, the only difference is that it is processed by the CPU cycles.= As you may > > have already seen the ipsec-library has wrapped these changes, and ipse= c-secgw > > has only minimum updates to adopt this change too. So to the end user, = if they > > use IPSec this patchset can seamlessly enabled with just commandline up= date > > when > > creating an SA. >=20 > In the IPSec application I do not see the changes wrt the new execution A= PI. > So the data path is not getting handled there. It looks incomplete. The u= ser experience > to use the new API will definitely be changed. I believe we do support it for libtre_ipsec mode. librte_ipsec hides all processing complexity inside and does call rte_security_process_cpu_crypto_bulk() internally. That's why for librte_ipsec it is literally 2 lines change: --- a/examples/ipsec-secgw/ipsec_process.c +++ b/examples/ipsec-secgw/ipsec_process.c @@ -101,7 +101,8 @@ fill_ipsec_session(struct rte_ipsec_session *ss, struc= t ipsec_ctx *ctx, } ss->crypto.ses =3D sa->crypto_session; /* setup session action type */ - } else if (sa->type =3D=3D RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) { + } else if (sa->type =3D=3D RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL || + sa->type =3D=3D RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO) { if (sa->sec_session =3D=3D NULL) { rc =3D create_lookaside_session(ctx, sa); if (rc !=3D 0) @@ -227,8 +228,8 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traf= fic *trf) =20 /* process packets inline */ else if (sa->type =3D=3D RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO || - sa->type =3D=3D - RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) { + sa->type =3D=3D RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL || + sa->type =3D=3D RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO) { =20 satp =3D rte_ipsec_sa_type(ips->sa);