>
> On Fri, Mar 6, 2020 at 8:25 PM Coyle, David <david.co...@intel.com> wrote:
> >
> > > >
> > > > /** Error Detection Algorithms */
> > > > enum rte_rawdev_multi_fn_err_detect_algorithm {
> > > > RTE_RAWDEV_MULTI_FN_ERR_DETECT_CRC32_ETH,
> > >
> > > IMO, It does not make sense to add protocol specific stuff in rawdev
> > > symbols.
> > >
> > > IMO, It is better to have a separate library for CRC and BIP32
> > > acceleration like the rte_security library and underneath still it
> > > can use rawdev or anydev if required.
> >
> > [DC] This protocol stuff is only in the rawdev interface definition, which
> > is
> known only to the application and the rawdev PMDs which will use this
> interface.
> > So these defines/enums/structs etc for CRC and BIP are completely
> opaque to rte_rawdev itself.
> >
> > This is how all existing rawdev PMDs interfaces are defined, where the
> interface is very specific to the job(s) the PMD is implementing.
>
> If you see .map file in driver/raw/. None of the drivers are exposing any API
> with rte_rawdev_*.
> This addition will be exposing new rte_rawdev_* APIs from driver/rawdev/.
> That's is not correct.
>
> $ find drivers/raw/ -name *.map
> drivers/raw/skeleton/rte_rawdev_skeleton_version.map
> drivers/raw/octeontx2_ep/rte_rawdev_octeontx2_ep_version.map
> drivers/raw/ntb/rte_rawdev_ntb_version.map
> drivers/raw/dpaa2_qdma/rte_rawdev_dpaa2_qdma_version.map
> drivers/raw/dpaa2_cmdif/rte_rawdev_dpaa2_cmdif_version.map
> drivers/raw/ioat/rte_rawdev_ioat_version.map
> drivers/raw/octeontx2_dma/rte_rawdev_octeontx2_dma_version.map
> drivers/raw/ifpga/rte_rawdev_ifpga_version.map
>
> IMO, Correct thing to do will be,
>
> Either of
>
> 1) As mentioned below, If you would like to limit the scope only to a new
> rawdev driver then
> a) Create a new driver at driver/raw/<new driver>/
> b) expose the drier specific customer API as
> rte_<new-driver>_...(example:
> drivers/raw/dpaa2_qdma/rte_rawdev_dpaa2_qdma_version.map
>
> 2) If we would like to have public API then create a subsystem like
> libsecurity
> to have features. Let the API exposed from lib/...
>
[DC] Yes you are right here, it was incorrect to include rawdev in the
interface filename and in the symbols within... rawdev will be removed from all
these
And we are going with option 1 above, to limit this to the new rawdev drivers.
As I mentioned in the original post, if it is found that this interface could
be useful to other drivers/applications in the future, then it can be moved to
the public API under lib as a new library or an extension of an existing one
possibly
> >
> > Also, these particular defines/enums/structs for CRC and BIP are only for
> defining xform and op chains containing these particular operations.
> > The actual code to do the CRC and BIP is already in the AESNI-MB
> > library or DPDK rte_net_crc library, which our aesni_mb and qat rawdev
> > PMDs will call/use
> >
> > >
> > > IMO, Exposing the public API in
> > > drivers/raw/common/rte_rawdev_multi_fn.h is a shortcut.
> > > IMO, public API should be in lib/..
> >
> > [DC] To be honest, I tend to agree. I don't like that public APIs are
> > exposed
> from the drivers directory.
> > But as I mentioned above, this is how all rawdev PMD interfaces are
> > defined, where the interface definition is within the PMD directory
> > (e.g. drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h)
> > Our's is slightly different in that we have 2 PMDs which will use the
> > same interface, which is why we have added it in drivers/raw/common So
> > by keeping our interface under drivers, we are trying to be consistent
> > with all existing rawdev PMDs
> >
> > As I mentioned in my previous post though, this could potentially be
> > moved under lib in the future if other PMDs would find it useful
>
> See above. Point (1).
>
> >
> > We could possibly rename our interface file to rte_pmd_multi_fn.h to be a
> bit more consistent with the majority of the existing PMDs and take away the
> idea for now that this is some kind of extension to the main rte_rawdev API.
> > But unfortunately there is no full consistency in the rawdev PMD
> > interface filenames (e.g. dpaa2_cmdif uses the "rte_pmd_" prefix -
> > rte_pmd_dpaa2_cmdif.h, octeontx2_dma uses the "_rawdev" suffix -
> > otx2_dpi_rawdev.h)
> >
> > >
> > > Just my 2c.
