Re: [dpdk-dev] [PATCH 2/5] cxgbe: add cxgbe poll mode driver.

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 26 May 2015 22:32:07 +0530
Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> wrote:

> On Sat, May 23, 2015 at 11:27:56 +0530, Rahul Lakkireddy wrote:
> > On Fri, May 22, 2015 at 09:42:50 -0700, Stephen Hemminger wrote:
> > > On Fri, 22 May 2015 18:54:20 +0530
> > > Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> wrote:
> > > 
> > > > +#define pr_err(y, args...) dev_err(0, y, ##args)
> > > > +#define pr_warn(y, args...) dev_warn(0, y, ##args)
> > > > +#define pr_info(y, args...) dev_info(0, y, ##args)
> > > > +#define BUG() pr_err("BUG at %s:%d", __func__, __LINE__)
> > > > +
> > > > +#define ASSERT(x) do {\
> > > > +	if (!(x)) \
> > > > +		rte_panic("CXGBE: x"); \
> > > > +} while (0)
> > > > +#define BUG_ON(x) ASSERT(!(x))
> > > > +
> > > > +#ifndef WARN_ON
> > > > +#define WARN_ON(x) do { \
> > > > +	int ret = !!(x); \
> > > > +	if (unlikely(ret)) \
> > > > +		pr_warn("WARN_ON: \"" #x "\" at %s:%d\n", __func__, __LINE__); \
> > > > +} while (0)
> > > > +#endif
> > > > +
> > > > +#define __iomem
> > > > +
> > > > +#ifndef BIT
> > > > +#define BIT(n) (1 << (n))
> > > > +#endif
> > > > +
> > > > +#define L1_CACHE_SHIFT  6
> > > > +#define L1_CACHE_BYTES  BIT(L1_CACHE_SHIFT)
> > > > +
> > > > +#define PAGE_SHIFT  12
> > > > +#define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
> > > > +#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
> > > > +
> > > > +#define VLAN_HLEN 4
> > > > +
> > > > +#define rmb()     rte_rmb() /* dpdk rte provided rmb */
> > > > +#define wmb()     rte_wmb() /* dpdk rte provided wmb */
> > > > +
> > > > +typedef uint8_t   u8;
> > > > +typedef int8_t    s8;
> > > > +typedef uint16_t  u16;
> > > > +typedef uint32_t  u32;
> > > > +typedef int32_t   s32;
> > > > +typedef uint64_t  u64;
> > > > +typedef int       bool;
> > > > +typedef uint64_t  dma_addr_t;
> > > > +
> > > > +#ifndef __le16
> > > > +#define __le16	uint16_t
> > > > +#endif
> > > > +#ifndef __le32
> > > > +#define __le32	uint32_t
> > > > +#endif
> > > > +#ifndef __le64
> > > > +#define __le64	uint64_t
> > > > +#endif
> > > > +#ifndef __be16
> > > > +#define __be16	uint16_t
> > > > +#endif
> > > > +#ifndef __be32
> > > > +#define __be32	uint32_t
> > > > +#endif
> > > > +#ifndef __be64
> > > > +#define __be64	uint64_t
> > > > +#endif
> > > > +#ifndef __u8
> > > > +#define __u8	uint8_t
> > > > +#endif
> > > > +#ifndef __u16
> > > > +#define __u16	uint16_t
> > > > +#endif
> > > > +#ifndef __u32
> > > > +#define __u32	uint32_t
> > > > +#endif
> > > > +#ifndef __u64
> > > > +#define __u64	uint64_t
> > > > +#endif
> > > > +
> > > > +#define FALSE	0
> > > > +#define TRUE	1
> > > > +#define false	0
> > > > +#define true	1
> > > > +
> > > > +#define min(a, b) RTE_MIN(a, b)
> > > > +#define max(a, b) RTE_MAX(a, b)
> > > 
> > > This is not Linux kernel.
> > > Please don't create wrappers for all the stuff in Linux to port your driver.
> > 
> > We actually referred several PMD's compat file including - enic_compat.h,
> > i40e_osdep.h, ixgbe_osdep.h, fm10k_osdep.h, etc.
> > 
> > Most of the types above are already defined by many of existing PMD's compat
> > file.  Can we at-least keep those which are already defined by several PMD's
> > compat file?
> 
> Just to give a background - since we are new to dpdk community, we studied the
> already merged PMD's compat files as reference to understand how things are
> done for driver submission. And so, we wrote cxgbe compat file along similar
> lines. However, if above wrappers are not acceptable then, we will definitely
> remove them in V2.
> 
> Just trying to get a clarification so that we don't repeat the same mistake in
> V2 submission. Reviews from you and dpdk community are more than welcome and
> appreciated.

Does this driver share source code with other platforms? If it does then the
compatibility wrappers make sense and reduce the maintenance effort.
If the driver is a standalone port to DPDK, then it makes sense to complete
the effort and use standard DPDK coding practices (stdint, stdbool, etc).

The other drivers in DPDK do things based on that. Many of the hardware
drivers share code with BSD. Others like the virtual drivers were written
or ported completely from scratch.