DPDK patches and discussions
 help / color / mirror / Atom feed
From: Anoob Joseph <anoobj@marvell.com>
To: "Medvedkin, Vladimir" <vladimir.medvedkin@intel.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" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: allow multiple security sessions to use one rte flow
Date: Thu, 19 Dec 2019 04:37:39 +0000	[thread overview]
Message-ID: <MN2PR18MB28777FD4DE38B8FCC09B7FD8DF520@MN2PR18MB2877.namprd18.prod.outlook.com> (raw)
In-Reply-To: <d54d7992-8438-1916-a064-dd57b67c0add@intel.com>

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.

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


  reply	other threads:[~2019-12-19  4:37 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-08 10:41 [dpdk-dev] " Anoob Joseph
2019-12-09  7:37 ` Jerin Jacob
2019-12-10 20:47   ` Ori Kam
2020-01-20  9:51     ` Thomas Monjalon
2019-12-09 13:18 ` Ananyev, Konstantin
2019-12-09 13:57   ` Anoob Joseph
2019-12-11 11:06     ` Ananyev, Konstantin
2019-12-11 17:33       ` Anoob Joseph
2019-12-13 11:55         ` Ananyev, Konstantin
2019-12-15  6:07           ` Anoob Joseph
2019-12-16 12:54             ` Ananyev, Konstantin
2019-12-16 15:37               ` Anoob Joseph
2019-12-16 15:58         ` Medvedkin, Vladimir
2019-12-16 16:16           ` [dpdk-dev] [EXT] " Anoob Joseph
2019-12-17 11:21             ` Medvedkin, Vladimir
2019-12-17 14:24               ` Anoob Joseph
2019-12-17 17:44                 ` Medvedkin, Vladimir
2019-12-18  3:54                   ` Anoob Joseph
2019-12-18 13:52                     ` Medvedkin, Vladimir
2019-12-19  4:37                       ` Anoob Joseph [this message]
2019-12-19 17:45                         ` Medvedkin, Vladimir
2019-12-23 13:34                         ` Ananyev, Konstantin
2020-01-08 14:29                           ` Medvedkin, Vladimir
2020-01-09  7:35                             ` Ori Kam
2020-01-14  9:27                               ` Anoob Joseph
2020-01-16 11:36                                 ` Ori Kam
2020-01-16 12:03                                   ` Anoob Joseph
2020-01-16 13:37                                     ` Ori Kam
2020-01-18  8:11                                       ` Anoob Joseph
2020-01-19  7:25                                         ` Ori Kam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MN2PR18MB28777FD4DE38B8FCC09B7FD8DF520@MN2PR18MB2877.namprd18.prod.outlook.com \
    --to=anoobj@marvell.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=adwivedi@marvell.com \
    --cc=akhil.goyal@nxp.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinj@marvell.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=matan@mellanox.com \
    --cc=pathreya@marvell.com \
    --cc=radu.nicolau@intel.com \
    --cc=shahafs@mellanox.com \
    --cc=thomas@monjalon.net \
    --cc=vladimir.medvedkin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).