[v2,0/2] crypto/scheduler: add support for security protocols

Message ID 20230811102401.12552-1-david.coyle@intel.com (mailing list archive)
Headers
Series crypto/scheduler: add support for security protocols |

Message

Coyle, David Aug. 11, 2023, 10:23 a.m. UTC
  This patchset adds support to the cryptodev scheduler PMD and unit
tests for the existing security protocols in the security library,
namely IPSec, MACSec, PDCP and DOCSIS.

v2:
* Improve inclusion of rte_security header files
* Fix typo in commit message

David Coyle (2):
  crypto/scheduler: support security protocols
  test/crypto: add security tests for cryptodev scheduler

 app/test/test_cryptodev.c                     |  14 +-
 doc/guides/rel_notes/release_23_11.rst        |   3 +
 drivers/crypto/scheduler/meson.build          |   2 +-
 .../scheduler/rte_cryptodev_scheduler.c       | 229 ++++++++++-
 drivers/crypto/scheduler/scheduler_failover.c |  12 +-
 .../crypto/scheduler/scheduler_multicore.c    |  10 +-
 .../scheduler/scheduler_pkt_size_distr.c      |  54 +--
 drivers/crypto/scheduler/scheduler_pmd.c      |  33 ++
 drivers/crypto/scheduler/scheduler_pmd_ops.c  | 375 +++++++++++++-----
 .../crypto/scheduler/scheduler_pmd_private.h  | 148 ++++---
 .../crypto/scheduler/scheduler_roundrobin.c   |   6 +-
 11 files changed, 656 insertions(+), 230 deletions(-)
  

Comments

Anoob Joseph Aug. 11, 2023, 11:09 a.m. UTC | #1
Hi David,

While it is desirable to add security under crypto/scheduler, would it be functionally possible if the PMDs perform stateful processing? For example, with lookaside protocol mode of IPsec, fields such as seq no & AR defines how the crypto operation can be performed. Without two PMDs sharing this (actively), how can the load balancing happen?

Said that, I agree utility of scheduler for stateless operations. My understanding is, PDCP offload that is available today is not stateful and that can leverage this. I'm not sure of DOCSIS and MACsec.

Should we make it such that only specific security sessions would be eligible for scheduler operation?

Thanks,
Anoob

> -----Original Message-----
> From: David Coyle <david.coyle@intel.com>
> Sent: Friday, August 11, 2023 3:54 PM
> To: dev@dpdk.org
> Cc: kai.ji@intel.com; kevin.osullivan@intel.com; David Coyle
> <david.coyle@intel.com>
> Subject: [EXT] [PATCH v2 0/2] crypto/scheduler: add support for security
> protocols
> 
> External Email
> 
> ----------------------------------------------------------------------
> This patchset adds support to the cryptodev scheduler PMD and unit tests
> for the existing security protocols in the security library, namely IPSec,
> MACSec, PDCP and DOCSIS.
> 
> v2:
> * Improve inclusion of rte_security header files
> * Fix typo in commit message
> 
> David Coyle (2):
>   crypto/scheduler: support security protocols
>   test/crypto: add security tests for cryptodev scheduler
> 
>  app/test/test_cryptodev.c                     |  14 +-
>  doc/guides/rel_notes/release_23_11.rst        |   3 +
>  drivers/crypto/scheduler/meson.build          |   2 +-
>  .../scheduler/rte_cryptodev_scheduler.c       | 229 ++++++++++-
>  drivers/crypto/scheduler/scheduler_failover.c |  12 +-
>  .../crypto/scheduler/scheduler_multicore.c    |  10 +-
>  .../scheduler/scheduler_pkt_size_distr.c      |  54 +--
>  drivers/crypto/scheduler/scheduler_pmd.c      |  33 ++
>  drivers/crypto/scheduler/scheduler_pmd_ops.c  | 375 +++++++++++++-----
> .../crypto/scheduler/scheduler_pmd_private.h  | 148 ++++---
>  .../crypto/scheduler/scheduler_roundrobin.c   |   6 +-
>  11 files changed, 656 insertions(+), 230 deletions(-)
> 
> --
> 2.25.1
  
Coyle, David Sept. 11, 2023, 4:02 p.m. UTC | #2
Hi Anoob,

Thank you for that feedback - I was on extended leave so only just getting back to it now.
See replies below.

Regards,
David

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Friday, August 11, 2023 12:09 PM
> To: Coyle, David <david.coyle@intel.com>; dev@dpdk.org
> Cc: Ji, Kai <kai.ji@intel.com>; O'Sullivan, Kevin <kevin.osullivan@intel.com>;
> Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: RE: [EXT] [PATCH v2 0/2] crypto/scheduler: add support for security
> protocols
> 
> Hi David,
> 
> While it is desirable to add security under crypto/scheduler, would it be
> functionally possible if the PMDs perform stateful processing? For example,
> with lookaside protocol mode of IPsec, fields such as seq no & AR defines how
> the crypto operation can be performed. Without two PMDs sharing this
> (actively), how can the load balancing happen?

