HI Fiona >-----Original Message----- >From: Trahe, Fiona [mailto:fiona.tr...@intel.com] >Sent: 15 February 2018 17:16 >To: Verma, Shally <shally.ve...@cavium.com>; dev@dpdk.org; Athreya, Narayana >Prasad <narayanaprasad.athr...@cavium.com>; >Murthy, Nidadavolu <nidadavolu.mur...@cavium.com>; Sahu, Sunila ><sunila.s...@cavium.com>; Gupta, Ashish ><ashish.gu...@cavium.com>; Doherty, Declan <declan.dohe...@intel.com>; >Keating, Brian A <brian.a.keat...@intel.com>; Griffin, >John <john.grif...@intel.com> >Cc: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>; Trahe, Fiona ><fiona.tr...@intel.com> >Subject: RE: [dpdk-dev] [RFC v1 1/1] lib/cryptodev: add support of asymmetric >crypto > >Hi Shally, > >> -----Original Message----- >> From: Verma, Shally [mailto:shally.ve...@cavium.com] >> Sent: Thursday, February 15, 2018 5:33 AM >> To: Trahe, Fiona <fiona.tr...@intel.com>; dev@dpdk.org; Athreya, Narayana >> Prasad >> <narayanaprasad.athr...@cavium.com>; Murthy, Nidadavolu >> <nidadavolu.mur...@cavium.com>; >> Sahu, Sunila <sunila.s...@cavium.com>; Gupta, Ashish >> <ashish.gu...@cavium.com>; Doherty, Declan >> <declan.dohe...@intel.com>; Keating, Brian A <brian.a.keat...@intel.com>; >> Griffin, John >> <john.grif...@intel.com> >> Cc: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com> >> Subject: RE: [dpdk-dev] [RFC v1 1/1] lib/cryptodev: add support of >> asymmetric crypto >> >> HI Fiona >> >> Thanks for your feedback. Response below. >> >> >-----Original Message----- >> >From: Trahe, Fiona [mailto:fiona.tr...@intel.com] >> >Sent: 09 February 2018 23:43 >> >To: dev@dpdk.org; Athreya, Narayana Prasad >> ><narayanaprasad.athr...@cavium.com>; Murthy, >> Nidadavolu >> ><nidadavolu.mur...@cavium.com>; Sahu, Sunila <sunila.s...@cavium.com>; >> >Gupta, Ashish >> <ashish.gu...@cavium.com>; Verma, >> >Shally <shally.ve...@cavium.com>; Doherty, Declan >> ><declan.dohe...@intel.com>; Keating, Brian A >> <brian.a.keat...@intel.com>; >> >Griffin, John <john.grif...@intel.com> >> >Cc: Trahe, Fiona <fiona.tr...@intel.com>; De Lara Guarch, Pablo >> ><pablo.de.lara.gua...@intel.com> >> >Subject: RE: [dpdk-dev] [RFC v1 1/1] lib/cryptodev: add support of >> >asymmetric crypto >> > >> >Hi Shally, >> >Comments below. >> > >> >> -----Original Message----- >> >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shally Verma >> >> Sent: Tuesday, January 23, 2018 9:54 AM >> >> To: Doherty, Declan <declan.dohe...@intel.com> >> >> Cc: dev@dpdk.org; pathr...@caviumnetworks.com; nmur...@caviumnetworks.com; >> >> ss...@caviumnetworks.com; agu...@caviumnetworks.com; Shally Verma >> >> <sve...@caviumnetworks.com> >> >> Subject: [dpdk-dev] [RFC v1 1/1] lib/cryptodev: add support of asymmetric >> >> crypto >> >> >> >> From: Shally Verma <sve...@caviumnetworks.com> >> >> >> >> Add support for asymmetric crypto operations in DPDK lib cryptodev >> >> >> >> Key feature include: >> >> - Only session based asymmetric crypto operations >> >> - new get and set APIs for symmetric and asymmetric session private >> >> data and other informations >> >> - APIs to create, configure and attch queue pair to asymmetric sessions >> >> - new capabilities in struct device_info to indicate >> >> -- number of dedicated queue pairs available for symmetric and >> >> asymmetric operations, if any >> >> -- number of asymmetric sessions possible per qp >> >> >> >[Fiona] Though it's probably premature to include on the API have you >> >considered >> >providing for pre-loaded keys in future, i.e. the ability to wrap keys or >> >refer to keys >> >already stored securely on the device, as an alternative to passing the >> >keys on the API? >> > >> [Shally] Current intention of DPDK asym spec is to expose HW capabilities to >> application to offload these >> compute intensive ops. >> Though your use-case is very much a practical requirement , but to achieve >> this I believe we need a layer >> above this spec and need different interfaces. Such as, some public/secure >> key library which provide >> interfaces to generate / store/ perform op using an opaque key handles which >> internally can use DPDK >> to do part of it. For such case, I envision dpdk (or library above) will >> likely run on some secure processor. >> Thus, currently I kept such use-case out of current spec scope. >[Fiona] ok > >> > >> >> Proposed asymmetric cryptographic operations are: >> >> - rsa >> >> - dsa >> >> - deffie-hellman key pair generation and shared key computation >> >> - ecdeffie-hellman >> >> - fundamental elliptic curve operations >> >> - elliptic curve DSA >> >> - modular exponentiation and inversion >> >> >> >> This patch primarily defines PMD operations and device capabilities >> >> to perform asymmetric crypto ops on queue pairs and intend to >> >> invite feedbacks on current proposal so as to ensure it encompass >> >> all kind of crypto devices with different capabilities and queue >> >> pair management. >> >> >> >> List of TBDs: >> >> - Currently, patch only updated for RSA xform and associated params. >> >> Other algoritms to be added in subsequent versions. >> >> - per-service stats update >> >> >> >> Signed-off-by: Shally Verma <sve...@caviumnetworks.com> >> >> --- >> >> >> >> It is derivative of RFC v2 asymmetric crypto patch series initiated by >> >> Umesh Kartha(mailto:umesh.kar...@caviumnetworks.com): >> >> >> >> http://dpdk.org/dev/patchwork/patch/24245/ >> >> http://dpdk.org/dev/patchwork/patch/24246/ >> >> http://dpdk.org/dev/patchwork/patch/24247/ >> >> >> >> And inclusive of all review comments given on RFC v2. >> >> ( See complete discussion thread here: >> >> http://dev.dpdk.narkive.com/yqTFFLHw/dpdk-dev-rfc-specifications-for-asymmetric-crypto- >> >> algorithms#post12) >> >> >> >> Some of the RFCv2 Review comments pending for closure: >> >> > " [Fiona] The count fn isn't used at all for sym - probably no need to >> >> > add for asym >> >> better instead to remove the sym fn." >> >> >> >> It is still present in dpdk-next-crypto for sym, so what has been >> >> decision >> >> on it? >> >[Fiona] No change. The rte_cryptodev_ops fn is still not called so useless >> >and should be removed. >> >rte_cryptodev_queue_pair_count() returns the num_qps configured in >> >rte_cryptodev_configure(), but never calls the PMD >> >dev_ops.queue_pair_count(). >> >So cryptodev_sym_queue_pair_count_t should be deprecated. >> >And no point in adding one for asym. >> > >> >> [Shally] Ok >> >> >> >> >> >"[Fiona] if each qp can handle only a specific service, i.e. a subset >> >> >off the capabilities >> >> Indicated by the device capability list, there's a need for a new API >> >> to query >> >> the capability of a qp." >> >> >> >> Current proposal doesn’t distinguish between device capability and qp >> >> capability. >> >> It rather leave such differences handling internal to PMDs. Thus no >> >> capability >> >> or API added for qp in current version. It is subject to revisit >> >> based on review >> >> feedback on current proposal. >> >[Fiona] This would not work for some devices, comments below. >> > >> >> >> >> - Sessionless Support. >> >> Current proposal only support Session-based because: >> >> 1. All one-time setup i.e. algos and associated params, such as, >> >> public-private keys >> >> or modulus length can be done in control path using session-init >> >> API >> >> 2. it’s an easier way to dedicate qp to do specific service (using >> >> queue_pair_attach()) >> >> which cannot be case in sessionless >> >> 3. Couldn’t find any significant advantage going sessionless way. >> >> Also existing most of PMDs are >> >> session-based. >> >> >> >> It could be added in subsequent versions, if requirement is >> >> identified, based on review comment >> >> on this RFC. >> >[Fiona] Our preference would be for sessionless, as it would need fewer API >> >calls (no >> session_create/session_clear) >> >and e.g. DH and ECDH sessions are likely to be only used for a single op. >> >However this is not a blocker >> for >> >this API, we can POC it later and propose an extension to the API if it >> >gives a performance >> improvement. >> > >> [Shally] You mean you recommend to have session-less support along with >> session-based? >[Fiona] Yes. But as stated, it's ok to start without it and add later. > > >> >> >> >> Summary >> >> ------- >> >> >> >> This section provides an overview of key feature enabled in current >> >> specification. >> >> It comprise of key design challenges as have been identified on RFCv2 and >> >> summary description of new interfaces and definitions added to handle >> >> same. >> >> >> >> Description >> >> ----------- >> >> >> >> This API set assumes that the max_nb_queue_pairs on a >> >> device can be allocated to any mix of sym or asym. Some devices >> >> may have a fixed max per service. Thus, rte_cryptodev_info >> >> is updated with max_sym_nb_queues and max_asym_nb_queues with rule: >> >> >> >> max_nb_queue_pair = max_nb_sym_qp + max_nb_asym_qp. >> >> >> >> If device has no restrictions on qp to be used per service, such PMDs can >> >> leave >> >> max_nb_sym_qp = max_nb_asym_qp = 0. In such case, application can setup >> >> any of >> >> the service upto limit defined by max_nb_queue_pair. >> >[Fiona] This seems awkward if a genuine 0 e.g. An appl wants to set up a >> >sym qp, and >> >sees that max_nb_queue_pair=4. Can't trust this - needs to look deeper. Sees >> >max_nb_sym_qp=0. Doesn't know if that means it can set up 4 sym qps or no >> >qps >> >until it also checks max_nb_asym_qp. If=4 then it can't set up any sym qp, >> >if=0 it can set up 4. >> >Would it be better if >> > max_nb_queue_pair = max(max_nb_sym_qp, max_nb_asym_qp). >> > max_nb_sym_qp = 0..max_nb_queue_pair >> > max_nb_asym_qp = 0..max_nb_queue_pair >> > >> >> [Shally] Ok. I can change definition to replace 0 by max num for no-limit >> case with slight modification to >> the condition i.e. >> max_nb_queue_pair >= max(max_nb_sym_qp, max_nb_asym_qp) >> >> because device can have distribution of (max_nb_queue_pair, max_nb_sym_qp, >> max_nb_asym_qp) as >> (4,4,4) or (4,3,1) >> where total number of qp setup by application cannot exceed >> max_nb_queue_pairs. >> >> >> Here, max_nb_sym_qp and max_nb_asym_qp, if non-zero, just define limit on >> >> qp which are >> >> available for each service and *are not* ids to be used during qp setup >> >> and enqueue i.e. >> >> if device supports both symmetric and asymmetric with n qp, then any of >> >> them >> >> can be configured for symmetric or asymmetric subject to limit defined by >> >> max_nb_sym_qp and max_nb_asym_qp. For example, >> >> if device has 6 queues and 5 for symmetric and 1 for asymmetric that imply >> >> application can setup only 1 out of any 6 qp for asymmetric and rest for >> >> symmetric. >> >> >> >> Additionally, application can dedicate qp to perform specific service via >> >> optional >> >> queue_pair_attach_sym/asym_session() API. >> >[Fiona] This is too late - some devices have qps which need to know at >> >setup which >> >service they'll be used for, as they reserve resources based on the service. >> >e.g. a device reports in info that it can support qps(max 4, max_sym 2, max >> >asym 2) >> >An application just wants to use 1 sym qp and 1 asym, so configures device >> >with >> >qps(max 2, max_sym 1, max asym 1) >> >Now appl calls qp_setup for qp index 0. Wants to start with running >> >asymmetric >> >service and will set up the symmetric later, maybe in a separate thread. >> >Our devices need to know at setup time which service will be used on the qp. >> >So haven't been given enough information to setup the qp. >> > >> >Even if the PMD could set up service-agnostic qps and just map later, you >> >suggest the qp_attach_sym/asym_session() would be optional. How could the >> >appl >> >make the decision whether to call it or not in above case? It needs to know >> >whether >> >the created qp is agnostic or not, so a capability must be reported by the >> >PMD. >> >OR the API should not be optional, must be called for every session. >> >But that's perf impacting and not helpful if a device could support >> >service-agnostic qps. >> > >> >I don't have a solution for service-agnostic qps. For our devices that's >> >not necessary. >> >Is it likely that anyone wants to set up a qp to process both sym and asym >> >ops? >> >Is it sufficient to accommodate qps which can support either >> >service, but one must be picked at setup after which the qp supports only >> >that service? >> >How about the following - : >> > - device reports in info qps(max, max_sym, max asym) where >> > max = max(max_sym, max_asym). >> > max_sym = 0..max >> > max_asym = 0..max >> > So if a device reports 4,2,2, it has 4 total, 2 can be setup for sym, 2 >> > for asym >> > Or if it reports 4,4,4, all 4 of them can be setup as sym or all 4 as >> > asym or a mix, but total setup can't be more than 4. >> > - appl configures qps(max, max_sym, max_asym) >> > If it specifies 4,1,3 it wants to use 4 in total, 1 for sym, 3 for asym. >> > If it specifies 1,1,0 it wants to use 1 in total, 1 for sym, 0 for asym. >> > The numbers it configures must be <= those reported in info. >> > - appl calls qp_setup(dev, qp, service) where service = sym or asym >> > qp index must be between 0..max-1 >> > The PMD maps that index to an internal qp which can support the >> > requested service. >> > If qp_setup is called on an index and the max configured qps have >> > already >> > been setup for that service then the setup fails. >> >In case the appl forgets which qp it set up for which service, I think it >> >would be necessary to add a new >> API >> > rte_cryptodev_qp_info() which would return what service is configured >> > on the qp >> > >> >If it's necessary to support service-agnostic qps, then one way would be to >> >specify a third service above >> as >> > - sym+asym. >> >And have devices report max_sym_asym_qps? >> > >> >It's difficult to find a solution for this without breaking the current API >> >- see simpler >> >alternative below. >> > >> >> Except the ones mentioned above, specification doesn't define any >> >> restrictions on >> >> enqueue/dequeue API for different crypto services. Application can >> >> enqueue both symmetric >> >> and asymmetric operations to same device and qp if device info indicate >> >> support of both >> >> symmetric and asymmetric and qp is not dedicated to any. However in >> >> practice, it could >> >> result in head-of-line blocking due to the asym ops taking longer than >> >> the symmetric ops. >> >> >> >> Some HW accelerators avoid this issue by supporting sym and asym ops >> >> on different qps on the same device. Such devices can set max_nb_sym_qp >> >> and >> >> max_nb_asym_qp to number of queues available for each respective service. >> >> >> >> This may give problem in indexing on enqueue/dequeue i.e. >> >> if 2 asym qps and 2 sym qps are created on the same device, >> >> the queue_pair_ids are duplicated. To handle such scenario, >> >> 2 alternatives are proposed for PMD design: >> >> >> >> 1. Allow all qp to input any operation type virtually. Say, >> >> >> >> Consider device with symmetric and asymmetric capability having >> >> max_nb_queue_pair as 6, then by design, all 6 are capable to input >> >> both op types. >> >> >> >> If PMD uses fixed set of qp, then internally, can map qp id to >> >> actual physical qp id to be used according to session/op type. >> >> If such mapping is done on data path, may impact performance of PMDs. >> >> Thus, to handle such mapping out of data path, library provides an API >> >> queue_pair_attach_sym/asym_session() which facilitate application to >> >> dedicate qp id to perform specific (sym/asym) service in control path >> >> and should be called before 1st call to enqueue_burst(). >> >> Here PMD can bind app provided logical qp id to actual physical qp id >> >> to use >> >> depending on session type , OR alternatively >> >> >> >> 2. PMDs can split it into two PMDs, one with symmetric ONLY capability >> >> and other >> >> with asymmetric ONLY capability (also proposed by Fiona in RFCv2). >> >> This may require some changes to EAL to facilitate pci device >> >> sharing between >> >> PMDs, enabling separation of sym and asym services onto different >> >> PMDs. >> >> As per RFCv2 feedback, Intel proposed to submit a patch to enable >> >> this. >> >[Fiona] The eal changes were problematic, and feedback from the Dublin >> >userspace >> >event last September was that several others have tried similar but >> >reverted to resolving >> >the problem within the PMD. This is what we are also in the process of >> >doing. >> >With this we intend to avoid all the above complications. >> >i.e. one pci device, in its probe fn, can create multiple API device >> >instances. >> >One instance of cryptodev will provide a sym crypto service, one instance of >> >compressdev will provide a compression service and a second cryptodev >> >instance will >> >provide the asymmetric service. All qps on each API device instance can >> >then support all >> >of the capabilities reported by that device. >> > >> >A suggestion for keeping the simplicity on the API would be to follow the >> >same pattern. >> >i.e. don't provide any support for specification of sym or asym qps. Expect >> >all >> >qps on each device to support all capabilities reported. >> > >> > >> >> [Shally] Initially I had thought of same proposal to add an input field in >> qp_conf so that PMD can know >> at qp_setup time which service it going to be used for. >> However looking that spec already had qp_attach_sym/asym APIs and also the >> possibility that some >> device can support service - agnostic qp, I initiated with current proposal. >> I marked them optional with a disclaimer that , if not attached, then PMD >> will then need to map logical >> qpid to physical during enqueue_burst() which might result in impacted >> performance. >> This is particularly applicable to devices which internally have fixed qps >> per each service and set >> max_nb_sym/asym_qp to indicate same. >> And thus for such devices, I primarily recommended that they should appear >> as two separate PMDs >> where one support sym and other asym. >> However, only point to is to see how well suited is this for each device >> driver design as it might rather >> turn to be simpler for some implementation to just have single way to >> dedicate a qp to a service via >> attach or qp_setup change than having two separate instances. >> >> But, I concur from your thoughts. We can start with simple definition and >> remove concept of >> max_nb_sym_qp and max_nb_asym_qp in first version and say >> -if device support service agnostic qp, that means all of its qp can be set >> up for any service. PMD can >> show in its feature flag support for symmetric and asymmetric crypto. >> Application can set up qp the way >> it want, spec define no recommended way of usage here. >[Fiona] I would word this differently, as no mechanism is provided for an appl >to setup a qp for a specific service. >If PMD shows in its feature flag that it supports both sym and asym then it >must support those on all its qps. >Application may choose to direct all asym ops to one qp and all asym to a >different qp to avoid >head-of-line blocking, but there is nothing in the qp setup or capabilities to >prevent the application >from sending an enqueue_burst() with a mix of asym and sym ops all to the same >qp and it should work. >
[Shally] Clear. >> -if device has fixed set of qp per each service, then it should split itself >> into two : symmetric only and >> asymmetric only. >[Fiona] For us this is ok, but it would be good to get feedback from other >providers of hw acceleration >as it is quite a constraint. > [Shally] That's the intention but until then I assume we agree to current proposal?! If yes, then just to align and close on our discussion on this here’s the summary : - There's no differentiation to device capability and qp capability. If PMD shows in its feature flag that it supports both sym and asym then it must support those on all its qps. - if PMD support both but internally has hw with dedicated qp for each service then it *can* split itself into two: symmetric only and asymmetric only PMD instances. - Currently we don’t see a requirement to dedicate qp to a service, thus no need for max_nb_sym/asym_qp or service_type info during qp_setup. However this is open for review and discussion and can be added later, if any requirement is identified for same. Is that correct? Thanks Shally >> This is well suited for session-less also. IMO, If any issues reported with >> these rules, then we can add >> this support later based on the then identified requirement. However, if we >> end up adding the support >> back of max_nb_sym/asym_qp in spec later, then attach/detach_qp APIs will >> not be sufficient as they >> are session specific. Given that, having an input param during qp_setup() is >> more generic way that suit >> both session and sessionless way. Does this all sounds right? >[Fiona] agreed. > >> >> I will wait to close on this design discussion first before giving feedback >> to rest of the comment further >> below. But consider all feedback regarding typo and licensing as >> acknowledged. >> >> Thanks >> Shally >>
