Re: [dpdk-dev] [PATCH v3 1/2] eal: add new rte color definition

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----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?