[v2,3/4] ethdev: get meter profile/policy objects

  Introduce a new Meter API to retrieve a Meter profile and policy
objects using the profile/policy ID previously created with
meter_profile_add() and meter_policy_create() functions.
That allows to save the pointer and avoid any lookups in the
corresponding lists for a quick access during a flow rule creation.
Also, it eliminates the need for CIR, CBS and EBS calculations
and conversion to a PMD-specific format when the profile is used.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
 .../traffic_metering_and_policing.rst         |  7 ++++
 doc/guides/rel_notes/release_22_07.rst        |  1 +
 lib/ethdev/rte_flow.h                         |  7 ++++
 lib/ethdev/rte_mtr.c                          | 41 +++++++++++++++++++
 lib/ethdev/rte_mtr.h                          | 40 ++++++++++++++++++
 lib/ethdev/rte_mtr_driver.h                   | 19 +++++++++
 lib/ethdev/version.map                        |  2 +
 7 files changed, 117 insertions(+)
  

Hi Alexander,

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Sunday, May 22, 2022 1:51 PM
> Subject: [PATCH v2 3/4] ethdev: get meter profile/policy objects
> 
> Introduce a new Meter API to retrieve a Meter profile and policy
> objects using the profile/policy ID previously created with
> meter_profile_add() and meter_policy_create() functions.
> That allows to save the pointer and avoid any lookups in the
> corresponding lists for a quick access during a flow rule creation.
> Also, it eliminates the need for CIR, CBS and EBS calculations
> and conversion to a PMD-specific format when the profile is used.

I think you should say also what happens to this pointer or values
in case of modification of the policy or destroy of the policy.

> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
>  .../traffic_metering_and_policing.rst         |  7 ++++
>  doc/guides/rel_notes/release_22_07.rst        |  1 +
>  lib/ethdev/rte_flow.h                         |  7 ++++
>  lib/ethdev/rte_mtr.c                          | 41 +++++++++++++++++++
>  lib/ethdev/rte_mtr.h                          | 40 ++++++++++++++++++
>  lib/ethdev/rte_mtr_driver.h                   | 19 +++++++++
>  lib/ethdev/version.map                        |  2 +
>  7 files changed, 117 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/traffic_metering_and_policing.rst
> b/doc/guides/prog_guide/traffic_metering_and_policing.rst
> index d1958a023d..cae35bccf6 100644
> --- a/doc/guides/prog_guide/traffic_metering_and_policing.rst
> +++ b/doc/guides/prog_guide/traffic_metering_and_policing.rst
> @@ -107,6 +107,13 @@ traffic meter and policing library.
>       to the list of meter actions (``struct rte_mtr_meter_policy_params::actions``)
>       specified per color as show in :numref:`figure_rte_mtr_chaining`.
> 
> +#. The ``rte_mtr_meter_profile_get()`` and ``rte_mtr_meter_policy_get()``
> +   API functions are available for getting the object pointers directly.
> +   These pointers allow quick access to profile/policy objects and are
> +   required by the ``RTE_FLOW_ACTION_TYPE_METER_MARK`` action.
> +   This action may omit the polciy definition to preovide a flexibility
> +   to match a color later with the ``RTE_FLOW_ITEM_TYPE_METER_COLOR`` item.
I think it is important to explain what happens if a profile is changed to destroyed.

