Re: [dpdk-dev] [RFC] ipsec: new library for IPsec data-path processing

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

Hi Akhil,

> 
> Hi Konstantin,
> 
> On 10/1/2018 2:30 AM, Ananyev, Konstantin wrote:
> >
> > Hi Akhil,
> >
> >> Hi Konstantin,
> >>
> >> On 9/24/2018 4:21 PM, Ananyev, Konstantin wrote:
> >>> Hi Akhil,
> >>>
> >>>> Hi Konstantin,
> >>>>
> >>>> On 9/18/2018 6:12 PM, Ananyev, Konstantin wrote:
> >>>>>>> I am not saying this should be the ONLY way to do as it does not work
> >>>>>>> very well with non NPU/FPGA class of SoC.
> >>>>>>>
> >>>>>>> So how about making the proposed IPSec library as plugin/driver to
> >>>>>>> rte_security.
> >>>>>> As I mentioned above, I don't think that pushing whole IPSec data-path into rte_security
> >>>>>> is the best possible approach.
> >>>>>> Though I probably understand your concern:
> >>>>>> In RFC code we always do whole prepare/process in SW (attach/remove ESP headers/trailers, so paddings etc.),
> >>>>>> i.e. right now only device types: RTE_SECURITY_ACTION_TYPE_NONE and RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO are
> >> covered.
> >>>>>> Though there are devices where most of prepare/process can be done in HW
> >>>>>> (RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL/RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL),
> >>>>>> plus in future could be devices where prepare/process would be split between HW/SW in a custom way.
> >>>>>> Is that so?
> >>>>>> To address that issue I suppose we can do:
> >>>>>> 1. Add support for RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL and RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL
> >>>>>>        security devices into ipsec.
> >>>>>>        We planned to do it anyway, just don't have it done yet.
> >>>>>> 2. For custom case - introduce RTE_SECURITY_ACTION_TYPE_INLINE_CUSTOM and
> >>>> RTE_SECURITY_ACTION_TYPE_LOOKASIDE_CUSTOM
> >>>>>>        and add into rte_security_ops   new functions:
> >>>>>>        uint16_t lookaside_prepare(struct rte_security_session *sess, struct rte_mbuf *mb[], struct struct rte_crypto_op *cop[],
> uint16_t
> >>>> num);
> >>>>>>        uint16_t lookaside_process(struct rte_security_session *sess, struct rte_mbuf *mb[], struct struct rte_crypto_op *cop[],
> uint16_t
> >>>> num);
> >>>>>>        uint16_t inline_process(struct rte_security_session *sess, struct rte_mbuf *mb[], struct struct rte_crypto_op *cop[], uint16_t
> >> num);
> >>>>>>        So for custom HW, PMD can overwrite normal prepare/process behavior.
> >>>>>>
> >>>>> Actually  after another thought:
> >>>>> My previous assumption (probably wrong one) was that for both
> >>>>> RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL and RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL
> >>>>> devices can do whole data-path ipsec processing totally in HW - no need for any SW support (except init/config).
> >>>>> Now looking at dpaa and dpaa2 devices (the only ones that supports RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL right
> now)
> >>>>> I am not so sure about that - looks like some SW help might be needed for replay window updates, etc.
> >>>>> Hemant, Shreyansh - can you guys confirm what is expected from RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL devices
> >>>>> (HW/SW roses/responsibilities)?
> >>>>> About RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL  - I didn't find any driver inside DPDK source tree that does support that
> >>>> capability.
> >>>>> So my question is there any devices/drivers that do support it?
> >>>>> If so, where could source code could be found, and what are HW/SW roles/responsibilities for that type of devices?
> >>>>> Konstantin
> >>>>>
> >>>>>
> >>>> In case of LOOKASIDE, the protocol errors like antireplay and sequence
> >>>> number overflow shall be the responsibility of either PMD or the HW.
> >>>> It should notify the application that the error has occurred and
> >>>> application need to decide what it needs to decide next.
> >>> Ok, thanks for clarification.
> >>> Just to confirm -  do we have a defined way for it right now in rte_security?
> >> As of now, there are no macros defined for antireplay/seq. no. overflow
> >> errors in crypto errors(rte_crypto_op_status), but it will be added soon.
> >> For inline cases, ipsec-secgw application gets error notification via
> >> rte_eth_event.
> > Ok.

Actually looking at it a bit closer -you are talking about RTE_ETH_EVENT_IPSEC, right?
I do see struct/types definitions, and to see code in ipsec-secgw to handle it,
but I don't see any driver that supports it.
Is that what intended?