[DC] So if some fields such as seq numbers are maintained within the PMDs for some protocols, then yes you are right - this would not work without some synchronization across PMD instances which I think we'd want to avoid at this point.

I tried to find some cases where a crypto PMD that supports IPSec, for example, maintains some global stateful parameters, but I could not find these cases.
I'm not at all familiar with these PMDs (cnxk, mvsam, dpaa_sec, dpaa2_sec) though, so maybe you could guide me as to where they are maintained?

> 
> Said that, I agree utility of scheduler for stateless operations. My
> understanding is, PDCP offload that is available today is not stateful and that
> can leverage this. I'm not sure of DOCSIS and MACsec.

[DC] I notice that the PDCP security xform struct has a seq number related field, which would also suggest it could be stateful, but I could be wrong.

From a google search MACSec is stateless, but again I'm not an expert.

The protocol I am familiar with is DOCSIS, and it is for this protocol that we have added security support to the cryptodev scheduler.
DOCSIS is 100% stateless, so will work no problem with the scheduler.

> 
> Should we make it such that only specific security sessions would be eligible for
> scheduler operation?

[DC] Do you think it would be acceptable to limit the scheduler to the DOCSIS protocol only for now, and let the IPSec, MACSec and PDCP experts add these later if applicable?
If you think this would be ok, I can easily make that change.

> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: David Coyle <david.coyle@intel.com>
> > Sent: Friday, August 11, 2023 3:54 PM
> > To: dev@dpdk.org
> > Cc: kai.ji@intel.com; kevin.osullivan@intel.com; David Coyle
> > <david.coyle@intel.com>
> > Subject: [EXT] [PATCH v2 0/2] crypto/scheduler: add support for
> > security protocols
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > This patchset adds support to the cryptodev scheduler PMD and unit
> > tests for the existing security protocols in the security library,
> > namely IPSec, MACSec, PDCP and DOCSIS.
> >
> > v2:
> > * Improve inclusion of rte_security header files
> > * Fix typo in commit message
> >
> > David Coyle (2):
> >   crypto/scheduler: support security protocols
> >   test/crypto: add security tests for cryptodev scheduler
> >
> >  app/test/test_cryptodev.c                     |  14 +-
> >  doc/guides/rel_notes/release_23_11.rst        |   3 +
> >  drivers/crypto/scheduler/meson.build          |   2 +-
> >  .../scheduler/rte_cryptodev_scheduler.c       | 229 ++++++++++-
> >  drivers/crypto/scheduler/scheduler_failover.c |  12 +-
> >  .../crypto/scheduler/scheduler_multicore.c    |  10 +-
> >  .../scheduler/scheduler_pkt_size_distr.c      |  54 +--
> >  drivers/crypto/scheduler/scheduler_pmd.c      |  33 ++
> >  drivers/crypto/scheduler/scheduler_pmd_ops.c  | 375
> > +++++++++++++----- .../crypto/scheduler/scheduler_pmd_private.h  | 148
> ++++---
> >  .../crypto/scheduler/scheduler_roundrobin.c   |   6 +-
> >  11 files changed, 656 insertions(+), 230 deletions(-)
> >
> > --
> > 2.25.1
  
Anoob Joseph Sept. 13, 2023, 7:02 a.m. UTC | #3
Hi David,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Coyle, David <david.coyle@intel.com>
> Sent: Monday, September 11, 2023 9:32 PM
> To: Anoob Joseph <anoobj@marvell.com>; dev@dpdk.org
> Cc: Ji, Kai <kai.ji@intel.com>; O'Sullivan, Kevin <kevin.osullivan@intel.com>;
> Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: RE: [EXT] [PATCH v2 0/2] crypto/scheduler: add support for security
> protocols
> 
> Hi Anoob,
> 
> Thank you for that feedback - I was on extended leave so only just getting
> back to it now.
> See replies below.
> 
> Regards,
> David
> 
> > -----Original Message-----
> > From: Anoob Joseph <anoobj@marvell.com>
> > Sent: Friday, August 11, 2023 12:09 PM
> > To: Coyle, David <david.coyle@intel.com>; dev@dpdk.org
> > Cc: Ji, Kai <kai.ji@intel.com>; O'Sullivan, Kevin
> > <kevin.osullivan@intel.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>
> > Subject: RE: [EXT] [PATCH v2 0/2] crypto/scheduler: add support for
> > security protocols
> >
> > Hi David,
> >
> > While it is desirable to add security under crypto/scheduler, would it
> > be functionally possible if the PMDs perform stateful processing? For
> > example, with lookaside protocol mode of IPsec, fields such as seq no
> > & AR defines how the crypto operation can be performed. Without two
> > PMDs sharing this (actively), how can the load balancing happen?
> 
> [DC] So if some fields such as seq numbers are maintained within the PMDs
> for some protocols, then yes you are right - this would not work without
> some synchronization across PMD instances which I think we'd want to avoid
> at this point.
> 
> I tried to find some cases where a crypto PMD that supports IPSec, for
> example, maintains some global stateful parameters, but I could not find
> these cases.
> I'm not at all familiar with these PMDs (cnxk, mvsam, dpaa_sec, dpaa2_sec)
> though, so maybe you could guide me as to where they are maintained?

