Re: [PATCH] common/idpf: remove unnecessary compile option

Re: [PATCH] common/idpf: remove unnecessary compile option

[Stephen Hemminger]

On Mon, 24 Apr 2023 18:47:00 -0400
Qi Zhang <qi.z.zhang@intel.com> wrote:

> Remove compile option "__KERNEL" which should not be considered in
> DPDK. Also only #include <rte_xxx> in idpf_osdep.h.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

This will cause some warnings in DPDK build depending on compiler
version and flags.  You need to leave the fallthrough comment or
use one of the other fallthrough annotations.

Re: [PATCH] common/idpf: remove unnecessary compile option

[Tyler Retzlaff]

On Mon, Apr 24, 2023 at 09:23:48AM -0700, Stephen Hemminger wrote:
> On Mon, 24 Apr 2023 18:47:00 -0400
> Qi Zhang <qi.z.zhang@intel.com> wrote:
> 
> > Remove compile option "__KERNEL" which should not be considered in
> > DPDK. Also only #include <rte_xxx> in idpf_osdep.h.
> > 
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> This will cause some warnings in DPDK build depending on compiler
> version and flags.  You need to leave the fallthrough comment or
> use one of the other fallthrough annotations.

if there are variations of annotating fallthrough i would not object to
a macro for it being exposed from rte_common.h

full disclosure it would let me use the equivalent that are provided
with windows and windows toolchains.

Re: [PATCH] common/idpf: remove unnecessary compile option

[Stephen Hemminger]

On Mon, 24 Apr 2023 10:29:19 -0700
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:

> On Mon, Apr 24, 2023 at 09:23:48AM -0700, Stephen Hemminger wrote:
> > On Mon, 24 Apr 2023 18:47:00 -0400
> > Qi Zhang <qi.z.zhang@intel.com> wrote:
> >   
> > > Remove compile option "__KERNEL" which should not be considered in
> > > DPDK. Also only #include <rte_xxx> in idpf_osdep.h.
> > > 
> > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>  
> > 
> > This will cause some warnings in DPDK build depending on compiler
> > version and flags.  You need to leave the fallthrough comment or
> > use one of the other fallthrough annotations.  
> 
> if there are variations of annotating fallthrough i would not object to
> a macro for it being exposed from rte_common.h
> 
> full disclosure it would let me use the equivalent that are provided
> with windows and windows toolchains.

Yes having something like __rte_fallthrough would help.
Wouldn't help code that is trying to always work in kernel, DPDK or other places.

Both Gcc and clang use statement attributes and C++ has [[fallthrough]]

https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html
https://clang.llvm.org/docs/AttributeReference.html#fallthrough

[PATCH] common/idpf: remove unnecessary compile option

[Qi Zhang]

Remove compile option "__KERNEL" which should not be considered in
DPDK. Also only #include <rte_xxx> in idpf_osdep.h.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/common/idpf/base/idpf_controlq.c     | 5 -----
 drivers/common/idpf/base/idpf_controlq.h     | 8 --------
 drivers/common/idpf/base/idpf_controlq_api.h | 6 ------
 drivers/common/idpf/base/idpf_lan_txrx.h     | 3 +--
 drivers/common/idpf/base/idpf_osdep.h        | 1 +
 5 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/common/idpf/base/idpf_controlq.c b/drivers/common/idpf/base/idpf_controlq.c
