Hi Akhil,
Can you see my responses below and see if I can pursue this RFC?
Thanks,
Anoob
> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Anoob Joseph
> Sent: Wednesday, October 9, 2019 4:25 PM
> To: Yigit, Ferruh <ferruh.yi...@linux.intel.com>; Akhil Goyal
> <akhil.go...@nxp.com>; Adrien Mazarguil <adrien.mazarg...@6wind.com>;
> Declan Doherty <declan.dohe...@intel.com>; Pablo de Lara
> <pablo.de.lara.gua...@intel.com>; Thomas Monjalon <tho...@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Narayana Prasad Raju
> Athreya <pathr...@marvell.com>; Ankur Dwivedi <adwiv...@marvell.com>;
> Shahaf Shuler <shah...@mellanox.com>; Hemant Agrawal
> <hemant.agra...@nxp.com>; Matan Azrad <ma...@mellanox.com>; Yongseok
> Koh <ys...@mellanox.com>; Wenzhuo Lu <wenzhuo...@intel.com>;
> Konstantin Ananyev <konstantin.anan...@intel.com>; Radu Nicolau
> <radu.nico...@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [EXT] Re: [RFC] ethdev: allow multiple security
> sessions
> to use one rte flow
>
> Hi Ferruh,
>
> I would like to pursue this RFC.
>
> @Akhil, Please see inline.
>
> Thanks,
> Anoob
>
> > -----Original Message-----
> > From: Yigit, Ferruh <ferruh.yi...@linux.intel.com>
> > Sent: Tuesday, October 8, 2019 6:30 PM
> > To: Akhil Goyal <akhil.go...@nxp.com>; Anoob Joseph
> > <ano...@marvell.com>; Adrien Mazarguil <adrien.mazarg...@6wind.com>;
> > Declan Doherty <declan.dohe...@intel.com>; Pablo de Lara
> > <pablo.de.lara.gua...@intel.com>; Thomas Monjalon
> > <tho...@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Narayana Prasad
> > Raju Athreya <pathr...@marvell.com>; Ankur Dwivedi
> > <adwiv...@marvell.com>; Shahaf Shuler <shah...@mellanox.com>; Hemant
> > Agrawal <hemant.agra...@nxp.com>; Matan Azrad
> <ma...@mellanox.com>;
> > Yongseok Koh <ys...@mellanox.com>; Wenzhuo Lu
> <wenzhuo...@intel.com>;
> > Konstantin Ananyev <konstantin.anan...@intel.com>; Radu Nicolau
> > <radu.nico...@intel.com>; dev@dpdk.org
> > Subject: [EXT] Re: [dpdk-dev] [RFC] ethdev: allow multiple security
> > sessions to use one rte flow
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On 8/19/2019 8:09 AM, Akhil Goyal wrote:
> > > Hi Anoob,
> > >>
> > >> Hi Akhil,
> > >>
> > >>>>>>>>
> > >>>>>>>> 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.
> > >>>>>
> > >>>>> SPI values are normally used to uniquely identify the SA that
> > >>>>> need to be applied on a particular flow.
> > >>>>> I believe SPI value should not be a range for applying a
> > >>>>> particular SA or session.
> > >>>>>
> > >>>>> Plain packet IP addresses can be a range. That is not an issue.
> > >>>>> Multiple plain packet flows can use the same session/SA.
> > >>>>>
> > >>>>> Why do you feel that security session provided should be NULL to
> > >>>>> support multiple flows.
> > >>>>> How will the keys and other SA related info will be passed to
> > >>>>> the
> > >>> driver/HW.
> > >>>>
> > >>>> [Anoob] The SA configuration would be done via rte_security session.
> > >>>> The proposal here only changes the 1:1 dependency of rte_flow and
> > >>>> rte_security session.
> > >>>
> > >>> I don't see this dependency for rte_flow and security session.
> > >>> Multiple flows can be configured to use the same security session.
> > >>>
> > >>>>
> > >>>> The h/w could use SPI field in the received packet to identify
> > >>>> SA(ie, rte_security session). If the h/w allows to index into a
> > >>>> table which holds SA information, then per SPI rte_flow is not
> > >>>> required. This is in fact our case. And for PMDs which doesn't do
> > >>>> it this way,
> > >>>> rte_flow_validate() would fail and then per SPI rte_flow would
> > >>>> require to
> > >>> be created.
> > >>>
> > >>> I am not able to understand the issue here. Flow are validated
> > >>> based on some pattern, You can identify the flow based on some
> > >>> parameter(currently it is spi in case of inline crypto and also your
> > >>> case).
> > >>> You can perform some action based on the security session that you
> > >>> have created before validating the flow And that session creation
> > >>> is nowhere linked to the type of flow. You can use the same
> > >>> session for as many flows you want.
> > >>>
> > >>>>
> > >>>> In the present model, a security session is created, and then
> > >>>> rte_flow will connect ESP packets with one SPI to one security session.
> > >>>> Instead, when we create the security session, h/w can populate
> > >>>> entries in a DB that would be accessed during data path handling.
> > >>>> And the rte_flow could say, all SPI in some range gets inline
> > >>>> processed with the
> > >>> security session identified with its SPI.
> > >>>>
> > >>>> Our PMD supports limited number of flow entries but our h/w can
> > >>>> do SA lookup without flow entries(using SPI instead). So the
> > >>>> current approach of one flow per session is creating an
> > >>>> artificial limit to the number
> > >>> of SAs that can be supported.
> > >>>
> > >>> Ok now I got it. You want to configure a single flow with multiple
> > >>> sessions in it.
> > >>> But defining a range in SPI and tunnel IP addresses does not make
> > >>> sense. In real world applications, Sessions can be created and
> > >>> destroyed at any time with varied values of SPI and tunnel IPs.
> > >>> How can One
> > put a range to that.
> > >>>
> > >>> I would rather say, you actually do not need the rte_flows to be
> > >>> configured for Inline protocol processing. You have configured all
> > >>> the session info in the hw while Creating the session and your H/W
> > >>> will be able to identify on the basis of SPI value which It has
> > >>> stored in the DB
> > and do all the processing.
> > >>
> > >> [Anoob] Yes. That is the model being followed right now. Concern
> > >> is, whether this would be deviating from the spec. In other words,
> > >> we could have devices which would need rte_flow for every
> > >> rte_security session (ixgbe needs for inline crypto), and then we
> > >> could have devices which doesn't need per session rte_flow (which is our
> case).
> > >> What do you think is the right approach for supporting both kinds of
> devices?
> > >
> > > Inline proto case is not using rte_flow at the moment.
> > > And as far as I understand, you also do not need rte_flow to be
> > > configured.
> > > Inline crypto cases are mainly for Intel and Mellanox cases which
> > > only supported Inline crypto. For Protocol offload cases, I don't
> > > feel we need rte_flow as all information related to ipsec is already
> > > there when we call the session create. Rte_flows are used For
> > > segregation of ethernet traffic for classification which can be
> > > configured for various factors
> > as well.
> > >
> > >>
> > >>>
> > >>> What are the changes that you need in the ipsec-secgw for inline
> > >>> proto to work, there is No flow processing currently in the inline
> > >>> proto case. Will it not work as is for you?
> > >>
> > >> [Anoob] In ipsec-secgw, a default flow would be created per
> > >> security enabled port with 'conf=NULL' & SPI = 'ANY'. Flow validate
> > >> would be done to make sure the underlying PMD supports it. For PMDs
> > >> which doesn't support this model, per SA flow would be created.
> > >
> > > Why do you need that flow as well. You have all the information in
> > > the session
> > already.
> > > You can process the packets based on that information. Isn't it?
> > > Current implementation in application is good enough in my opinion.
> > >
> > >>
> > >>> Atleast for NXP devices we are able to work as is without any issue.
> > >>
> > >> [Anoob] Just curious, would having such a dependency on rte_flow be
> > >> an issue for NXP devices?
> > >
> > > As of now I do not have any comment on this. We are not using
> > > rte_flow in
> > our work as of now.
> > > It is kind of POC for us, we may not upstream it.
> > > This will depend on the changes that will be done.
>
> [Anoob] For inline crypto/protocol processing, a h/w lookup is required.
> Whether we need one per SPI is the question here.
>
> The current approach in inline crypto assumes per session, rte_flow would be
> created. But for inline protocol, ipsec-secgw doesn't create any rte_flow,
> which
> also could work if we can have static rules to enable SECURITY processing for
> all
> ESP packets.
>
> The problem is, in that case any other action on ESP packets could result in a
> conflict. Or if the applications wants to enable SECURITY processing only for
> specific flows, the current usage model wouldn't allow that.
>
> May be I'll summarize my concerns,
> 1. rte_flow SECURITY action type requires a security session.
> From the spec of rte_flow, "For INLINE_PROTOCOL, the security protocol is
> fully
> offloaded to HW, providing full encapsulation and decapsulation of packets in
> security protocols. The flow pattern specifies both the outer security header
> fields and the inner packet fields. The security session specified in the
> action
> must match the pattern parameters."
>
> Issue: If only this is allowed, then the numbers of SAs supported will be
> limited
> by max number of flows supported. [Current ipsec-secgw is in violation of the
> above]
>
> 2. A static rule to enable security processing on all ESP packets can be
> enabled.
> With this, application won't be required to create rte_flow per flow and will
> be
> able to use SPI field to identify the security processing to be done on the
> packet.
>
> Issue: If there are any conflicting rte_flow entries for ESP packets, the
> behavior
> is not defined. Also, if application wants to control the flows on which
> SECURITY
> processing is done (as in SECURITY is enabled only for selected SPI values),
> then
> it is not possible.
>
> I believe it is okay to have either of them in ipsec-secgw, but as a spec we
> should
> allow both use cases. Also, all these can be protected by capability
> check/rte_flow_validate(). So if any PMD doesn't support these modes, an
> alternate path can be taken. What do you think?
>
> >
> > Is there any follow up to the RFC? Is it still valid?
> >
> > >
> > >>
> > >>>
> > >>>>
> > >>>>>
> > >>>>>>>>
> > >>>>>>>> Application should do an rte_flow_validate() to make sure the
> > >>>>>>>> flow is supported on the PMD.
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Anoob Joseph <ano...@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 f3a8fb1..4977d3c 100644
> > >>>>>>>> --- a/lib/librte_ethdev/rte_flow.h
> > >>>>>>>> +++ b/lib/librte_ethdev/rte_flow.h
> > >>>>>>>> @@ -1879,6 +1879,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.
> > >>>
> > >>> What you intent here is " The rule thus created can enable
> > >>> multiple security sessions on a single rte flow"
> > >>>
> > >>>
> > >>> Regards,
> > >>> Akhil
