From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2A765A04C2; Thu, 14 Nov 2019 06:46:48 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9263B2BAF; Thu, 14 Nov 2019 06:46:46 +0100 (CET) Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by dpdk.org (Postfix) with ESMTP id 12F012B87; Thu, 14 Nov 2019 06:46:45 +0100 (CET) Received: by mail-io1-f65.google.com with SMTP id p6so5436041iod.7; Wed, 13 Nov 2019 21:46:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6EZrtxrKvYXRvli3M1t4xZusjTj/DkZll32Bn2hfifE=; b=CFlQ0JkZalPlLRXXz9qob1lO9Q3L2FH9s4+9iK9yfvtN4DBSxQCaVkNoRJ/bGgs5x/ WZjcxPE3IYl1zcIWrmxg5j1HgRqNs/YV9arZfzOnBATHkIGN9xK6koao2gt1JQirwQoQ OMBfgCB1yO4asSfRbySz4A/ofdEiE1PgzykIBnSzhneZAggpXHf4/crwwUl4Umy2/tFF +oWgfpFYbdpkN1ym1wFFCL41nC0nmPsBnbooBosDqj+nBafi2wIHQ6Sq0bzhgd3xNOnb MbVcKDsvejBcu5lX+SYqprGbXJMz3d7Yc5C/KFvZYir3Od/Ub9KLzu+w8fOUqnPV/Vy0 hIMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=6EZrtxrKvYXRvli3M1t4xZusjTj/DkZll32Bn2hfifE=; b=kV8vumejzb83zd7mPCQIEdMahKC3pR0h7l+cX2Wb/cXinCOgsZy1mEUWEsBy7Xesen A8CdnceDpYytloR9VvIXtXdP3vfxIkjzvDRLKyYiurdQKefrHXEi+kv0ROtozXQWxyhO DcLeWHgCghhlD9oTulNpa0t+Ue+2wuNMqpbd0kKHTexngGseoGFGBKbJlneVQce8JtW7 onZhgBCPxy2b6YuDMemZb3W1xRtTAmaeqihn6F5UY1v7o2e5v1bbqYvZYt9uEhDdv0tR r6UTtVZsStJpBDh0ke4PUBozynL/Ip3qQqUSMVCDQ0ZuN3Ktcu6CjD2PpVY7zGgtjrNw DWyQ== X-Gm-Message-State: APjAAAWgruqG09XQPhPEWvzZLjv8uKbw4Cq/4hW2wb498rOkZvfVFSYP wkqy1vZujLe7pCk0X0ColxyFVDQfM/bkYwawBAY= X-Google-Smtp-Source: APXvYqwAVOlR9/AM7yrXreACtGDgb5AQOVXfBn5oNQ7xiuB+LFe7YhkORCHIyBHdOs9LxeYhpK0pYim6PG2/XLjtKIk= X-Received: by 2002:a5d:8141:: with SMTP id f1mr7096979ioo.162.1573710404044; Wed, 13 Nov 2019 21:46:44 -0800 (PST) MIME-Version: 1.0 References: <20191105184122.15172-1-konstantin.ananyev@intel.com> In-Reply-To: <20191105184122.15172-1-konstantin.ananyev@intel.com> From: Jerin Jacob Date: Thu, 14 Nov 2019 11:16:27 +0530 Message-ID: To: Konstantin Ananyev Cc: dpdk-dev , techboard@dpdk.org, roy.fan.zhang@intel.com, declan.doherty@intel.com, Akhil Goyal Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [RFC 0/4] cpu-crypto API choices 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" On Wed, Nov 6, 2019 at 12:11 AM Konstantin Ananyev wrote: > > Originally both SW and HW crypto PMDs use rte_crypot_op based API to > process the crypto workload asynchronously. This way provides uniformity > to both PMD types, but also introduce unnecessary performance penalty to > SW PMDs that have to "simulate" HW async behavior > (crypto-ops enqueue/dequeue, HW addresses computations, > storing/dereferencing user provided data (mbuf) for each crypto-op, > etc). > > The aim is to introduce a new optional API for SW crypto-devices > to perform crypto processing in a synchronous manner. > As summarized by Akhil, we need a synchronous API to perform crypto > operations on raw data using SW PMDs, that provides: > - no crypto-ops. > - avoid using mbufs inside this API, use raw data buffers instead. > - no separate enqueue-dequeue, only single process() API for data path. > - input data buffers should be grouped by session, > i.e. each process() call takes one session and group of input buffers > that belong to that session. > - All parameters that are constant accross session, should be stored > inside the session itself and reused by all incoming data buffers. > > While there seems no controversy about need of such functionality, > there seems to be no agreement on what would be the best API for that. > So I am requesting for TB input on that matter. > > Series structure: > - patch #1 - intorduce basic data structures to be used by sync API > (no controversy here, I hope ..) > [RFC 1/4] cpu-crypto: Introduce basic data structures > - patch #2 - Intel initial approach for new API (via rte_security) > [RFC 2/4] security: introduce cpu-crypto API > - patch #3 - approach that reuses existing rte_cryptodev API as much as > possible > [RFC 3/4] cryptodev: introduce cpu-crypto API > - patch #4 - approach via introducing new session data structure and API > [RFC 4/4] cryptodev: introduce rte_crypto_cpu_sym_session API > > Patches 2,3,4 are mutually exclusive, > and we probably have to choose which one to go forward with. > I put some explanations in each of the patches, hopefully that will help > to understand pros and cons of each one. > > Akhil strongly supports #3, AFAIK mainly because it allows PMDs to > reuse existing API and minimize API level changes. > My favorite is #4, #2 is less preferable but ok too. > #3 seems problematic to me by the reasons I outlined in #4 patch > description. > > Please provide your opinion. I spend some time on the proposal and I agree that sync API is needed and it makes sense to remove queue emulation and allocating/freeing the crypto_ops in case of sync API. # I would prefer to not duplicate the session. If the newly added fields are for optimization then those can be applicable for HW too. For example, if we consider, offset to be constant for one session HW PMD will be able to leverage this. ref: rte_crypto_aead_xfrom::cpu_crypto:offset # I would prefer to not duplicate ops parameters, instead of the existing rte_crypto_ops can be updated. I see that most members introduced in rte_crypto_sym_vec & rte_crypto_vec are already existing in rte_crypto_op. Also, since we are agreeing that the ops for SYNC API can be from stack/one time allocated, the size shouldn't matter. I understand that this would cause ABI breakage, but for this release, we can work together and add some reserved fields that we can implement later. I believe that's the reason why you want to introduce new structures. I think that will bloat the existing crypto lib. If I understand it correctly, this will be used in conjunction with IXGBE to handle fragmented IPsec traffic. If that's the fundamental reasoning, then there is an alternate path possible. Currently, the issue is, rte_security doesn't define the treatment for fragmented packets. Maybe let's define it and then a similar CPU crypto processing can be done inside the PMD. By creating an internal function in S/W PMDs and calling it from the inline crypto enabled eth PMDs, fragmentation support for inline crypto devices can be achieved. This way the application would look clean. All the fragmentation related configuration (no of fragmentation contexts needed, reassembly timeout etc) need to be added in rte_security library and the result for that operation will come as dynamic fields in the mbuf. Just my 2c. > > Konstantin Ananyev (4): > cpu-crypto: Introduce basic data structures > security: introduce cpu-crypto API > cryptodev: introduce cpu-crypto API > cryptodev: introduce rte_crypto_cpu_sym_session API > > lib/librte_cryptodev/rte_crypto_sym.h | 63 +++++++++++++++++++++-- > lib/librte_cryptodev/rte_cryptodev.c | 14 +++++ > lib/librte_cryptodev/rte_cryptodev.h | 24 +++++++++ > lib/librte_cryptodev/rte_cryptodev_pmd.h | 22 ++++++++ > lib/librte_security/rte_security.c | 11 ++++ > lib/librte_security/rte_security.h | 28 +++++++++- > lib/librte_security/rte_security_driver.h | 20 +++++++ > 7 files changed, 177 insertions(+), 5 deletions(-) > > -- > 2.17.1 >