[v3,1/2] eal: add new rte color definition

Message ID 20181213180851.4862-1-reshma.pattan@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Cristian Dumitrescu
Headers
Series [v3,1/2] eal: add new rte color definition |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Pattan, Reshma Dec. 13, 2018, 6:08 p.m. UTC
  Added new rte_color definition in eal to
consolidate color definition which is currently replicated
in various places such as rte_meter.h, rte_tm.h and rte_mtr.h

Created aliases for rte_tm_color, rte_mtr_color and rte_meter_color
to use new rte_color values.

The definitions of rte_tm_color, rte_mtr_color and rte_meter_color
will be deprecated in future.

Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
v2: Addressed review comments given in the below link
http://patches.dpdk.org/patch/48588/
Added rte_color.h to meson build.
---
---
 lib/librte_eal/common/Makefile            |  2 +-
 lib/librte_eal/common/include/rte_color.h | 26 +++++++++++++++++++++++
 lib/librte_eal/common/meson.build         |  1 +
 lib/librte_ethdev/rte_mtr.h               |  8 +++++++
 lib/librte_ethdev/rte_tm.h                |  8 +++++++
 lib/librte_meter/rte_meter.h              |  8 +++++++
 6 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_eal/common/include/rte_color.h
  

Comments

Cristian Dumitrescu Dec. 14, 2018, 10:19 p.m. UTC | #1
> -----Original Message-----
> From: Pattan, Reshma
> Sent: Thursday, December 13, 2018 6:09 PM
> To: dev@dpdk.org; jerin.jacob@caviumnetworks.com; Rao, Nikhil
> <nikhil.rao@intel.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: Pattan, Reshma <reshma.pattan@intel.com>
> Subject: [PATCH v3 1/2] eal: add new rte color definition
> 
> Added new rte_color definition in eal to
> consolidate color definition which is currently replicated
> in various places such as rte_meter.h, rte_tm.h and rte_mtr.h
> 
> Created aliases for rte_tm_color, rte_mtr_color and rte_meter_color
> to use new rte_color values.
> 
> The definitions of rte_tm_color, rte_mtr_color and rte_meter_color
> will be deprecated in future.
> 
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
> v2: Addressed review comments given in the below link
> http://patches.dpdk.org/patch/48588/
> Added rte_color.h to meson build.
> ---
> ---
>  lib/librte_eal/common/Makefile            |  2 +-
>  lib/librte_eal/common/include/rte_color.h | 26
> +++++++++++++++++++++++
>  lib/librte_eal/common/meson.build         |  1 +
>  lib/librte_ethdev/rte_mtr.h               |  8 +++++++
>  lib/librte_ethdev/rte_tm.h                |  8 +++++++
>  lib/librte_meter/rte_meter.h              |  8 +++++++
>  6 files changed, 52 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_eal/common/include/rte_color.h
> 
> diff --git a/lib/librte_eal/common/Makefile
> b/lib/librte_eal/common/Makefile
> index 87d8c455d..c76bb2dd7 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -17,7 +17,7 @@ INC += rte_pci_dev_feature_defs.h
> rte_pci_dev_features.h
>  INC += rte_malloc.h rte_keepalive.h rte_time.h
>  INC += rte_service.h rte_service_component.h
>  INC += rte_bitmap.h rte_vfio.h rte_hypervisor.h rte_test.h
> -INC += rte_reciprocal.h rte_fbarray.h rte_uuid.h
> +INC += rte_reciprocal.h rte_fbarray.h rte_uuid.h rte_color.h
> 
>  GENERIC_INC := rte_atomic.h rte_byteorder.h rte_cycles.h rte_prefetch.h
>  GENERIC_INC += rte_spinlock.h rte_memcpy.h rte_cpuflags.h rte_rwlock.h
> diff --git a/lib/librte_eal/common/include/rte_color.h
> b/lib/librte_eal/common/include/rte_color.h
> new file mode 100644
> index 000000000..fafe1146d
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_color.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#ifndef __INCLUDE_RTE_COLOR_H__
> +#define __INCLUDE_RTE_COLOR_H__
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * Color
> + */
> +enum rte_color {
> +	RTE_COLOR_GREEN = 0, /**< Green */
> +	RTE_COLOR_YELLOW, /**< Yellow */
> +	RTE_COLOR_RED, /**< Red */
> +	RTE_COLORS /**< Number of colors */
> +};
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __RTE_COLOR_H__ */
> diff --git a/lib/librte_eal/common/meson.build
> b/lib/librte_eal/common/meson.build
> index 2a10d57d8..f0a561f21 100644
> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> @@ -52,6 +52,7 @@ common_headers = files(
>  	'include/rte_bus.h',
>  	'include/rte_bitmap.h',
>  	'include/rte_class.h',
> +	'include/rte_color.h',
>  	'include/rte_common.h',
>  	'include/rte_debug.h',
>  	'include/rte_devargs.h',
> diff --git a/lib/librte_ethdev/rte_mtr.h b/lib/librte_ethdev/rte_mtr.h
> index c4819b274..1de20ef2b 100644
> --- a/lib/librte_ethdev/rte_mtr.h
> +++ b/lib/librte_ethdev/rte_mtr.h
> @@ -76,6 +76,7 @@
>  #include <stdint.h>
>  #include <rte_compat.h>
>  #include <rte_common.h>
> +#include <rte_color.h>
> 
>  #ifdef __cplusplus
>  extern "C" {
> @@ -91,6 +92,13 @@ enum rte_mtr_color {
>  	RTE_MTR_COLORS /**< Number of colors. */
>  };
> 
> +/* New rte_color is defined and used to deprecate rte_mtr_color soon. */
> +#define rte_mtr_color rte_color
> +#define RTE_MTR_GREEN RTE_COLOR_GREEN
> +#define RTE_MTR_YELLOW RTE_COLOR_YELLOW
> +#define RTE_MTR_RED RTE_COLOR_RED
> +#define RTE_MTR_COLORS RTE_COLORS
> +
>  /**
>   * Statistics counter type
>   */
> diff --git a/lib/librte_ethdev/rte_tm.h b/lib/librte_ethdev/rte_tm.h
> index 646ef3880..cce8230b6 100644
> --- a/lib/librte_ethdev/rte_tm.h
> +++ b/lib/librte_ethdev/rte_tm.h
> @@ -51,6 +51,7 @@
>  #include <stdint.h>
> 
>  #include <rte_common.h>
> +#include <rte_color.h>
> 
>  #ifdef __cplusplus
>  extern "C" {
> @@ -125,6 +126,13 @@ enum rte_tm_color {
>  	RTE_TM_COLORS /**< Number of colors */
>  };
> 
> +/* New rte_color is defined and used to deprecate rte_tm_color soon. */
> +#define rte_tm_color rte_color
> +#define RTE_TM_GREEN RTE_COLOR_GREEN
> +#define RTE_TM_YELLOW RTE_COLOR_YELLOW
> +#define RTE_TM_RED RTE_COLOR_RED
> +#define RTE_TM_COLORS RTE_COLORS
> +
>  /**
>   * Node statistics counter type
>   */
> diff --git a/lib/librte_meter/rte_meter.h b/lib/librte_meter/rte_meter.h
> index 58a051583..e8d878b12 100644
> --- a/lib/librte_meter/rte_meter.h
> +++ b/lib/librte_meter/rte_meter.h
> @@ -21,6 +21,7 @@ extern "C" {
> 
>  #include <stdint.h>
> 
> +#include <rte_color.h>
>  /*
>   * Application Programmer's Interface (API)
>   *
> @@ -34,6 +35,13 @@ enum rte_meter_color {
>  	e_RTE_METER_COLORS     /**< Number of available colors */
>  };
> 
> +/* New rte_color is defined and used to deprecate rte_meter_color soon.
> */
> +#define rte_meter_color rte_color
> +#define e_RTE_METER_GREEN RTE_COLOR_GREEN
> +#define e_RTE_METER_YELLOW RTE_COLOR_YELLOW
> +#define e_RTE_METER_RED RTE_COLOR_RED
> +#define e_RTE_METER_COLORS RTE_COLORS
> +
>  /** srTCM parameters per metered traffic flow. The CIR, CBS and EBS
> parameters only
>  count bytes of IP packets and do not include link specific headers. At least
> one of
>  the CBS or EBS parameters has to be greater than zero. */
> --
> 2.17.1

This makes sense to me, but you need to remove the definitions of the existing enums (rte_mtr_color, rte_tm_color, rte_meter_color) first, otherwise what is the purpose of aliasing them to the new rte_color enum through the above macros?
  
Thomas Monjalon Dec. 17, 2018, 8:35 p.m. UTC | #2
13/12/2018 19:08, Reshma Pattan:
> Added new rte_color definition in eal to
> consolidate color definition which is currently replicated
> in various places such as rte_meter.h, rte_tm.h and rte_mtr.h

I don't think EAL is the right place for such definition.
Why ethdev would not rely on definitions from rte_meter.h?
  

Patch

diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index 87d8c455d..c76bb2dd7 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -17,7 +17,7 @@  INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
 INC += rte_malloc.h rte_keepalive.h rte_time.h
 INC += rte_service.h rte_service_component.h
 INC += rte_bitmap.h rte_vfio.h rte_hypervisor.h rte_test.h
-INC += rte_reciprocal.h rte_fbarray.h rte_uuid.h
+INC += rte_reciprocal.h rte_fbarray.h rte_uuid.h rte_color.h
 
 GENERIC_INC := rte_atomic.h rte_byteorder.h rte_cycles.h rte_prefetch.h
 GENERIC_INC += rte_spinlock.h rte_memcpy.h rte_cpuflags.h rte_rwlock.h
diff --git a/lib/librte_eal/common/include/rte_color.h b/lib/librte_eal/common/include/rte_color.h
new file mode 100644
index 000000000..fafe1146d
--- /dev/null
+++ b/lib/librte_eal/common/include/rte_color.h
@@ -0,0 +1,26 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#ifndef __INCLUDE_RTE_COLOR_H__
+#define __INCLUDE_RTE_COLOR_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Color
+ */
+enum rte_color {
+	RTE_COLOR_GREEN = 0, /**< Green */
+	RTE_COLOR_YELLOW, /**< Yellow */
+	RTE_COLOR_RED, /**< Red */
+	RTE_COLORS /**< Number of colors */
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __RTE_COLOR_H__ */
diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 2a10d57d8..f0a561f21 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -52,6 +52,7 @@  common_headers = files(
 	'include/rte_bus.h',
 	'include/rte_bitmap.h',
 	'include/rte_class.h',
+	'include/rte_color.h',
 	'include/rte_common.h',
 	'include/rte_debug.h',
 	'include/rte_devargs.h',
diff --git a/lib/librte_ethdev/rte_mtr.h b/lib/librte_ethdev/rte_mtr.h
index c4819b274..1de20ef2b 100644
--- a/lib/librte_ethdev/rte_mtr.h
+++ b/lib/librte_ethdev/rte_mtr.h
@@ -76,6 +76,7 @@ 
 #include <stdint.h>
 #include <rte_compat.h>
 #include <rte_common.h>
+#include <rte_color.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -91,6 +92,13 @@  enum rte_mtr_color {
 	RTE_MTR_COLORS /**< Number of colors. */
 };
 
+/* New rte_color is defined and used to deprecate rte_mtr_color soon. */
+#define rte_mtr_color rte_color
+#define RTE_MTR_GREEN RTE_COLOR_GREEN
+#define RTE_MTR_YELLOW RTE_COLOR_YELLOW
+#define RTE_MTR_RED RTE_COLOR_RED
+#define RTE_MTR_COLORS RTE_COLORS
+
 /**
  * Statistics counter type
  */
diff --git a/lib/librte_ethdev/rte_tm.h b/lib/librte_ethdev/rte_tm.h
index 646ef3880..cce8230b6 100644
--- a/lib/librte_ethdev/rte_tm.h
+++ b/lib/librte_ethdev/rte_tm.h
@@ -51,6 +51,7 @@ 
 #include <stdint.h>
 
 #include <rte_common.h>
+#include <rte_color.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -125,6 +126,13 @@  enum rte_tm_color {
 	RTE_TM_COLORS /**< Number of colors */
 };
 
+/* New rte_color is defined and used to deprecate rte_tm_color soon. */
+#define rte_tm_color rte_color
+#define RTE_TM_GREEN RTE_COLOR_GREEN
+#define RTE_TM_YELLOW RTE_COLOR_YELLOW
+#define RTE_TM_RED RTE_COLOR_RED
+#define RTE_TM_COLORS RTE_COLORS
+
 /**
  * Node statistics counter type
  */
diff --git a/lib/librte_meter/rte_meter.h b/lib/librte_meter/rte_meter.h
index 58a051583..e8d878b12 100644
--- a/lib/librte_meter/rte_meter.h
+++ b/lib/librte_meter/rte_meter.h
@@ -21,6 +21,7 @@  extern "C" {
 
 #include <stdint.h>
 
+#include <rte_color.h>
 /*
  * Application Programmer's Interface (API)
  *
@@ -34,6 +35,13 @@  enum rte_meter_color {
 	e_RTE_METER_COLORS     /**< Number of available colors */
 };
 
+/* New rte_color is defined and used to deprecate rte_meter_color soon. */
+#define rte_meter_color rte_color
+#define e_RTE_METER_GREEN RTE_COLOR_GREEN
+#define e_RTE_METER_YELLOW RTE_COLOR_YELLOW
+#define e_RTE_METER_RED RTE_COLOR_RED
+#define e_RTE_METER_COLORS RTE_COLORS
+
 /** srTCM parameters per metered traffic flow. The CIR, CBS and EBS parameters only
 count bytes of IP packets and do not include link specific headers. At least one of
 the CBS or EBS parameters has to be greater than zero. */