> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
> Sent: Friday, October 21, 2022 2:40 PM
> To: Guo, Junfeng <junfeng....@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>; Wu, Jingjing <jingjing...@intel.com>; Xing, Beilei
> <beilei.x...@intel.com>
> Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.w...@intel.com>
> Subject: Re: [PATCH v9 01/14] common/idpf: introduce common library
> 
> On 10/21/22 08:18, Junfeng Guo wrote:
> > Introduce common library for IDPF (Infrastructure Data Path Function)
> > PMD.
> >
> > Also add OS specific implementation about some MACRO definitions and
> > small functions which are specific for DPDK.
> 
> Common drivers are required when different class drivers need to share
> some code. So, it must be expalined here why do you create common driver
> instead of usage of base/ driver in your net driver.

Hi Andrew,
Thanks for all your comments.
The common driver will be also used in another PMD which should be upstream in 
23.03, so we create it.

> 
> Note that common driver is a DPDK driver and it must follow DPDK coding
> style. If the code is actually shared with something else and do not follow
> DPDK coding style from the very beginning (since it is an existing code), it
> should be in base/ subdir in either common driver or net driver.

The common driver is a BSD license release provided by Intel internal team, 
basically we won't change it. It's the same process as Intel other PMDs, such 
as base driver of iavf, etc.
Is it OK if it's in common/idpf/base/ folder?

> 
> Also you should not use own trivial wrappers for DPDK API in DPDK-specific
> code. It just complicates reading.
> E.g. BIT() vs RTE_BIT32().

Make sense, will try my best to address all your comments in the next version.
Thanks again.

> 
> So, I need an answer on above questions before I continue review.
> 
> >
> > Signed-off-by: Beilei Xing <beilei.x...@intel.com>
> > Signed-off-by: Xiao Wang <xiao.w.w...@intel.com>
> > Signed-off-by: Junfeng Guo <junfeng....@intel.com>
> 
> [snip]
> 
> > diff --git a/drivers/common/idpf/idpf_alloc.h
> > b/drivers/common/idpf/idpf_alloc.h
> > new file mode 100644
> > index 0000000000..bc054851b3
> > --- /dev/null
> > +++ b/drivers/common/idpf/idpf_alloc.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2001-2022 Intel Corporation  */
> > +
> > +#ifndef _IDPF_ALLOC_H_
> > +#define _IDPF_ALLOC_H_
> > +
> > +/* Memory types */
> 
> If it is a DPDK-specific driver and it is an interface provided by common 
> driver,
> it should use Doxygen-style comments to be a part of genereated API
> documentation.

Reply via email to