From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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" <konstantin.ananyev@intel.com>,
 Anoob Joseph <anoobj@marvell.com>, Akhil Goyal <akhil.goyal@nxp.com>,
 Adrien Mazarguil <adrien.mazarguil@6wind.com>,
 "Doherty, Declan" <declan.doherty@intel.com>,
 "Yigit, Ferruh" <ferruh.yigit@intel.com>,
 Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
 Thomas Monjalon <thomas@monjalon.net>
Cc: Ankur Dwivedi <adwivedi@marvell.com>,
 Hemant Agrawal <hemant.agrawal@nxp.com>, Matan Azrad <matan@mellanox.com>,
 "Nicolau, Radu" <radu.nicolau@intel.com>,
 Shahaf Shuler <shahafs@mellanox.com>,
 Narayana Prasad Raju Athreya <pathreya@marvell.com>,
 "dev@dpdk.org" <dev@dpdk.org>
References: <1575801683-27269-1-git-send-email-anoobj@marvell.com>
 <BN7PR11MB25478BD0152762D4CB1E16FC9A580@BN7PR11MB2547.namprd11.prod.outlook.com>
 <MN2PR18MB2877B0C35B11432B9FA02452DF580@MN2PR18MB2877.namprd18.prod.outlook.com>
 <BN7PR11MB2547EAF3CE86AAE432F903E29A5A0@BN7PR11MB2547.namprd11.prod.outlook.com>
 <MN2PR18MB2877C4D70B32ECDFED9F82E8DF5A0@MN2PR18MB2877.namprd18.prod.outlook.com>
 <1fc05516-3686-4267-a760-edbe0b92bc87@intel.com>
 <MN2PR18MB287719FEB9B4BADCA2E081B8DF510@MN2PR18MB2877.namprd18.prod.outlook.com>
 <0a7d957d-e1f6-835b-15d8-4bccc491b4f9@intel.com>
 <MN2PR18MB28771464CD8362870320FBCADF500@MN2PR18MB2877.namprd18.prod.outlook.com>
 <8d5062b2-d7a7-9788-5788-01720dbad2f5@intel.com>
 <MN2PR18MB28773AE361D052AE1F0A5389DF530@MN2PR18MB2877.namprd18.prod.outlook.com>
 <d54d7992-8438-1916-a064-dd57b67c0add@intel.com>
 <MN2PR18MB28777FD4DE38B8FCC09B7FD8DF520@MN2PR18MB2877.namprd18.prod.outlook.com>
 <SN6PR11MB2558CAB88A529007DF3235FE9A2E0@SN6PR11MB2558.namprd11.prod.outlook.com>
From: "Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>
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: <SN6PR11MB2558CAB88A529007DF3235FE9A2E0@SN6PR11MB2558.namprd11.prod.outlook.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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 <konstantin.ananyev@intel.com>

I also don't have objections.

Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>

>
>>>>> - 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 <anoobj@marvell.com>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>       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