Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: allow multiple security sessions to use one rte flow

["Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>]

Hi Anoob,

On 19/12/2019 04:37, Anoob Joseph wrote:
> Hi Vladimir,
>
> Please see inline.
>
> Thanks,
> Anoob
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Medvedkin, Vladimir
>> Sent: Wednesday, December 18, 2019 7:22 PM
>> To: Anoob Joseph <anoobj@marvell.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.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
>> Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: allow multiple security
>> sessions to use one rte flow
>>
>> Hi Anoob,
>>
>> On 18/12/2019 03:54, Anoob Joseph wrote:
>>> Hi Vladimir,
>>>
>>> Please see inline.
>>>
>>> Thanks,
>>> Anoob
>>>
>>>> -----Original Message-----
>>>> From: Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
>>>> Sent: Tuesday, December 17, 2019 11:14 PM
>>>> To: Anoob Joseph <anoobj@marvell.com>; Ananyev, Konstantin
>>>> <konstantin.ananyev@intel.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
>>>> Subject: Re: [EXT] Re: [dpdk-dev] [PATCH] ethdev: allow multiple
>>>> security sessions to use one rte flow
>>>>
>>>> Hi Anoob,
>>>>
>>>> On 17/12/2019 14:24, Anoob Joseph wrote:
>>>>> Hi Vladimir,
>>>>>
>>>>> Please see inline.
>>>>>
>>>>> Thanks,
>>>>> Anoob
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
>>>>>> Sent: Tuesday, December 17, 2019 4:51 PM
>>>>>> To: Anoob Joseph <anoobj@marvell.com>; Ananyev, Konstantin
>>>>>> <konstantin.ananyev@intel.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
>>>>>> Subject: Re: [EXT] Re: [dpdk-dev] [PATCH] ethdev: allow multiple
>>>>>> security sessions to use one rte flow
>>>>>>
>>>>>> Hi Anoob,
>>>>>>
>>>>>> On 16/12/2019 16:16, Anoob Joseph wrote:
>>>>>>> Hi Vladimir,
>>>>>>>
>>>>>>> Please see inline.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Anoob
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
>>>>>>>> Sent: Monday, December 16, 2019 9:29 PM
>>>>>>>> To: Anoob Joseph <anoobj@marvell.com>; Ananyev, Konstantin
>>>>>>>> <konstantin.ananyev@intel.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
>>>>>>>> Subject: [EXT] Re: [dpdk-dev] [PATCH] ethdev: allow multiple
>>>>>>>> security sessions to use one rte flow
>>>>>>>>
>>>>>>>> External Email
>>>>>>>>
>>>>>>>> -----------------------------------------------------------------
>>>>>>>> --
>>>>>>>> --
>>>>>>>> -
>>>>>>>> Hi Anoob,
>>>>>>>>
>>>>>>>> On 11/12/2019 17:33, Anoob Joseph wrote:
>>>>>>>>> Hi Konstantin,
>>>>>>>>>
>>>>>>>>> Please see inline.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Anoob
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ananyev,
>>>> Konstantin
>>>>>>>>>> Sent: Wednesday, December 11, 2019 4:36 PM
>>>>>>>>>> To: 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
>>>>>>>>>> Subject: Re: [dpdk-dev] [PATCH] ethdev: allow multiple security
>>>>>>>>>> sessions to use one rte flow
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>>> 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.

Thanks for clarification, no objections from my side.

>
>>>> - 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