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

[Anoob Joseph <anoobj@marvell.com>]

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