[Anoob] I can comment about cnxk. 

In cn9k, PMD updates the states.
https://elixir.bootlin.com/dpdk/v23.07/source/drivers/crypto/cnxk/cn9k_ipsec_la_ops.h#L177

In cn10k, hw updates the states. Please check the corresponding fields,
https://elixir.bootlin.com/dpdk/v23.07/source/drivers/common/cnxk/roc_ie_ot.h#L258

> 
> >
> > Said that, I agree utility of scheduler for stateless operations. My
> > understanding is, PDCP offload that is available today is not stateful
> > and that can leverage this. I'm not sure of DOCSIS and MACsec.
> 
> [DC] I notice that the PDCP security xform struct has a seq number related
> field, which would also suggest it could be stateful, but I could be wrong.

[Anoob] The field there is seq no size. That is not stateful. But then, it has HFN field which is the upper few bits of seq no. It is unclear if HFN is expected to be incremented when lower bits overflow. May be it's better PDCP is also left unsupported. I'll let Hemanth confirm.

> 
> From a google search MACSec is stateless, but again I'm not an expert.
> 
> The protocol I am familiar with is DOCSIS, and it is for this protocol that we
> have added security support to the cryptodev scheduler.
> DOCSIS is 100% stateless, so will work no problem with the scheduler.
> 
> >
> > Should we make it such that only specific security sessions would be
> > eligible for scheduler operation?
> 
> [DC] Do you think it would be acceptable to limit the scheduler to the DOCSIS
> protocol only for now, and let the IPSec, MACSec and PDCP experts add
> these later if applicable?
> If you think this would be ok, I can easily make that change.

[Anoob] I think that would be a good approach. For any stateless protocols, addition of crypto scheduler is a huge plus.

> 
> >
> > Thanks,
> > Anoob
> >
> > > -----Original Message-----
> > > From: David Coyle <david.coyle@intel.com>
> > > Sent: Friday, August 11, 2023 3:54 PM
> > > To: dev@dpdk.org
> > > Cc: kai.ji@intel.com; kevin.osullivan@intel.com; David Coyle
> > > <david.coyle@intel.com>
> > > Subject: [EXT] [PATCH v2 0/2] crypto/scheduler: add support for
> > > security protocols
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > -- This patchset adds support to the cryptodev scheduler PMD and
> > > unit tests for the existing security protocols in the security
> > > library, namely IPSec, MACSec, PDCP and DOCSIS.
> > >
> > > v2:
> > > * Improve inclusion of rte_security header files
> > > * Fix typo in commit message
> > >
> > > David Coyle (2):
> > >   crypto/scheduler: support security protocols
> > >   test/crypto: add security tests for cryptodev scheduler
> > >
> > >  app/test/test_cryptodev.c                     |  14 +-
> > >  doc/guides/rel_notes/release_23_11.rst        |   3 +
> > >  drivers/crypto/scheduler/meson.build          |   2 +-
> > >  .../scheduler/rte_cryptodev_scheduler.c       | 229 ++++++++++-
> > >  drivers/crypto/scheduler/scheduler_failover.c |  12 +-
> > >  .../crypto/scheduler/scheduler_multicore.c    |  10 +-
> > >  .../scheduler/scheduler_pkt_size_distr.c      |  54 +--
> > >  drivers/crypto/scheduler/scheduler_pmd.c      |  33 ++
> > >  drivers/crypto/scheduler/scheduler_pmd_ops.c  | 375
> > > +++++++++++++----- .../crypto/scheduler/scheduler_pmd_private.h  |
> > > +++++++++++++148
> > ++++---
> > >  .../crypto/scheduler/scheduler_roundrobin.c   |   6 +-
> > >  11 files changed, 656 insertions(+), 230 deletions(-)
> > >
> > > --
> > > 2.25.1