common/idpf: remove unnecessary compile option

Message ID 20230424224700.997910-1-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series common/idpf: remove unnecessary compile option |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Qi Zhang April 24, 2023, 10:47 p.m. UTC
  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(-)
  

Comments

Stephen Hemminger April 24, 2023, 4:23 p.m. UTC | #1
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.
  
Tyler Retzlaff April 24, 2023, 5:29 p.m. UTC | #2
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.
  
Stephen Hemminger April 24, 2023, 5:41 p.m. UTC | #3
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
  
Qi Zhang April 26, 2023, 7:12 a.m. UTC | #4
> -----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

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