> +
>  Protocol based input color selection
>  ------------------------------------
> 
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index 451ff8d460..6d030bead5 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -73,6 +73,7 @@ New Features
> 
>    * Added METER_COLOR item to match Color Marker set by a Meter.
>    * Added ability to set Color Marker via modify_field Flow API.
> +  * Added Meter API to get a pointer to profile/policy by their ID.
> 
>  * **Updated Intel iavf driver.**
> 
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 68af109554..9754f6630a 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3827,6 +3827,13 @@ struct rte_flow_action {
>   */
>  struct rte_flow;
> 
> +/**
> + * Opaque type for Meter profile object returned by MTR API.
> + *
> + * This handle can be used to create Meter actions instead of profile ID.
> + */
> +struct rte_flow_meter_profile;
> +
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this structure may change without prior notice
> diff --git a/lib/ethdev/rte_mtr.c b/lib/ethdev/rte_mtr.c
> index 441ea1dca9..90fd277040 100644
> --- a/lib/ethdev/rte_mtr.c
> +++ b/lib/ethdev/rte_mtr.c
> @@ -57,6 +57,25 @@ rte_mtr_ops_get(uint16_t port_id, struct rte_mtr_error *error)
>  	ops->func;					\
>  })
> 
> +#define RTE_MTR_HNDL_FUNC(port_id, func)		\
> +({							\
> +	const struct rte_mtr_ops *ops =			\
> +		rte_mtr_ops_get(port_id, error);	\
> +	if (ops == NULL)				\
> +		return NULL;				\
> +							\
> +	if (ops->func == NULL) {			\
> +		rte_mtr_error_set(error,		\
> +			ENOSYS,				\
> +			RTE_MTR_ERROR_TYPE_UNSPECIFIED,	\
> +			NULL,				\
> +			rte_strerror(ENOSYS));		\
> +		return NULL;				\
> +	}						\
> +							\
> +	ops->func;					\
> +})
> +
>  /* MTR capabilities get */
>  int
>  rte_mtr_capabilities_get(uint16_t port_id,
> @@ -91,6 +110,17 @@ rte_mtr_meter_profile_delete(uint16_t port_id,
>  		meter_profile_id, error);
>  }
> 
> +/** MTR meter profile get */
> +struct rte_flow_meter_profile *
> +rte_mtr_meter_profile_get(uint16_t port_id,
> +	uint32_t meter_profile_id,
> +	struct rte_mtr_error *error)
> +{
> +	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> +	return RTE_MTR_HNDL_FUNC(port_id, meter_profile_get)(dev,
> +		meter_profile_id, error);
> +}
> +
>  /* MTR meter policy validate */
>  int
>  rte_mtr_meter_policy_validate(uint16_t port_id,
> @@ -125,6 +155,17 @@ rte_mtr_meter_policy_delete(uint16_t port_id,
>  		policy_id, error);
>  }
> 
> +/** MTR meter policy get */
> +struct rte_flow_meter_policy *
> +rte_mtr_meter_policy_get(uint16_t port_id,
> +	uint32_t policy_id,
> +	struct rte_mtr_error *error)
> +{
> +	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> +	return RTE_MTR_HNDL_FUNC(port_id, meter_policy_get)(dev,
> +		policy_id, error);
> +}
> +
>  /** MTR object create */
>  int
>  rte_mtr_create(uint16_t port_id,
> diff --git a/lib/ethdev/rte_mtr.h b/lib/ethdev/rte_mtr.h
> index 008bc84f0d..58f0d26215 100644
> --- a/lib/ethdev/rte_mtr.h
> +++ b/lib/ethdev/rte_mtr.h
> @@ -623,6 +623,26 @@ rte_mtr_meter_profile_delete(uint16_t port_id,
>  	uint32_t meter_profile_id,
>  	struct rte_mtr_error *error);
> 
> +/**
> + * Meter profile object get
> + *
> + * Get meter profile object for a given meter profile ID.
> + *
> + * @param[in] port_id
> + *   The port identifier of the Ethernet device.
> + * @param[in] meter_profile_id
> + *   Meter profile ID. Needs to be the valid.
> + * @param[out] error
> + *   Error details. Filled in only on error, when not NULL.
> + * @return
> + *   A valid handle in case of success, NULL otherwise.
> + */
> +__rte_experimental
> +struct rte_flow_meter_profile *
> +rte_mtr_meter_profile_get(uint16_t port_id,
> +	uint32_t meter_profile_id,
> +	struct rte_mtr_error *error);
> +
>  /**
>   * Check whether a meter policy can be created on a given port.
>   *
> @@ -679,6 +699,26 @@ rte_mtr_meter_policy_add(uint16_t port_id,
>  	struct rte_mtr_meter_policy_params *policy,
>  	struct rte_mtr_error *error);
> 
> +/**
> + * Meter policy object get
> + *
> + * Get meter policy object for a given meter policy ID.
> + *
> + * @param[in] port_id
> + *   The port identifier of the Ethernet device.
> + * @param[in] policy_id
> + *   Meter policy ID. Needs to be the valid.
> + * @param[out] error
> + *   Error details. Filled in only on error, when not NULL.
> + * @return
> + *   A valid handle in case of success, NULL otherwise.
> + */
> +__rte_experimental
> +struct rte_flow_meter_policy *
> +rte_mtr_meter_policy_get(uint16_t port_id,
> +	uint32_t policy_id,
> +	struct rte_mtr_error *error);
> +
>  /**
>   * Define meter policy action list:
>   * GREEN - GREEN, YELLOW - YELLOW, RED - RED
> diff --git a/lib/ethdev/rte_mtr_driver.h b/lib/ethdev/rte_mtr_driver.h
> index f7dca9a54c..948a629b93 100644
> --- a/lib/ethdev/rte_mtr_driver.h
> +++ b/lib/ethdev/rte_mtr_driver.h
> @@ -41,6 +41,12 @@ typedef int (*rte_mtr_meter_profile_delete_t)(struct rte_eth_dev *dev,
>  	uint32_t meter_profile_id,
>  	struct rte_mtr_error *error);
> 
> +/** @internal MTR meter profile get. */
> +typedef struct rte_flow_meter_profile *
> +(*rte_mtr_meter_profile_get_t)(struct rte_eth_dev *dev,
> +	uint32_t meter_profile_id,
> +	struct rte_mtr_error *error);
> +
>  /** @internal MTR meter policy validate. */
>  typedef int (*rte_mtr_meter_policy_validate_t)(struct rte_eth_dev *dev,
>  	struct rte_mtr_meter_policy_params *policy,
> @@ -57,6 +63,13 @@ typedef int (*rte_mtr_meter_policy_delete_t)(struct rte_eth_dev *dev,
>  	uint32_t policy_id,
>  	struct rte_mtr_error *error);
> 
> +/** @internal MTR meter policy get. */
> +typedef struct rte_flow_meter_policy *
> +(*rte_mtr_meter_policy_get_t)(struct rte_eth_dev *dev,
> +	uint32_t policy_id,
> +	struct rte_mtr_error *error);
> +
> +
>  /** @internal MTR object create. */
>  typedef int (*rte_mtr_create_t)(struct rte_eth_dev *dev,
>  	uint32_t mtr_id,
> @@ -194,6 +207,12 @@ struct rte_mtr_ops {
> 
>  	/** MTR object meter policy update */
>  	rte_mtr_meter_policy_update_t meter_policy_update;
> +
> +	/** MTR meter profile get */
> +	rte_mtr_meter_profile_get_t meter_profile_get;
> +
> +	/** MTR meter policy get */
> +	rte_mtr_meter_policy_get_t meter_policy_get;
>  };
> 
>  /**
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index daca7851f2..2609f2e709 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>  	rte_mtr_color_in_protocol_priority_get;
>  	rte_mtr_color_in_protocol_set;
>  	rte_mtr_meter_vlan_table_update;
> +	rte_mtr_meter_profile_get;
> +	rte_mtr_meter_policy_get;
>  };
> 
>  INTERNAL {
> --
> 2.18.2

Best,
Ori
  

On Thursday, May 26, 2022 8:28 Ori Kam <orika@nvidia.com> wrote:
> I think you should say also what happens to this pointer or values
> in case of modification of the policy or destroy of the policy.

Will mention this in the commit message in v3.
Pointer is expected to be invalid and cannot be used once policy destruction.

> I think it is important to explain what happens if a profile is changed to
> destroyed.
I thought it is pretty straight-forward.
Pointer is not valid in case an object is gone.
If it is updated then you can continue to use it.