index 3af81e5a64..93a3a20fd1 100644
--- a/drivers/common/idpf/base/idpf_controlq.c
+++ b/drivers/common/idpf/base/idpf_controlq.c
@@ -162,11 +162,6 @@ int idpf_ctlq_add(struct idpf_hw *hw,
 	switch (qinfo->type) {
 	case IDPF_CTLQ_TYPE_MAILBOX_RX:
 		is_rxq = true;
-#ifdef __KERNEL__
-		fallthrough;
-#else
-		/* fallthrough */
-#endif /* __KERNEL__ */
 	case IDPF_CTLQ_TYPE_MAILBOX_TX:
 		status = idpf_ctlq_alloc_ring_res(hw, *cq_out);
 		break;
diff --git a/drivers/common/idpf/base/idpf_controlq.h b/drivers/common/idpf/base/idpf_controlq.h
index e7b0d803b3..fea8dda618 100644
--- a/drivers/common/idpf/base/idpf_controlq.h
+++ b/drivers/common/idpf/base/idpf_controlq.h
@@ -5,14 +5,8 @@
 #ifndef _IDPF_CONTROLQ_H_
 #define _IDPF_CONTROLQ_H_
 
-#ifdef __KERNEL__
-#include <linux/slab.h>
-#endif
-
-#ifndef __KERNEL__
 #include "idpf_osdep.h"
 #include "idpf_alloc.h"
-#endif
 #include "idpf_controlq_api.h"
 
 /* Maximum buffer lengths for all control queue types */
@@ -26,14 +20,12 @@
 	((u16)((((R)->next_to_clean > (R)->next_to_use) ? 0 : (R)->ring_size) + \
 	       (R)->next_to_clean - (R)->next_to_use - 1))
 
-#ifndef __KERNEL__
 /* Data type manipulation macros. */
 #define IDPF_HI_DWORD(x)	((u32)((((x) >> 16) >> 16) & 0xFFFFFFFF))
 #define IDPF_LO_DWORD(x)	((u32)((x) & 0xFFFFFFFF))
 #define IDPF_HI_WORD(x)		((u16)(((x) >> 16) & 0xFFFF))
 #define IDPF_LO_WORD(x)		((u16)((x) & 0xFFFF))
 
-#endif
 /* Control Queue default settings */
 #define IDPF_CTRL_SQ_CMD_TIMEOUT	250  /* msecs */
 
diff --git a/drivers/common/idpf/base/idpf_controlq_api.h b/drivers/common/idpf/base/idpf_controlq_api.h
index 32d17baadf..e80debebb8 100644
--- a/drivers/common/idpf/base/idpf_controlq_api.h
+++ b/drivers/common/idpf/base/idpf_controlq_api.h
@@ -5,14 +5,8 @@
 #ifndef _IDPF_CONTROLQ_API_H_
 #define _IDPF_CONTROLQ_API_H_
 
-#ifdef __KERNEL__
-#include "idpf_mem.h"
-#else /* !__KERNEL__ */
 #include "idpf_osdep.h"
 
-#include <rte_compat.h>
-#endif /* !__KERNEL__ */
-
 struct idpf_hw;
 
 /* Used for queue init, response and events */
diff --git a/drivers/common/idpf/base/idpf_lan_txrx.h b/drivers/common/idpf/base/idpf_lan_txrx.h
index 98484b267c..2d635a0b9c 100644
--- a/drivers/common/idpf/base/idpf_lan_txrx.h
+++ b/drivers/common/idpf/base/idpf_lan_txrx.h
@@ -4,9 +4,8 @@
 
 #ifndef _IDPF_LAN_TXRX_H_
 #define _IDPF_LAN_TXRX_H_
-#ifndef __KERNEL__
+
 #include "idpf_osdep.h"
-#endif
 
 enum idpf_rss_hash {
 	/* Values 0 - 28 are reserved for future use */
diff --git a/drivers/common/idpf/base/idpf_osdep.h b/drivers/common/idpf/base/idpf_osdep.h
index 99ae9cf60a..78049e25b4 100644
--- a/drivers/common/idpf/base/idpf_osdep.h
+++ b/drivers/common/idpf/base/idpf_osdep.h
@@ -23,6 +23,7 @@
 #include <rte_log.h>
 #include <rte_random.h>
 #include <rte_io.h>
+#include <rte_compat.h>
 
 #define INLINE inline
 #define STATIC static
-- 
2.31.1

RE: [PATCH] common/idpf: remove unnecessary compile option

[Zhang, Qi Z]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, April 25, 2023 12:24 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH] common/idpf: remove unnecessary compile option
> 
> On Mon, 24 Apr 2023 18:47:00 -0400
> Qi Zhang <qi.z.zhang@intel.com> wrote:
> 
> > Remove compile option "__KERNEL" which should not be considered in
> > DPDK. Also only #include <rte_xxx> in idpf_osdep.h.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> This will cause some warnings in DPDK build depending on compiler version
> and flags.  You need to leave the fallthrough comment or use one of the
> other fallthrough annotations.

no intention of removing that comment, as it serves to assist the reader. 
It's good to learn that it has also helped to resolve the compiler warning - thank you for bringing this to my attention.

[PATCH v2] common/idpf: remove unnecessary compile option

[Qi Zhang]

Remove compile option "__KERNEL" which should not be considered in
DPDK. Also only #include <rte_xxx> in idpf_osdep.h.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
v2:
- add fallthrough comment back

 drivers/common/idpf/base/idpf_controlq.c     | 4 ----
 drivers/common/idpf/base/idpf_controlq.h     | 8 --------
 drivers/common/idpf/base/idpf_controlq_api.h | 6 ------
 drivers/common/idpf/base/idpf_lan_txrx.h     | 3 +--
 drivers/common/idpf/base/idpf_osdep.h        | 1 +
 5 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/common/idpf/base/idpf_controlq.c b/drivers/common/idpf/base/idpf_controlq.c
index 3af81e5a64..8adc727926 100644
--- a/drivers/common/idpf/base/idpf_controlq.c
+++ b/drivers/common/idpf/base/idpf_controlq.c
@@ -162,11 +162,7 @@ int idpf_ctlq_add(struct idpf_hw *hw,
 	switch (qinfo->type) {
 	case IDPF_CTLQ_TYPE_MAILBOX_RX:
 		is_rxq = true;
-#ifdef __KERNEL__
-		fallthrough;
-#else
 		/* fallthrough */
-#endif /* __KERNEL__ */
 	case IDPF_CTLQ_TYPE_MAILBOX_TX:
 		status = idpf_ctlq_alloc_ring_res(hw, *cq_out);
 		break;
diff --git a/drivers/common/idpf/base/idpf_controlq.h b/drivers/common/idpf/base/idpf_controlq.h
index e7b0d803b3..fea8dda618 100644
--- a/drivers/common/idpf/base/idpf_controlq.h
+++ b/drivers/common/idpf/base/idpf_controlq.h
@@ -5,14 +5,8 @@
 #ifndef _IDPF_CONTROLQ_H_
 #define _IDPF_CONTROLQ_H_
 
-#ifdef __KERNEL__
-#include <linux/slab.h>
-#endif
-
-#ifndef __KERNEL__
 #include "idpf_osdep.h"
 #include "idpf_alloc.h"
-#endif
 #include "idpf_controlq_api.h"
 
 /* Maximum buffer lengths for all control queue types */
@@ -26,14 +20,12 @@
 	((u16)((((R)->next_to_clean > (R)->next_to_use) ? 0 : (R)->ring_size) + \
 	       (R)->next_to_clean - (R)->next_to_use - 1))
 
-#ifndef __KERNEL__
 /* Data type manipulation macros. */
 #define IDPF_HI_DWORD(x)	((u32)((((x) >> 16) >> 16) & 0xFFFFFFFF))
 #define IDPF_LO_DWORD(x)	((u32)((x) & 0xFFFFFFFF))
 #define IDPF_HI_WORD(x)		((u16)(((x) >> 16) & 0xFFFF))
 #define IDPF_LO_WORD(x)		((u16)((x) & 0xFFFF))
 
-#endif
 /* Control Queue default settings */
 #define IDPF_CTRL_SQ_CMD_TIMEOUT	250  /* msecs */
 
diff --git a/drivers/common/idpf/base/idpf_controlq_api.h b/drivers/common/idpf/base/idpf_controlq_api.h
index 32d17baadf..e80debebb8 100644
--- a/drivers/common/idpf/base/idpf_controlq_api.h
+++ b/drivers/common/idpf/base/idpf_controlq_api.h
@@ -5,14 +5,8 @@
 #ifndef _IDPF_CONTROLQ_API_H_
 #define _IDPF_CONTROLQ_API_H_
 
-#ifdef __KERNEL__
-#include "idpf_mem.h"
-#else /* !__KERNEL__ */
 #include "idpf_osdep.h"
 
-#include <rte_compat.h>
-#endif /* !__KERNEL__ */
-
 struct idpf_hw;
 
 /* Used for queue init, response and events */
diff --git a/drivers/common/idpf/base/idpf_lan_txrx.h b/drivers/common/idpf/base/idpf_lan_txrx.h
index 98484b267c..2d635a0b9c 100644
--- a/drivers/common/idpf/base/idpf_lan_txrx.h
+++ b/drivers/common/idpf/base/idpf_lan_txrx.h
@@ -4,9 +4,8 @@
 
 #ifndef _IDPF_LAN_TXRX_H_
 #define _IDPF_LAN_TXRX_H_
-#ifndef __KERNEL__
+
 #include "idpf_osdep.h"
-#endif
 
 enum idpf_rss_hash {
 	/* Values 0 - 28 are reserved for future use */
diff --git a/drivers/common/idpf/base/idpf_osdep.h b/drivers/common/idpf/base/idpf_osdep.h
index 49bd7c4b21..3703421575 100644
--- a/drivers/common/idpf/base/idpf_osdep.h
+++ b/drivers/common/idpf/base/idpf_osdep.h
@@ -23,6 +23,7 @@
 #include <rte_log.h>
 #include <rte_random.h>
 #include <rte_io.h>
+#include <rte_compat.h>
 
 #define INLINE inline
 #define STATIC static
-- 
2.31.1

RE: [PATCH v2] common/idpf: remove unnecessary compile option

[Xing, Beilei]

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Wednesday, April 26, 2023 11:39 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: [PATCH v2] common/idpf: remove unnecessary compile option
> 
> Remove compile option "__KERNEL" which should not be considered in DPDK.
> Also only #include <rte_xxx> in idpf_osdep.h.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Acked-by: Beilei Xing <beilei.xing@intel.com>

RE: [PATCH v2] common/idpf: remove unnecessary compile option

[Zhang, Qi Z]

> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Thursday, April 27, 2023 5:36 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2] common/idpf: remove unnecessary compile option
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: Wednesday, April 26, 2023 11:39 PM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> > Subject: [PATCH v2] common/idpf: remove unnecessary compile option
> >
> > Remove compile option "__KERNEL" which should not be considered in
> DPDK.
> > Also only #include <rte_xxx> in idpf_osdep.h.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Acked-by: Beilei Xing <beilei.xing@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi