[dpdk-dev] [PATCH v7 2/2] cryptodev: change burst API to be crypto op oriented

[Trahe, Fiona]

Hi Thomas,

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Trahe, Fiona
> Sent: Wednesday, March 09, 2016 12:56 PM
> To: Thomas Monjalon
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v7 2/2] cryptodev: change burst API to be crypto
> op oriented
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > Sent: Tuesday, March 08, 2016 2:32 PM
> > To: Trahe, Fiona
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v7 2/2] cryptodev: change burst API to
> > be crypto op oriented
> >
> > 2016-03-07 11:50, Fiona Trahe:
> > > This patch modifies the crypto burst enqueue/dequeue APIs to operate
> > > on bursts rte_crypto_op's rather than the current implementation
> > > which operates on rte_mbuf bursts, this simplifies the burst
> > > processing in the crypto PMDs and the use of crypto operations in general.
> > >
> > > The changes also continues the separatation of the symmetric
> > > operation parameters from the more general operation parameters,
> > > this will simplify the integration of asymmetric crypto operations in the
> future.
> > >
> > > As well as the changes to the crypto APIs this patch adds functions
> > > for managing rte_crypto_op pools to the cryptodev API. It modifies
> > > the existing PMDs, unit tests and sample application to work with
> > > the modified APIs and finally removes the now unused rte_mbuf_offload
> library.
> >
> > Why not doing several patches?
> >
We will post v8 patchset today with more granular patches 
  
> > > -Packet buffer offload - EXPERIMENTAL
> > > -M: Declan Doherty <declan.doherty at intel.com>
> > > -F: lib/librte_mbuf_offload/
> >
> > Removing a library is important. It is not mentioned in the message.
> > It deserves a separate commit, please.
> >
ok

> > > @@ -62,8 +61,7 @@ struct crypto_unittest_params {
> > >
> > >  	struct rte_cryptodev_sym_session *sess;
> > >
> > > -	struct rte_mbuf_offload *ol;
> > > -	struct rte_crypto_sym_op *op;
> > > +	struct rte_crypto_op *op;
> >
> > Isn't it something which was just renamed in the previous patch?
It looks like a double rename, but it's more than that.
In first patch rte_crypto_op was renamed rte_crypto_sym_op and moved from rte_crypto.h to to rte_crypto_sym.h
because it was exclusively for symmetric operations.
In the later patch a more generic rte_crypto_op was introduced in rte_crypto.h which can handle various operation types by having a type and union. Initially the only type is symmetric and so the union points to an rte_crypto_sym_op but it's planned to extended to handle asymmetric.

> >
> > > -#if HEX_DUMP
> > > +#ifdef HEX_DUMP
> > >  static void
> > >  hexdump_mbuf_data(FILE *f, const char *title, struct rte_mbuf *m)
> >
> > A better clean-up would be to remove this ifdef.
> > If you need a debug function which is not already in EAL, you can
> > consider adding it.
> >
Agreed. We will look at adding the debug needed. However this is not likely to make it into the patchset today.

> 
> Hi Thomas,
> We're working on this. Will spin the patchset as soon as we can.
> Fiona