> >
> >
> >>>> As Jerin said in other email, the roles/responsibility of the PMD in
> >>>> case of inline proto and lookaside case, nothing much is required from
> >>>> the application to do any processing for ipsec.
> >>>>
> >>>> As per my understanding, the proposed RFC is to make the application
> >>>> code cleaner for  the protocol processing.
> >>> Yes, unified data-path API is definitely one of the main goals.
> >>>
> >>>> 1. For inline proto and lookaside there won't be any change in the data
> >>>> path. The main changes would be in the control path.
> >>> Yes, from your and Jerin description data-path processing looks
> >>> really lightweight for these cases.
> >>> For control path - there is no much change, user would have to call
> >>> rte_ipsec_sa_init() to start using given SA.
> >>>
> >>>> 2. But in case of inline crypto and RTE_SECURITY_ACTION_TYPE_NONE, the
> >>>> protocol processing will be done in the library and there would be
> >>>> changes in both control and data path.
> >>> Yes.
> >>>
> >>>> As the rte_security currently provide generic APIs for control path only
> >>>> and we may have it expanded for protocol specific datapath processing.
> >>>> So for the application, working with inline crypto/ inline proto would
> >>>> be quite similar and it won't need to do some extra processing for
> >>>> inline crypto.
> >>>> Same will be the case for RTE_SECURITY_ACTION_TYPE_NONE and lookaside.
> >>>>
> >>>> We may have the protocol specific APIs reside inside the rte_security
> >>>> and we can use either the crypto/net PMD underneath it.
> >>> As I understand, you suggest instead of introducing new library,
> >>> introduce similar data-path functions inside rte_security.
> >>> Probably something like:
> >>>
> >>> uint16_t rte_security_process(struct rte_security_session *s, struct rte_mbuf *mb[], uint16_t num);
> >>> uint16_t rte_security_crypto_prepare(struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> >>>                                                                         struct rte_crypto_op *cop[], uint16_t num);
> >>> ...
> >>> Is that correct?
> >> "rte_security_process_ipsec" and "rte_security_crypto_prepare_ipsec" will be better.
> >> We can have such APIs for other protocols as well.
> >> Also, we should leave the existing functionality as is and we should let the user decide whether
> >> it needs to manage the ipsec on it's own or with the new APIs.
> > There are no plans to void any existing API.
> > If the user has working code that uses rte_crytpodev/rte_security directly and wants to keep it,
> > that's fine.
> >
> >>> I thought about that approach too, and indeed from one side it looks cleaner and easier
> >>> to customize - each of these functions would just call related function inside rte_security_ops.
> >>> The problem with that approach - it would mean that each SA would be able to work with one
> >>> device only.
> >>> So if someone needs an SA that could be processed by multiple cores and multiple crypto-devices
> >>> in parallel such approach wouldn’t fit.
> >> One SA should be processed by a single core or else we need to have an
> >> event based application which support ordered queues,
> >> because if we process packets of single SA on multiple cores, then
> >> packets will get re-ordered and we will get the anti-replay late errors
> >> on decap side.
> > I suppose in some cases one core would be enough to handle SA traffic,
> > for some not, as I said before, I think it should be configurable.
> > Of course for MT case some entity that would  guarantee proper ordering
> > for final packet processing would be needed.
> > It could be some eventdev, or SW FIFO queue, or something else.
> >
> >> And if we have event based solution, then the scheduler will be able to
> >> handle the load balancing accordingly.
> > Didn't understand that sentence.
> I mean the event device will be able to handle that which has an inbuilt
> scheduler in it for balancing the load of single SA,
> and if the queues are ordered and it support order restoration, then it
> will be able to maintain the ordering. And for that
> you would not have to bother about giving the same SA to different
> cryptodevs on multi cores.

If such event device will be available for the user, and it would be a user preference to use it -
that's fine.
In such case there is no need for MT support  just ST version of SA code could be used.
But I suppose such scheduler shouldn't be the only option.

> >>> That was the main reason to keep rte_security as it is right now and go ahead with new library.
> >>> One thing that worries me -  do we need a way to share SQN and replay window information
> >>> between rte_security and upper layer (rte_ipsec)?
> >>> If 'no', then ok, if 'yes' then probably we need to discuss how to do it now?
> >> anti-replay window size shall be a parameter in ipsec_xform, which shall
> >> be added.
> >> And the error notification
> >>    - in case of using crypto, then use rte_crypto_op_status
> >> - in case of inline cases, then use rte_eth_event callbacks.
> >> I don't see rte_ipsec needs to take care of that in your initial approach.
> >> However, if you plan to include session reset inside rte_ipsec, then you
> >> may need that inside the rte_ipsec.
> > I am not talking rte_ipsec, my concern here is rte_security.
> > Suppose you need to switch from device that can do inline_proto to the device that doesn't.
> In what use case you would need such switching?

As an example - device detach,  VM live migration, in some cases even changes in routing table.
As another example - limitations in HW offload supported.
Let say ixgbe doesn't support ip reassemble.

> > Right now the only way - renegotiate all SAs that were handled by inline_proto device
> > (because there is no way to retrieve from rte_security device SQN information).
> > Renegotiation should work, but it looks like quite expensive approach.
> This will be only for the first packet.

Sure, and now imagine you have 1M SAs on inline-proto device and sysadmin wants
to detach that device.
How long it would take to re-negotiate all of them?

> > If rte_security would have a way to share its SQN status with SW, then I think it would
> > be possible to do such switch without SA termination.
> what kind of SQN status you are looking for? overflow?

Nope, I am talking about last-seq and replay-window state:
https://tools.ietf.org/html/rfc4303#section-3.4.3

Konstantin

> If yes,
> application need to re-negotiate the session,
> which will be done periodically anyways.
> > Again with such info available - load-balancing for the same SA on multiple devices
> > might be possible.
> > Konstantin
> >