Re: [dpdk-dev] [PATCH v2 06/12] cryptodev: move vdev functions to a separate file

[Thomas Monjalon <thomas@monjalon.net>]

27/06/2017 17:11, Ferruh Yigit:
> On 6/27/2017 3:28 PM, Thomas Monjalon wrote:
> > 27/06/2017 11:51, De Lara Guarch, Pablo:
> >> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> >>> 23/06/2017 14:52, Declan Doherty:
> >>>> On 21/06/2017 7:28 AM, Pablo de Lara wrote:
> >>>>> Move all functions handling virtual devices to a separate header
> >>>>> file "rte_cryptodev_vdev.h", in order to leave only generic
> >>>>> functions for any device in the rest of the files.
> >>>>>
> >>>>> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >>>>
> >>>> Acked-by: Declan Doherty <declan.doherty@intel.com>
> >>>
> >>> This patch is moving some code from a .c to a .h.
> >>> The consequence is that driver are now compiling new functions from the .h
> >>> and require librte_kvargs to link with.
> >>> It is a build error in shared lib mode.
> >>>
> >>> Please keep the code in a .c file if possible.
> >>
> >> Sorry about this. I just fixed it in the subtree, so pull it whenever you are ready.
> >> Also, since ethdev have separated pci and vdev functions in separate .h files,
> >> I preferred to be consistent and do the same.
> > 
> > There is no reason to implement these functions in .h files.
> > I prefer to keep them in a .c file and just export the proto in .h.
> 
> This is same in the eth_dev too.
> 
> These are helper functions, and implementing them in .h file has the
> benefit of function goes into PMD library, and there is no dependency
> from PMDs to librte_ether for these functions (there are dependencies
> for other functions ..).
> 
> It looks acceptable for me for helper functions to be implemented in
> header file. But if we decide to move them into .c file, .map file
> should be updated to export them, and I suppose ethdev also should be
> updated to be compatible.

Yes we should avoid adding code in .h.
It is accepted only for performance reasons.
ethdev can be reworked later.
cryptodev can be improved now and avoid adding rte_devargs deps on PMDs.