From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 663F82BF7 for ; Mon, 17 Sep 2018 20:13:10 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Sep 2018 11:13:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,386,1531810800"; d="scan'208";a="73722957" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by orsmga007.jf.intel.com with ESMTP; 17 Sep 2018 11:12:50 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.54]) by IRSMSX151.ger.corp.intel.com ([169.254.4.94]) with mapi id 14.03.0319.002; Mon, 17 Sep 2018 19:12:49 +0100 From: "Ananyev, Konstantin" To: Jerin Jacob , "Joseph, Anoob" CC: "dev@dpdk.org" , "Awal, Mohammad Abdul" , "Doherty, Declan" , Narayana Prasad Thread-Topic: [dpdk-dev] [RFC] ipsec: new library for IPsec data-path processing Thread-Index: AQHUO8sQ1vNhPIIy2kCq+ieD7nPPdqTefpOAgABfO7CAAuY7AIALGZsggAAAuDCABMYgAIABKxCAgAHAZSA= Date: Mon, 17 Sep 2018 18:12:48 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258EA95724C@irsmsx105.ger.corp.intel.com> References: <1535129598-27301-1-git-send-email-konstantin.ananyev@intel.com> <358d1b6c-26f2-b125-07a4-cfb1c0e2a57b@caviumnetworks.com> <2601191342CEEE43887BDE71AB977258EA95089D@irsmsx105.ger.corp.intel.com> <475cf471-b46a-671a-5485-0042c652430c@caviumnetworks.com> <2601191342CEEE43887BDE71AB977258EA954BAD@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258EA954E9D@irsmsx105.ger.corp.intel.com> <20180916105640.GA4803@jerin> In-Reply-To: <20180916105640.GA4803@jerin> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzIzNjI1NmMtODllNS00MWFkLWFiMjgtZWI5NjVhYzIxYmYxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiN0RxY01HSjVvVE03NEpJZW5mVE9kaEpIczNOTzh4M2NuOGs2Y0RYK0Nyb3J6cll0dHk4bmlmd0g0b3ZsZEZsQiJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC] ipsec: new library for IPsec data-path processing 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: Mon, 17 Sep 2018 18:13:11 -0000 Hi Jerin, > > > > > > > > Anyway, let's pretend we found some smart way to distribute inbound p= ackets for the same SA to multiple HW queues/CPU > cores. > > > To make ipsec processing for such case to work correctly just atomici= ty on check/update segn/replay_window is not enough. > > > I think it would require some extra synchronization: > > > make sure that we do final packet processing (seq check/update) at th= e same order as we received the packets > > > (packets entered ipsec processing). > > > I don't really like to introduce such heavy mechanisms on SA level, = after all it supposed to be light and simple. > > > Though we plan CTX level API to support such scenario. > > > What I think would be useful addition for SA level API - have an abil= ity to do one update seqn/replay_window and multiple checks > concurrently. > > > > > > > In case of ingress also, the same problem exists. We will not be ab= le to use RSS and spread the traffic to multiple cores. > Considering > > > > IPsec being CPU intensive, this would limit the net output of the c= hip. > > > That's true - but from other side implementation can offload heavy pa= rt > > > (encrypt/decrypt, auth) to special HW (cryptodev). > > > In that case single core might be enough for SA and extra synchroniza= tion would just slowdown things. > > > That's why I think it should be configurable what behavior (ST or MT= ) to use. > > I do agree that these are the issues that we need to address to make th= e > > library MT safe. Whether the extra synchronization would slow down thin= gs is > > a very subjective question and will heavily depend on the platform. The > > library should have enough provisions to be able to support MT without > > causing overheads to ST. Right now, the library assumes ST. >=20 >=20 > I agree with Anoob here. >=20 > I have two concerns with librte_ipsec as a separate library >=20 > 1) There is an overlap with rte_security and new proposed library. I don't think there really is an overlap. rte_security is a 'framework for management and provisioning of security pr= otocol operations offloaded to hardware based devices'. While rte_ipsec is aimed to be a library for IPsec data-path processing. There is no plans for rte_ipsec to 'obsolete' rte_security. Quite opposite rte_ipsec supposed to work with both rte_cryptodev and rte_s= ecurity APIs (devices). It is possible to have an SA that would use both crypto and security devic= es. Or to have an SA that would use multiple crypto devs (though right now it is up the user level to do load-balancing logic).=20 > For IPsec, If an application needs to use rte_security for HW > implementation and and application needs to use librte_ipsec for > SW implementation then it is bad and a lot duplication of work on > he slow path too. The plan is that application would need to use just rte_ipsec API for all d= ata-paths (HW/SW, lookaside/inline).=20 Let say right now there is rte_ipsec_inline_process() function if user prefers to use inline security device to process given group packets, and rte_ipsec_crypto_process(/prepare) if user decides to use lookaside security or simple crypto device for it. >=20 > The rte_security spec can support both inline and look-aside IPSec > protocol support. AFAIK right now rte_security just provides API to create/free/manipulate se= curity sessions. I don't see how it can support all the functionality mentioned above, plus SAD and SPD. >=20 > 2) This library is tuned for fat CPU core in mind like single SA on core > etc. Which is fine for x86 servers and arm64 server category of machines > but it does not work very well with NPU class of SoC or FPGA. >=20 > As there are the different ways to implement the IPSec, For instance, > use of eventdev can help in situation for handling millions of SA and > equence number of update and anti reply check can be done by leveraging > some of the HW specific features like > ORDERED, ATOMIC schedule type(mapped as eventdev feature)in HW with PIPEL= INE model. >=20 > # Issues with having one SA one core, > - In the outbound side, there could be multiple flows using the same SA. > Multiple flows could be processed parallel on different lcores, > but tying one SA to one core would mean we won't be able to do that. >=20 > - In the inbound side, we will have a fat flow hitting one core. If > IPsec library assumes single core, we will not be able to to spread > fat flow to multiple cores. And one SA-one core would mean all ports on > which we would expect IPsec traffic has to be handled by that core. I suppose that all refers to the discussion about MT safe API for rte_ipsec= , right? If so, then as I said in my reply to Anoob:=20 We will try to make API usable in MT environment for v1, so you can review and provide comments at early stages. >=20 > I have made a simple presentation. This presentation details ONE WAY to > implement the IPSec with HW support on NPU. >=20 > https://docs.google.com/presentation/d/1e3IDf9R7ZQB8FN16Nvu7KINuLSWMdyKEw= 8_0H05rjj4/edit?usp=3Dsharing >=20 Thanks, quite helpful. Actually from page 3, it looks like your expectations don't contradict in g= eneral with proposed API: ... } else if (ev.event_type =3D=3D RTE_EVENT_TYPE_LCORE && ev.sub_event_id =3D= =3D APP_STATE_SEQ_UPDATE) { sa =3D ev.flow_queue_id; = =20 /* do critical section work per sa */ = =20 do_critical_section_work(sa); =20 [KA] that's the place where I expect either=20 rte_ipsec_inline_process(sa, ...); OR rte_ipsec_crypto_prepare(sa, ...); = =09 would be called. = =20 /* Issue the crypto request and generate the following= on crypto work completion */ [KA] that's the place where I expect rte_ipsec_crypto_process(...) be invok= ed. ev.flow_queue_id =3D tx_port; = =20 ev.sub_event_id =3D tx_queue_id; =20 ev.sched_sync =3D RTE_SCHED_SYNC_ATOMIC; = =20 rte_cryptodev_event_enqueue(cryptodev, ev.mbuf, eve= ntdev, ev); =20 } > I am not saying this should be the ONLY way to do as it does not work > very well with non NPU/FPGA class of SoC. >=20 > So how about making the proposed IPSec library as plugin/driver to > rte_security. As I mentioned above, I don't think that pushing whole IPSec data-path into= rte_security is the best possible approach. Though I probably understand your concern: In RFC code we always do whole prepare/process in SW (attach/remove ESP hea= ders/trailers, so paddings etc.), i.e. right now only device types: RTE_SECURITY_ACTION_TYPE_NONE and RTE_SEC= URITY_ACTION_TYPE_INLINE_CRYPTO are covered. Though there are devices where most of prepare/process can be done in HW (RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL/RTE_SECURITY_ACTION_TYPE_LOOKASID= E_PROTOCOL), plus in future could be devices where prepare/process would be split betwee= n HW/SW in a custom way. Is that so? To address that issue I suppose we can do: 1. Add support for RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL and RTE_SECURIT= Y_ACTION_TYPE_LOOKASIDE_PROTOCOL security devices into ipsec. We planned to do it anyway, just don't have it done yet. 2. For custom case - introduce RTE_SECURITY_ACTION_TYPE_INLINE_CUSTOM and R= TE_SECURITY_ACTION_TYPE_LOOKASIDE_CUSTOM and add into rte_security_ops new functions:=20 uint16_t lookaside_prepare(struct rte_security_session *sess, struct rt= e_mbuf *mb[], struct struct rte_crypto_op *cop[], uint16_t num); uint16_t lookaside_process(struct rte_security_session *sess, struct rt= e_mbuf *mb[], struct struct rte_crypto_op *cop[], uint16_t num); uint16_t inline_process(struct rte_security_session *sess, struct rte_m= buf *mb[], struct struct rte_crypto_op *cop[], uint16_t num); So for custom HW, PMD can overwrite normal prepare/process behavior. > This would give flexibly for each vendor/platform choose to different > IPse implementation based on HW support WITHOUT CHANGING THE APPLICATION > INTERFACE. Not sure what API changes you are referring to? As I am aware we do introduce new API, but all existing APIs remain in plac= e. >=20 > IMO, rte_security IPsec look aside support can be simply added by > creating the virtual crypto device(i.e move the proposed code to the virt= ual crypto device) > likewise inline support > can be added by the virtual ethdev device. That's probably possible and if someone would like to introduce such abstra= ction - NP in general (though my suspicion - it might be too heavy to be really useful). Though I don't think it should be the only possible way for the user to ena= ble IPsec data-processing inside his app. Again I guess such virtual-dev will still use rte_ipsec inside. > This would avoid the need for > updating ipsec-gw application as well i.e unified interface to applicatio= n. I think - it would really good to simplify existing ipsec-secgw sample app= . Some parts of it seems unnecessary complex to me. One of the reasons for it - we don't really have an unified (and transpare= nt) API for ipsec data-path. Let's look at ipsec_enqueue() and related code (examples/ipsec-secgw/ipsec.= c:365) It is huge (and ugly) - user has to handle dozen different cases just to e= nqueue packet for IPsec processing. One of the aims of rte_ipsec library - hide all that complexities inside th= e library and provide to the upper layer clean and transparent API. >=20 > If you don't like the above idea, any scheme of plugin based > implementation would be fine so that vendor or platform can choose its ow= n implementation. > It can be based on partial HW implement too. i.e SA look can be used in S= W, remaining stuff in HW > (for example IPsec inline case) I am surely ok with the idea to give vendors an ability to customize implem= entation and enable their HW capabilities. Do you think proposed additions to the rte_security would be enough, or something extra is needed? Konstantin >=20 > # For protocols like UDP, it makes sense to create librte_udp as there > no much HW specific offload other than ethdev provides. >=20 > # PDCP could be another library to offload to HW, So talking > rte_security path makes more sense in that case too. >=20 > Jerin