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 09518A04F3; Wed, 8 Jan 2020 15:30:06 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C0B621DA3D; Wed, 8 Jan 2020 15:30:06 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 830861DA3B for ; Wed, 8 Jan 2020 15:30:04 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Jan 2020 06:30:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,410,1571727600"; d="scan'208";a="254239705" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.251.84.3]) ([10.251.84.3]) by fmsmga002.fm.intel.com with ESMTP; 08 Jan 2020 06:29:59 -0800 To: "Ananyev, Konstantin" , Anoob Joseph , Akhil Goyal , Adrien Mazarguil , "Doherty, Declan" , "Yigit, Ferruh" , Jerin Jacob Kollanukkaran , Thomas Monjalon Cc: Ankur Dwivedi , Hemant Agrawal , Matan Azrad , "Nicolau, Radu" , Shahaf Shuler , Narayana Prasad Raju Athreya , "dev@dpdk.org" References: <1575801683-27269-1-git-send-email-anoobj@marvell.com> <1fc05516-3686-4267-a760-edbe0b92bc87@intel.com> <0a7d957d-e1f6-835b-15d8-4bccc491b4f9@intel.com> <8d5062b2-d7a7-9788-5788-01720dbad2f5@intel.com> From: "Medvedkin, Vladimir" Message-ID: <23ee9ff1-93ac-3381-d10d-867681c25932@intel.com> Date: Wed, 8 Jan 2020 14:29:58 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: allow multiple security sessions to use one rte flow 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 Anoob, On 23/12/2019 13:34, Ananyev, Konstantin wrote: > >>>>>>>>>>>>>> The rte_security API which enables inline protocol/crypto >>>>>>>>>>>>>> feature mandates that for every security session an rte_flow >>>>>>>>>>>>>> is >>>>> created. >>>>>>>>>>>>>> This would internally translate to a rule in the hardware >>>>>>>>>>>>>> which would do packet classification. >>>>>>>>>>>>>> >>>>>>>>>>>>>> In rte_securty, one SA would be one security session. And if >>>>>>>>>>>>>> an rte_flow need to be created for every session, the number >>>>>>>>>>>>>> of SAs supported by an inline implementation would be >>>>>>>>>>>>>> limited by the number of rte_flows the PMD would be able to >>> support. >>>>>>>>>>>>>> If the fields SPI & IP addresses are allowed to be a range, >>>>>>>>>>>>>> then this limitation can be overcome. Multiple flows will be >>>>>>>>>>>>>> able to use one rule for SECURITY processing. In this case, >>>>>>>>>>>>>> the security session provided as conf would be NULL. >>>>>>>>>>>>> Wonder what will be the usage model for it? >>>>>>>>>>>>> AFAIK, RFC 4301 clearly states that either SPI value alone >>>>>>>>>>>>> or in conjunction with dst (and src) IP should clearly >>>>>>>>>>>>> identify SA for inbound SAD >>>>>>>>>>> lookup. >>>>>>>>>>>>> Am I missing something obvious here? >>>>>>>>>>>> [Anoob] Existing SECURITY action type requires application to >>>>>>>>>>>> create an 'rte_flow' per SA, which is not really required if >>>>>>>>>>>> h/w can use SPI to uniquely >>>>>>>>>>> identify the security session/SA. >>>>>>>>>>>> Existing rte_flow usage: IP (dst,src) + ESP + SPI -> security >>>>>>>>>>>> processing enabled on one security session (ie on SA) >>>>>>>>>>>> >>>>>>>>>>>> The above rule would uniquely identify packets for an SA. But >>>>>>>>>>>> with the above usage, we would quickly exhaust entries >>>>>>>>>>>> available in h/w lookup tables (which are limited on our >>>>>>>>>>>> hardware). But if h/w can use SPI field to index >>>>>>>>>>> into a table (for example), then the above requirement of one >>>>>>>>>>> rte_flow per SA is not required. >>>>>>>>>>>> Proposed rte_flow usage: IP (any) + ESP + SPI (any) -> >>>>>>>>>>>> security processing enabled on all ESP packets >>>>>>>>> So this means that SA will be indexed only by spi? What about >>>>>>>>> SA's which are indexed by SPI+DIP+SIP? >>>>>>>>>>>> Now h/w could use SPI to index into a pre-populated table to >>>>>>>>>>>> get security session. Please do note that, SPI is not ignored >>>>>>>>>>>> during the actual >>>>>>>>>>> lookup. Just that it is not used while creating 'rte_flow'. >>>>>>>>>>> >>>>>>>>>>> And this table will be prepopulated by user and pointer to it >>>>>>>>>>> will be somehow passed via rte_flow API? >>>>>>>>>>> If yes, then what would be the mechanism? >>>>>>>>>> [Anoob] I'm not sure what exactly you meant by user. But may be >>>>>>>>>> I'll explain >>>>>>>>> how it's done in OCTEONTX2 PMD. >>>>>>>>>> The application would create security_session for every SA. SPI >>>>>>>>>> etc would be >>>>>>>>> available to PMD (in conf) when the session is created. Now the >>>>>>>>> PMD would populate SA related params in a specific location that >>>>>>>>> h/w would access. This memory is allocated during device >>>>>>>>> configure and h/w would have the pointer after the initialization is >>> done. >>>>>>>>> If memory is allocated during device configure what is upper >>>>>>>>> limit for number of sessions? What if app needs more? >>>>>>>>>> PMD uses SPI as index to write into specific locations(during >>>>>>>>>> session create) >>>>>>>>> and h/w would use it when it sees an ESP packet eligible for >>>>>>>>> SECURITY (in receive path, per packet). As long as session >>>>>>>>> creation could populate at memory locations that h/w would look >>>>>>>>> at, this scheme would >>>>>>> work. >>>>>>>> [Anoob] Yes. But we need to allow application to control the h/w >>>>>>>> ipsec >>>>>>> processing as well. Let's say, application wants to handle a >>>>>>> specific SPI range in lookaside mode (may be because of unsupported >>>>>>> capabilities?), in that case having rte_flow will help in fine >>>>>>> tuning how the >>>>> h/w packet steering happens. >>>>>>> Also, rte_flow enables H/w parsing on incoming packets. This info >>>>>>> is useful even after IPsec processing is complete. Or if >>>>>>> application wants to give higher priority to a range of SPIs, >>>>>>> rte_flow would allow doing >>>>> so. >>>>>>>>> What algorithm of indexing by SPI is there? Could I use any >>>>>>>>> arbitrary SPI? If some kind of hashing is used, what about collisions? >>>>>>>> [Anoob] That is implementation dependent. In our PMD, we map it >>>>>>>> one >>>>> to one. >>>>>>> As in, SPI is used as index in the table. >>>>>>> So, as far as you are mapping one to one and using SPI as an index, >>>>>>> a lot of memory is wasted in the table for unused SPI's. Also, you >>>>>>> are not able to have a table with 2^32 sessions. It is likely that >>>>>>> some number of SPI's least significant bits are used as an index. >>>>>>> And it raises a question - what if application needs two sessions >>>>>>> with different >>>>> SPI's which have the same lsb's? >>>>>> [Anoob] rte_security_session_create() would fail. Why do you say we >>>>> cannot support 2^32 sessions? If it's memory limitation, the same >>>>> memory limitation would apply even if you have dynamic allocation of >>>>> memory for sessions. So at some point session creation would start >>>>> failing. In our PMD, we allow user to specify the range it requires using >>> devargs. >>>>>> Also, collision of LSBs can be avoided by introducing a "MARK" rule >>>>>> in >>>>> addition to "SECURITY" for the rte_flow created for inline ipsec. >>>>> Currently that model is not supported (in the library), but that is >>>>> one solution to the collisions that can be pursued later. >>>>>>> Moreover, what about >>>>>>> two sessions with same SPI but different dst and src ip addresses? >>>>>> [Anoob] Currently our PMD only support UCAST IPSEC. So another >>>>>> session >>>>> with same SPI would result in session creation failure. >>>>> >>>>> Aha, I see, thanks for the explanation. So my suggestion here would be: >>>>> >>>>> - Application defines that some subset of SA's would be inline >>>>> protocol processed. And this SA's will be indexed by SPI only. >>>>> >>>>> - App defines special range for SPI values of this SA's (size of this >>>>> range is defined using devargs) and first SPI value (from configuration?). >>>>> >>>>> - App installs rte_flow only for this range (from first SPI to first >>>>> SPI >>>>> + range size), not for all SPI values. >>>> [Anoob] This is exactly what this patch proposes. Allowing the SPI and the >>> IP addresses to be range and have security_session provided as NULL. What >>> you have described would be achievable only if we can allow this >>> modification in the lib. >>>> So can I assume you are in agreement with this patch? >>> Not exactly. I meant it is better to make more specified flow like: >>> >>> ... >>> >>> struct rte_flow_item_esp esp_spec = { >>> >>> .hdr = { >>> .spi = rte_cpu_to_be_32(first_spi), >>> }, >>> >>> }; >>> >>> struct rte_flow_item_esp esp_mask = { >>> >>> .hdr = { >>> .spi = rte_cpu_to_be_32(nb_ipsec_in_sa - 1), >>> }, >>> >>> }; >>> >>> pattern[0].type = RTE_FLOW_ITEM_TYPE_ESP; >>> >>> pattern[0].spec = & esp_spec; >>> >>> pattern[0].mask = &esp_mask; >>> >>> ... >>> >>> So this means inline proto device would process only special subset of SPI's. >>> All other will be processed as usual. Sure, you can assign all >>> 2^32 SPI range and it work as you intended earlier. I think we need to have >>> finer grained control here. >>> >> [Anoob] Allowing a range for SPI is what you have also described. What you described is one way to define a range. That will come as >> part of the implementation, ie, a change in the example application. This patch intends to allow using a range for SPI than a fixed >> value. I believe you are also in agreement there. > I also don't have objections for that patch. > The only obseravion from reading your replies to that at ipsec-secgw patches: > Extra API to retrieve size of that HW table seems to be needed. > Though I suppose it could be a subject of separate patch/discussion. > > Acked-by: Konstantin Ananyev I also don't have objections. Acked-by: Vladimir Medvedkin > >>>>> - Other SPI values would be processed non inline. >>>>> >>>>> In this case we would be able to have SA addressed by longer tuple (i.e. >>>>> SPI+DIP+SIP) outside of before mentioned range, as well as SA with >>>>> unsupported capabilities by inline protocol device. >>>>> >>>>>>>>>>>> The usage of one 'rte_flow' for multiple SAs is not mandatory. >>>>>>>>>>>> It is only required when application requires large number of >>> SAs. >>>>>>>>>>>> The proposed >>>>>>>>>>> change is to allow more efficient usage of h/w resources where >>>>>>>>>>> it's permitted by the PMD. >>>>>>>>>>>>>> Application should do an rte_flow_validate() to make sure >>>>>>>>>>>>>> the flow is supported on the PMD. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Signed-off-by: Anoob Joseph >>>>>>>>>>>>>> --- >>>>>>>>>>>>>> lib/librte_ethdev/rte_flow.h | 6 ++++++ >>>>>>>>>>>>>> 1 file changed, 6 insertions(+) >>>>>>>>>>>>>> >>>>>>>>>>>>>> diff --git a/lib/librte_ethdev/rte_flow.h >>>>>>>>>>>>>> b/lib/librte_ethdev/rte_flow.h index 452d359..21fa7ed >>> 100644 >>>>>>>>>>>>>> --- a/lib/librte_ethdev/rte_flow.h >>>>>>>>>>>>>> +++ b/lib/librte_ethdev/rte_flow.h >>>>>>>>>>>>>> @@ -2239,6 +2239,12 @@ struct rte_flow_action_meter { >>>>>>>>>>>>>> * direction. >>>>>>>>>>>>>> * >>>>>>>>>>>>>> * Multiple flows can be configured to use the same >>>>>>>>>>>>>> security >>>>> session. >>>>>>>>>>>>>> + * >>>>>>>>>>>>>> + * The NULL value is allowed for security session. If >>>>>>>>>>>>>> + security session is NULL, >>>>>>>>>>>>>> + * then SPI field in ESP flow item and IP addresses in flow >>>>>>>>>>>>>> + items 'IPv4' and >>>>>>>>>>>>>> + * 'IPv6' will be allowed to be a range. The rule thus >>>>>>>>>>>>>> + created can enable >>>>>>>>>>>>>> + * SECURITY processing on multiple flows. >>>>>>>>>>>>>> + * >>>>>>>>>>>>>> */ >>>>>>>>>>>>>> struct rte_flow_action_security { >>>>>>>>>>>>>> void *security_session; /**< Pointer to security >>>>>>>>>>>>>> session >>>>>>> structure. >>>>>>>>>>>>>> */ >>>>>>>>>>>>>> -- >>>>>>>>>>>>>> 2.7.4 >>>>>>>>> -- >>>>>>>>> Regards, >>>>>>>>> Vladimir >>>>>>> -- >>>>>>> Regards, >>>>>>> Vladimir >>>>> -- >>>>> Regards, >>>>> Vladimir >>> -- >>> Regards, >>> Vladimir -- Regards, Vladimir