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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

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

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