Hi Anoob,
Please see inline.
> >
> > +#include <rte_cryptodev_core.h>
>
> [Anoob] Is this intentional? Shouldn't we keep includes together in the top of
> the file?
>
It is intentional, and will be removed in a later patch, when the new framework
is ready.
> > +/**
> > + *
>
> [Anoob] Is there an extra blank line?
>
> > + * Dequeue a burst of processed crypto operations from a queue on the
> > +crypto
> > + * device. The dequeued operation are stored in *rte_crypto_op*
> > +structures
> > + * whose pointers are supplied in the *ops* array.
> > + *
> > + * The rte_cryptodev_dequeue_burst() function returns the number of
> ops
> > + * actually dequeued, which is the number of *rte_crypto_op* data
> > +structures
> > + * effectively supplied into the *ops* array.
> > + *
> > + * A return value equal to *nb_ops* indicates that the queue contained
> > + * at least *nb_ops* operations, and this is likely to signify that
> > +other
> > + * processed operations remain in the devices output queue.
> > +Applications
> > + * implementing a "retrieve as many processed operations as possible"
> > +policy
> > + * can check this specific case and keep invoking the
> > + * rte_cryptodev_dequeue_burst() function until a value less than
> > + * *nb_ops* is returned.
> > + *
> > + * The rte_cryptodev_dequeue_burst() function does not provide any
> > +error
> > + * notification to avoid the corresponding overhead.
> > + *
> > + * @param dev_id The symmetric crypto device identifier
>
> [Anoob] I do realize this is copied from existing code, but "symmetric" should
> not be there in the above string, right? The API is equally applicable for all
> cryptodev operations, right?
Agreed, I did not realize it was like this till now. This code will be removed
In a later patch of the series, but will check the final thing.
>
> > +
> > +
>
> [Anoob] Couple of extra blank lines can be removed?
This code is removed in a later patch.
>
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/cryptodev/rte_cryptodev_core.h
> > b/lib/cryptodev/rte_cryptodev_core.h
> > new file mode 100644
> > index 0000000000..1633e55889
> > --- /dev/null
> > +++ b/lib/cryptodev/rte_cryptodev_core.h
> > @@ -0,0 +1,100 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(C) 2021 Marvell.
>
> [Anoob] Since the code is mostly a copy from rte_cryptodev.h shouldn't we
> retain original licenses also?
This is an intermediate patch. At the end of this patchset, the new file will
be
A very small file with stuff only related to the new framework and will not have
Anything copied from other files, hence did not mention previous licenses.
>
> > + */
> > +
> > +#ifndef _RTE_CRYPTODEV_CORE_H_
> > +#define _RTE_CRYPTODEV_CORE_H_
>
> [Anoob] We are not including any of the dependent headers in this file. So
> the caller would be required to include all the dependent headers. Might be
> better to include atleast basic required ones (for rte_crypto_op etc).
This file is not required to be included directly as mentioned at the top of
the file.
Hence no need to include other header files.
>
> > +/** @internal The data structure associated with each crypto device. */
> > +struct rte_cryptodev {
> > + dequeue_pkt_burst_t dequeue_burst;
> > + /**< Pointer to PMD receive function. */
> > + enqueue_pkt_burst_t enqueue_burst;
> > + /**< Pointer to PMD transmit function. */
>
> [Anoob] Transmit & receive are more ethdev specific terminology, right? Why
> not enqueue/dequeue itself?
This is kind of a legacy code, I just copied. We need not correct here, as this
code
Will be removed in a later patch.
