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