> > > > > > 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. > > Yes, that was my thought. > > > > > 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``? > > I am definitely not against this new flag, but if we'll have 2 new functions > instead, do you still need it? > > > > > 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. > > As I can see there are two options to fulfill that requirement: > > 1. Introduce a new function that would update sa.sqn value. > Something like rte_ipsec_sa_update_sqn(...). > So when metadata from HW arrives, SW can call it and sync sa.sqn with new > HW value, and then continue with conventional rte_ipsec_crypto_prepare(...); >
[Aakash] With option 1, one issue that I can see is that, multiple cores can call rte_ipsec_sa_update_sqn() creating a race condition. > 2. Introduce new (extended) variants of ipsec_crypto_prepare/process that > would take SQN (might be something else ?) as extra parameter, something > like: > > rte_ipcec_xprepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[], > struct rte_crypto_op *cop[], uint16_t num, /* extra params will be > there*/); > > Which one is better, hard to say for me off-hand... > Might be both are needed. > But probably we don't need to decide right now? > For me it would be enough if you'll outline the plan to change and extend > ipsec > lib > API with new data-path functions and probably new flag for session create. > [Aakash] Will go ahead with the option of introducing two new APIs: 1. rte_ipsec_pkt_crypto_seq_no_assign 2. rte_ipsec_pkt_crypto_xprepare along with the config structure changes and the new flag for session create. I will update v3 of the announcement with this. > > > > Initially we are looking at outbound only. But similar kind of use > > > > case would > > > be applicable for inbound also. > > > > > > > > > > > > > > > > Signed-off-by: Aakash Sasidharan <asasidha...@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 > > > > > >
