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