RE: [PATCH v2] doc: announce rte_ipsec API changes

[Aakash Sasidharan <asasidharan@marvell.com>]

> -----Original Message-----
> From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> Sent: Wednesday, July 24, 2024 10:39 PM
> To: Aakash Sasidharan <asasidharan@marvell.com>
> Cc: Akhil Goyal <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>;
> Anoob Joseph <anoobj@marvell.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>; dev@dpdk.org; konstantin.v.ananyev@yandex.ru;
> vladimir.medvedkin@intel.com
> Subject: [EXTERNAL] RE: [PATCH v2] doc: announce rte_ipsec API changes
> 
> > > > In case of event mode operations where event device can help in
> > > > atomic sequence number increment across cores, sequence number
> > > > need to be provided by the application instead of being updated in
> > > > rte_ipsec or the PMD. To support this, a new flag
> > > > ``RTE_IPSEC_SAFLAG_SQN_ASSIGN_DISABLE``
> > > > will be added to disable sequence number update inside IPsec
> > > > library and the API rte_ipsec_pkt_crypto_prepare will be extended
> > > > to include ``sqn`` as an additional parameter to specify sequence
> > > > number to be used for IPsec from the application.
> > >
> > > Could you probably elaborate a bit more:
> > > Why such change is necessary for event-dev mode, what exactly will
> > > be affected in librte_ipsec (would it be for outbound mode, or both), etc.
> > >
> >
> > [Aakash] When using eventdev, it is possible to have multiple cores
> > process packets from the same flow at the same time, but still have ordering
> maintained.
> >
> > Sequence for IPsec would be like below, 1. Ethdev Rx computes flow
> > hash and submits packets to an ORDERED eventdev queue.
> >     One flow would always hit one event dev queue.
> >     One eventdev queue can be attached to multiple eventdev ports.
> > 2. Lcores receives packets via these eventdev ports.
> >     Lcores can now process the packets from the same flow in parallel.
> > 3. Lcores submit the packets to an ATOMIC queue
> >     This is needed as IPsec seq no update needs to be done atomically.
> > 4. After seq no update, packets are moved to ORDERED queue.
> >     Lcores can now processes the packets in parallel again.
> > 5. During Tx, eventdev ensures packet ordering based on ORDERED queue.
> >
> > Since lib IPsec takes care of sequence number assignment, complete
> > rte_ipsec_pkt_crypto_prepare() routine need to be made as ATOMIC stage.
> > But apart from seq no update, rest of the operations can be done in parallel.
> 
> Thanks for explanation.
> Basically you are seeking ability to split rte_ipsec_pkt_crypto_prepare() for
> outbound into two stages:
> 1. update sqn
> 2. all other preps
> To be able to do step #2 in parallel, correct?
> My thought always was that step #2 is not that expensive in terms of
> performance, and there probably not much point to make it parallel.
> But I suppose you measured step#2 overhead on your platform and
> concluded that it worth it...
> 
> One concern I have with the way you suggested - now we need to
> store/update sa.sqn by some external entity.
> Another thing - don't really want to pollute crypto_prepare() API with new
> parameters which meaning is a bit obscure and depends on other API calls...
> 
> Wouldn't it be easier and probably more straightforward to just introduce 2
> new functions here that would represent step #1 and step #2?
> Then we can keep crypto_prepare() intact, and user will have a choice:
> - either use  original crypto_prepare() - nothing needs to be changed
> -  or instead call these new functions on his own, if he wants to.
> 

[Aakash] As I understand, your suggestion is to introduce a set of two new APIs by splitting the current logic in crypto_prepare(). This should be okay.
For this, I believe we would need change in the structure rte_ipsec_sa_pkt_func to hold the function pointers for the new APIs.

Assuming that, introduction of the new flag RTE_IPSEC_SAFLAG_SQN_ASSIGN_DISABLE to disable seq no assignment in lib IPsec is fine, shall I send v3 announcing changes in ``struct rte_ipsec_sa_pkt_func``?

> > In addition, we are also looking at another use case when a set of
> > packets from a session can be IPsec processed by rte_security device
> > and some packets from the same session would need to be SW processed
> with lib IPsec. Here again the sequence number assignment would need to
> occur at central place so that sequence number is not repeated.
> 
> Interesting, and how SW/HW SQN will be synchronized in that case?
> 

[Aakash] The design is such that HW would assign sequence number for all cases. HW would then pass this data as a metadata to SW so that it can do SW processing with the assigned sequence number.

> > Initially we are looking at outbound only. But similar kind of use case would
> be applicable for inbound also.
> >
> > > >
> > > > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > > > ---
> > > >  doc/guides/rel_notes/deprecation.rst | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > b/doc/guides/rel_notes/deprecation.rst
> > > > index 6948641ff6..bc1d93cca7 100644
> > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > @@ -133,6 +133,13 @@ Deprecation Notices
> > > >    Since these functions are not called directly by the application,
> > > >    the API remains unaffected.
> > > >
> > > > +* ipsec: The rte_ipsec library is updated to support sequence
> > > > +number provided
> > > > +  by application. A new flag
> > > > +``RTE_IPSEC_SAFLAG_SQN_ASSIGN_DISABLE``
> > > > +is introduced
> > > > +  to disable sequence number assignment in lib IPsec.
> > > > +  The API rte_ipsec_pkt_crypto_prepare is extended to include
> > > > +``sqn`` as an
> > > > +  additional parameter allowing application to specify the
> > > > +sequence number to be
> > > > +  used for the IPsec operation.
> > > > +
> > > >  * pipeline: The pipeline library legacy API (functions rte_pipeline_*)
> > > >    will be deprecated and subsequently removed in DPDK 24.11 release.
> > > >    Before this, the new pipeline library API (functions
> > > > rte_swx_pipeline_*)
> > > > --
> > > > 2.25.1
> >