[v4] ethdev: mtr: support protocol based input color selection

  From: Jerin Jacob <jerinj@marvell.com>

Currently, meter object supports only DSCP based on input color table,
The patch enhance that to support VLAN based input color table,
color table based on inner field for the tunnel use case, and
support for fallback color per meter if packet based on a different field.

All of the above features are exposed through capability and added
additional capability to specify the implementation supports
more than one input color table per ethdev port.

Suggested-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
Signed-off-by: Jerin Jacob <jerinj@marvell.com>
---
v4..v3:

- Aligned with community meeting call which is documented in
https://patches.dpdk.org/project/dpdk/patch/20220301085824.1041009-1-skori@marvell.com/
as last message. With following exception, 
- Used RTE_MTR_COLOR_IN_*_DSCP instead of RTE_MTR_COLOR_IN_*_IP as
there is already dscp_table and rte_mtr_meter_dscp_table_update() API.
Changing above symbols break existing application for no good.
- Updated 22.07 release notes
- Remove testpmd changes from series to finalize the API spec first and
  then we can send testpmd changes.

v3..v2:

- Fix input color flags as a bitmask
- Add definitions for newly added API

v2..v1:
- Fix seperate typo

v1..RFC:

Address the review comments by Cristian at
https://patches.dpdk.org/project/dpdk/patch/20210820082401.3778736-1-jerinj@marvell.com/

- Moved to v22.07 release
- Updated rte_mtr_input_color_method to support all VLAN, DSCP, Inner
  cases
- Added input_color_method
- Removed union between vlan_table and dscp_table
- Kept VLAN instead of PCP as HW coloring based on DEI(1bit), PCP(3
  bits)

 .../traffic_metering_and_policing.rst         |  33 ++++
 doc/guides/rel_notes/release_22_07.rst        |  10 +
 lib/ethdev/rte_mtr.c                          |  23 +++
 lib/ethdev/rte_mtr.h                          | 186 +++++++++++++++++-
 lib/ethdev/rte_mtr_driver.h                   |  19 ++
 lib/ethdev/version.map                        |   4 +
 6 files changed, 265 insertions(+), 10 deletions(-)
  

jerinj@marvell.com writes:

> From: Jerin Jacob <jerinj@marvell.com>
>
> Currently, meter object supports only DSCP based on input color table,
> The patch enhance that to support VLAN based input color table,
> color table based on inner field for the tunnel use case, and
> support for fallback color per meter if packet based on a different field.
>
> All of the above features are exposed through capability and added
> additional capability to specify the implementation supports
> more than one input color table per ethdev port.
>
> Suggested-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> ---
> v4..v3:
>
> - Aligned with community meeting call which is documented in
> https://patches.dpdk.org/project/dpdk/patch/20220301085824.1041009-1-skori@marvell.com/
> as last message. With following exception, 
> - Used RTE_MTR_COLOR_IN_*_DSCP instead of RTE_MTR_COLOR_IN_*_IP as
> there is already dscp_table and rte_mtr_meter_dscp_table_update() API.
> Changing above symbols break existing application for no good.
> - Updated 22.07 release notes
> - Remove testpmd changes from series to finalize the API spec first and
>   then we can send testpmd changes.
>
> v3..v2:
>
> - Fix input color flags as a bitmask
> - Add definitions for newly added API
>
> v2..v1:
> - Fix seperate typo
>
> v1..RFC:
>
> Address the review comments by Cristian at
> https://patches.dpdk.org/project/dpdk/patch/20210820082401.3778736-1-jerinj@marvell.com/
>
> - Moved to v22.07 release
> - Updated rte_mtr_input_color_method to support all VLAN, DSCP, Inner
>   cases
> - Added input_color_method
> - Removed union between vlan_table and dscp_table
> - Kept VLAN instead of PCP as HW coloring based on DEI(1bit), PCP(3
>   bits)
>
>  .../traffic_metering_and_policing.rst         |  33 ++++
>  doc/guides/rel_notes/release_22_07.rst        |  10 +
>  lib/ethdev/rte_mtr.c                          |  23 +++
>  lib/ethdev/rte_mtr.h                          | 186 +++++++++++++++++-
>  lib/ethdev/rte_mtr_driver.h                   |  19 ++
>  lib/ethdev/version.map                        |   4 +
>  6 files changed, 265 insertions(+), 10 deletions(-)
>
> diff --git a/doc/guides/prog_guide/traffic_metering_and_policing.rst b/doc/guides/prog_guide/traffic_metering_and_policing.rst
> index ceb5a96488..75deabbaf1 100644
> --- a/doc/guides/prog_guide/traffic_metering_and_policing.rst
> +++ b/doc/guides/prog_guide/traffic_metering_and_policing.rst
> @@ -21,6 +21,7 @@ The main features are:
>  * Policer actions (per meter output color): recolor, drop
>  * Statistics (per policer output color)
>  * Chaining multiple meter objects
> +* Protocol based input color selection
>  
>  Configuration steps
>  -------------------
> @@ -105,3 +106,35 @@ traffic meter and policing library.
>     * Adding one (or multiple) actions of the type ``RTE_FLOW_ACTION_TYPE_METER``
>       to the list of meter actions (``struct rte_mtr_meter_policy_params::actions``)
>       specified per color as show in :numref:`figure_rte_mtr_chaining`.
> +
> +Protocol based input color selection
> +------------------------------------
> +
> +The API supports selecting the input color based on the packet content.
> +Following is the API usage model for the same.
> +
> +#. Probe the protocol based input color selection device capabilities using
> +   following parameter using ``rte_mtr_capabilities_get()`` API.
> +
> +   * ``struct rte_mtr_capabilities::input_color_proto_mask;``
> +   * ``struct rte_mtr_capabilities::separate_input_color_table_per_port``
> +
> +#. When creating the meter object using ``rte_mtr_create()``, configure
> +   relevant input color selection parameters such as
> +
> +   * Input color protocols with ``struct rte_mtr_params::input_color_proto_mask``
> +
> +   * If ``struct rte_mtr_params::input_color_proto_mask`` has multiple bits set then
> +     ``rte_mtr_color_in_protocol_priority_set()`` shall be used to set the priority,
> +     in the order, in which protocol to be used to find the input color.
> +
> +   * Fill the tables ``struct rte_mtr_params::dscp_table``,
> +     ``struct rte_mtr_params::vlan_table`` based on input color selected.
> +
> +   * Update the ``struct rte_mtr_params::default_input_color`` to determine
> +     the default input color in case the input packet does not match
> +     the input color method.
> +
> +   * If needed, update the input color table at runtime using
> +     ``rte_mtr_meter_vlan_table_update()`` and ``rte_mtr_meter_dscp_table_update()``
> +     APIs.
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index 42a5f2d990..746622f9b3 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -55,6 +55,13 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
>  
> +* **Added protocol based input color for meter.**
> +
> +  Added new APIs ``rte_mtr_color_in_protocol_priority_set()``,
> +  ``rte_mtr_meter_vlan_table_update()`` and updated ``struct rte_mtr_params``
> +  and ``struct rte_mtr_capabilities`` to support protocol based
> +  input color for meter.
> +
>  
>  Removed Items
>  -------------
> @@ -102,6 +109,9 @@ ABI Changes
>  
>  * No ABI change that would break compatibility with 21.11.
>  
> +* Experimental structures ``struct rte_mtr_params``
> +  and ``struct rte_mtr_capabilities`` updated to support
> +  protocol based input color for meter.
>  
>  Known Issues
>  ------------
> diff --git a/lib/ethdev/rte_mtr.c b/lib/ethdev/rte_mtr.c
> index e49fcf271c..a112fe65f5 100644
> --- a/lib/ethdev/rte_mtr.c
> +++ b/lib/ethdev/rte_mtr.c
> @@ -207,6 +207,29 @@ rte_mtr_meter_dscp_table_update(uint16_t port_id,
>  		mtr_id, dscp_table, error);
>  }
>  
> +/** MTR object meter VLAN table update */
> +int
> +rte_mtr_meter_vlan_table_update(uint16_t port_id, uint32_t mtr_id,
> +				enum rte_color *vlan_table,
> +				struct rte_mtr_error *error)
> +{
> +	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> +	return RTE_MTR_FUNC(port_id, meter_vlan_table_update)(dev,
> +		mtr_id, vlan_table, error);
> +}
> +
> +/** Set the priority for input color protocol on MTR object */
> +int
> +rte_mtr_color_in_protocol_priority_set(uint16_t port_id, uint32_t mtr_id,
> +				       enum rte_mtr_color_in_protocol proto,
> +				       uint32_t priority,
> +				       struct rte_mtr_error *error)
> +{
> +	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> +	return RTE_MTR_FUNC(port_id, in_proto_prio_set)(dev,
> +		mtr_id, proto, priority, error);
> +}
> +
>  /** MTR object enabled stats update */
>  int
>  rte_mtr_stats_update(uint16_t port_id,
> diff --git a/lib/ethdev/rte_mtr.h b/lib/ethdev/rte_mtr.h
> index 40df0888c8..76ffbcf724 100644
> --- a/lib/ethdev/rte_mtr.h
> +++ b/lib/ethdev/rte_mtr.h
> @@ -213,6 +213,52 @@ struct rte_mtr_meter_policy_params {
>  	const struct rte_flow_action *actions[RTE_COLORS];
>  };
>  
> +/**
> + * Input color protocol method
> + */
> +enum rte_mtr_color_in_protocol {
> +	/**
> +	 * If the input packet has at least one VLAN label, its input color is
> +	 * detected by the outermost VLAN DEI(1bit), PCP(3 bits)
> +	 * indexing into the struct rte_mtr_params::vlan_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *
> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::vlan_table
> +	 */
> +	RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN = RTE_BIT64(0),
> +	/**
> +	 * If the input packet has at least one VLAN label, its input color is
> +	 * detected by the innermost VLAN DEI(1bit), PCP(3 bits)
> +	 * indexing into the struct rte_mtr_params::vlan_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *
> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::vlan_table
> +	 */
> +	RTE_MTR_COLOR_IN_PROTO_INNER_VLAN = RTE_BIT64(1),
> +	/**
> +	 * If the input packet is IPv4 or IPv6, its input color is detected by
> +	 * the outermost DSCP field indexing into the
> +	 * struct rte_mtr_params::dscp_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *
> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::dscp_table
> +	 */
> +	RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP = RTE_BIT64(2),
> +	/**
> +	 * If the input packet is IPv4 or IPv6, its input color is detected by
> +	 * the innermost DSCP field indexing into the
> +	 * struct rte_mtr_params::dscp_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *
> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::dscp_table
> +	 */
> +	RTE_MTR_COLOR_IN_PROTO_INNER_DSCP = RTE_BIT64(3),
> +};
> +
>  /**
>   * Parameters for each traffic metering & policing object
>   *
> @@ -233,20 +279,58 @@ struct rte_mtr_params {
>  	 */
>  	int use_prev_mtr_color;
>  
> -	/** Meter input color. When non-NULL: it points to a pre-allocated and
> +	/** Meter input color based on IP DSCP protocol field.
> +	 *
> +	 * Valid when *input_color_proto_mask* set to any of the following
> +	 * RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP,
> +	 * RTE_MTR_COLOR_IN_PROTO_INNER_DSCP
> +	 *
> +	 * When non-NULL: it points to a pre-allocated and
>  	 * pre-populated table with exactly 64 elements providing the input
>  	 * color for each value of the IPv4/IPv6 Differentiated Services Code
> -	 * Point (DSCP) input packet field. When NULL: it is equivalent to
> -	 * setting this parameter to an all-green populated table (i.e. table
> -	 * with all the 64 elements set to green color). The color blind mode
> -	 * is configured by setting *use_prev_mtr_color* to 0 and *dscp_table*
> -	 * to either NULL or to an all-green populated table. When
> -	 * *use_prev_mtr_color* is non-zero value or when *dscp_table* contains
> -	 * at least one yellow or red color element, then the color aware mode
> -	 * is configured.
> +	 * Point (DSCP) input packet field.
> +	 *
> +	 * When NULL: it is equivalent to setting this parameter to an all-green
> +	 * populated table (i.e. table with all the 64 elements set to green
> +	 * color). The color blind mode is configured by setting
> +	 * *use_prev_mtr_color* to 0 and *dscp_table* to either NULL or to an
> +	 * all-green populated table.
> +	 *
> +	 * When *use_prev_mtr_color* is non-zero value or when *dscp_table*
> +	 * contains at least one yellow or red color element, then the color
> +	 * aware mode is configured.
> +	 *
> +	 * @see enum rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP
> +	 * @see enum rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_INNER_DSCP
> +	 * @see struct rte_mtr_params::input_color_proto_mask
>  	 */
>  	enum rte_color *dscp_table;
> -
> +	/** Meter input color based on VLAN DEI(1bit), PCP(3 bits) protocol
> +	 * fields.
> +	 *
> +	 * Valid when *input_color_proto_mask* set to any of the following
> +	 * RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN,
> +	 * RTE_MTR_COLOR_IN_PROTO_INNER_VLAN
> +	 *
> +	 * When non-NULL: it points to a pre-allocated and pre-populated
> +	 * table with exactly 16 elements providing the input color for
> +	 * each value of the DEI(1bit), PCP(3 bits) input packet field.
> +	 *
> +	 * When NULL: it is equivalent to setting this parameter to an
> +	 * all-green populated table (i.e. table with
> +	 * all the 16 elements set to green color). The color blind mode
> +	 * is configured by setting *use_prev_mtr_color* to 0 and
> +	 * *vlan_table* to either NULL or to an all-green populated table.
> +	 *
> +	 * When *use_prev_mtr_color* is non-zero value
> +	 * or when *vlan_table* contains at least one yellow or
> +	 * red color element, then the color aware mode is configured.
> +	 *
> +	 * @see enum rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN
> +	 * @see enum rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_INNER_VLAN
> +	 * @see struct rte_mtr_params::input_color_proto_mask
> +	 */
> +	enum rte_color *vlan_table;
>  	/** Non-zero to enable the meter, zero to disable the meter at the time
>  	 * of MTR object creation. Ignored when the meter profile indicated by
>  	 * *meter_profile_id* is set to NONE.
> @@ -261,6 +345,25 @@ struct rte_mtr_params {
>  
>  	/** Meter policy ID. @see rte_mtr_meter_policy_add() */
>  	uint32_t meter_policy_id;
> +
> +	/** Set of input color protocols to be enabled.
> +	 *
> +	 * Set value to zero to configure as color blind mode.
> +	 *
> +	 * When multiple bits set then rte_mtr_color_in_protocol_priority_set()
> +	 * shall be used to set the priority, in the order, in which protocol
> +	 * to be used to find the inpput color.
> +	 *
> +	 * @see enum rte_mtr_color_in_protocol
> +	 * @see rte_mtr_color_in_protocol_priority_set()
> +	 */
> +	uint64_t input_color_proto_mask;
> +
> +	/** Input color to be set for the input packet when none of the
> +	 * enabled input color methods is applicable to the input packet.
> +	 * Ignored when this when *input_color_proto_mask* set to zero.
> +	 */
> +	enum rte_color default_input_color;
>  };
>  
>  /**
> @@ -417,6 +520,16 @@ struct rte_mtr_capabilities {
>  	 * @see enum rte_mtr_stats_type
>  	 */
>  	uint64_t stats_mask;
> +
> +	/** Set of supported input color protocol.
> +	 * @see enum rte_mtr_color_in_protocol
> +	 */
> +	uint64_t input_color_proto_mask;
> +
> +	/** When non-zero, it indicates that driver supports separate
> +	 * input color table for given ethdev port.
> +	 */
> +	int separate_input_color_table_per_port;
>  };
>  
>  /**
> @@ -832,6 +945,59 @@ rte_mtr_meter_dscp_table_update(uint16_t port_id,
>  	enum rte_color *dscp_table,
>  	struct rte_mtr_error *error);
>  
> +/**
> + * MTR object VLAN table update
> + *
> + * @param[in] port_id
> + *   The port identifier of the Ethernet device.
> + * @param[in] mtr_id
> + *   MTR object ID. Needs to be valid.
> + * @param[in] vlan_table
> + *   When non-NULL: it points to a pre-allocated and pre-populated table with
> + *   exactly 16 elements providing the input color for each value of the
> + *   each value of the DEI(1bit), PCP(3 bits) input packet field.
> + *   When NULL: it is equivalent to setting this parameter to an "all-green"
> + *   populated table (i.e. table with all the 16 elements set to green color).
> + * @param[out] error
> + *   Error details. Filled in only on error, when not NULL.
> + * @return
> + *   0 on success, non-zero error code otherwise.
> + */
> +__rte_experimental
> +int
> +rte_mtr_meter_vlan_table_update(uint16_t port_id, uint32_t mtr_id,
> +				enum rte_color *vlan_table,
> +				struct rte_mtr_error *error);
> +/**
> + * Set the priority for input color protocol
> + *
> + * When multiple bits set in struct rte_mtr_params::input_color_proto_mask
> + * then this API shall be used to set the priority, in the order, in
> + * which protocol to be used to find the input color.
> + *
> + * @param[in] port_id
> + *   The port identifier of the Ethernet device.
> + * @param[in] mtr_id
> + *   MTR object ID. Needs to be valid.
> + * @param[in] proto
> + *   Input color protocol to apply priority.
> + *   MTR object's *input_color_proto_mask* should be configured
> + *   with proto value.
> + * @param[in] priority
> + *   Input color protocol priority. Value zero indicates
> + *   the highest priority.
> + * @param[out] error
> + *   Error details. Filled in only on error, when not NULL.
> + * @return
> + *   0 on success, non-zero error code otherwise.
> + *
> + * @see rte_mtr_params::input_color_proto_mask
> + */
> +__rte_experimental
> +int
> +rte_mtr_color_in_protocol_priority_set(uint16_t port_id, uint32_t mtr_id,
> +	enum rte_mtr_color_in_protocol proto, uint32_t priority,
> +	struct rte_mtr_error *error);
>  /**
>   * MTR object enabled statistics counters update
>   *
> diff --git a/lib/ethdev/rte_mtr_driver.h b/lib/ethdev/rte_mtr_driver.h
> index ee8c6ef7ad..827952adc6 100644
> --- a/lib/ethdev/rte_mtr_driver.h
> +++ b/lib/ethdev/rte_mtr_driver.h
> @@ -97,6 +97,19 @@ typedef int (*rte_mtr_meter_dscp_table_update_t)(struct rte_eth_dev *dev,
>  	enum rte_color *dscp_table,
>  	struct rte_mtr_error *error);
>  
> +/** @internal mtr object meter vlan table update. */
> +typedef int (*rte_mtr_meter_vlan_table_update_t)(struct rte_eth_dev *dev,
> +	uint32_t mtr_id,
> +	enum rte_color *vlan_table,
> +	struct rte_mtr_error *error);
> +
> +/** @internal Set the priority for input color protocol on MTR object. */
> +typedef int (*rte_mtr_meter_color_in_proto_prio_set_t)(struct rte_eth_dev *dev,
> +	uint32_t mtr_id,
> +	enum rte_mtr_color_in_protocol proto,
> +	uint32_t priority,
> +	struct rte_mtr_error *error);
> +
>  /** @internal MTR object enabled stats update. */
>  typedef int (*rte_mtr_stats_update_t)(struct rte_eth_dev *dev,
>  	uint32_t mtr_id,
> @@ -139,6 +152,12 @@ struct rte_mtr_ops {
>  	/** MTR object meter DSCP table update */
>  	rte_mtr_meter_dscp_table_update_t meter_dscp_table_update;
>  
> +	/** MTR object meter VLAN table update */
> +	rte_mtr_meter_vlan_table_update_t meter_vlan_table_update;
> +
> +	/** Set the priority for input color protocol on MTR object. */
> +	rte_mtr_meter_color_in_proto_prio_set_t in_proto_prio_set;
> +
>  	/** MTR object enabled stats update */
>  	rte_mtr_stats_update_t stats_update;
>  
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 20391ab29e..cccbf6dee5 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -279,6 +279,10 @@ EXPERIMENTAL {
>  	rte_flow_async_action_handle_create;
>  	rte_flow_async_action_handle_destroy;
>  	rte_flow_async_action_handle_update;
> +
> +	# added in 22.07

Symbols are not in alphabetical order.  

> +	rte_mtr_meter_vlan_table_update;
> +	rte_mtr_color_in_protocol_priority_set;
>  };
>  
>  INTERNAL {
  

Hi Jerin,

Thank you for implementing according to our agreement, I am happy to see that we are converging.

Here are some comments below:

<snip>

> diff --git a/lib/ethdev/rte_mtr.h b/lib/ethdev/rte_mtr.h
> index 40df0888c8..76ffbcf724 100644
> --- a/lib/ethdev/rte_mtr.h
> +++ b/lib/ethdev/rte_mtr.h
> @@ -213,6 +213,52 @@ struct rte_mtr_meter_policy_params {
>  	const struct rte_flow_action *actions[RTE_COLORS];
>  };
> 
> +/**
> + * Input color protocol method

I suggest adding some more explanations here:
More than one method can be enabled for a given meter. Even if enabled, a method might not be applicable to each input packet, in case the associated protocol header is not present in the packet. The highest priority method that is both enabled for the meter and also applicable for the current input packet wins; if none is both enabled and applicable, the default input color is used. @see function rte_mtr_color_in_protocol_priority_set()

> + */
> +enum rte_mtr_color_in_protocol {
> +	/**
> +	 * If the input packet has at least one VLAN label, its input color is
> +	 * detected by the outermost VLAN DEI(1bit), PCP(3 bits)
> +	 * indexing into the struct rte_mtr_params::vlan_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *

The statement "Otherwise, the *default_input_color* is applied" is incorrect IMO and should be removed, as multiple methods might be enabled and also applicable to a specific input packet, in which case the highest priority method wins, as opposed to the default input color.

I suggest a simplification "Enable the detection of the packet input color based on the outermost VLAN header fields DEI (1 bit) and PCP (3 bits). These fields are used as index into the VLAN table"

> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::vlan_table
> +	 */
> +	RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN = RTE_BIT64(0),
> +	/**
> +	 * If the input packet has at least one VLAN label, its input color is
> +	 * detected by the innermost VLAN DEI(1bit), PCP(3 bits)
> +	 * indexing into the struct rte_mtr_params::vlan_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *
> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::vlan_table
> +	 */

Same simplification suggested here.

> +	RTE_MTR_COLOR_IN_PROTO_INNER_VLAN = RTE_BIT64(1),
> +	/**
> +	 * If the input packet is IPv4 or IPv6, its input color is detected by
> +	 * the outermost DSCP field indexing into the
> +	 * struct rte_mtr_params::dscp_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *
> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::dscp_table
> +	 */

Same simplification suggested here.

> +	RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP = RTE_BIT64(2),

I am OK to keep DSCP for the name of the table instead of renaming the table, as you suggested, but this method name should reflect the protocol, not the field: RTE_MTR_COLOR_IN_PROTO_OUTER_IP.

> +	/**
> +	 * If the input packet is IPv4 or IPv6, its input color is detected by
> +	 * the innermost DSCP field indexing into the
> +	 * struct rte_mtr_params::dscp_table.
> +	 * Otherwise, the *default_input_color* is applied.
> +	 *
> +	 * @see struct rte_mtr_params::default_input_color
> +	 * @see struct rte_mtr_params::dscp_table
> +	 */

Same simplification suggested here.

> +	RTE_MTR_COLOR_IN_PROTO_INNER_DSCP = RTE_BIT64(3),

I am OK to keep DSCP for the name of the table instead of renaming the table, as you suggested, but this method name should reflect the protocol, not the field: RTE_MTR_COLOR_IN_PROTO_INNER_IP.

> +
>  /**
>   * Parameters for each traffic metering & policing object
>   *
> @@ -233,20 +279,58 @@ struct rte_mtr_params {
>  	 */
>  	int use_prev_mtr_color;
> 
> -	/** Meter input color. When non-NULL: it points to a pre-allocated and
> +	/** Meter input color based on IP DSCP protocol field.
> +	 *
> +	 * Valid when *input_color_proto_mask* set to any of the following
> +	 * RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP,
> +	 * RTE_MTR_COLOR_IN_PROTO_INNER_DSCP
> +	 *
> +	 * When non-NULL: it points to a pre-allocated and
>  	 * pre-populated table with exactly 64 elements providing the input
>  	 * color for each value of the IPv4/IPv6 Differentiated Services Code
> -	 * Point (DSCP) input packet field. When NULL: it is equivalent to
> -	 * setting this parameter to an all-green populated table (i.e. table
> -	 * with all the 64 elements set to green color). The color blind mode
> -	 * is configured by setting *use_prev_mtr_color* to 0 and *dscp_table*
> -	 * to either NULL or to an all-green populated table. When
> -	 * *use_prev_mtr_color* is non-zero value or when *dscp_table*
> contains
> -	 * at least one yellow or red color element, then the color aware mode
> -	 * is configured.
> +	 * Point (DSCP) input packet field.
> +	 *
> +	 * When NULL: it is equivalent to setting this parameter to an all-green
> +	 * populated table (i.e. table with all the 64 elements set to green
> +	 * color). The color blind mode is configured by setting
> +	 * *use_prev_mtr_color* to 0 and *dscp_table* to either NULL or to an
> +	 * all-green populated table.
> +	 *
> +	 * When *use_prev_mtr_color* is non-zero value or when
> *dscp_table*
> +	 * contains at least one yellow or red color element, then the color
> +	 * aware mode is configured.
> +	 *
> +	 * @see enum
> rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP
> +	 * @see enum
> rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_INNER_DSCP
> +	 * @see struct rte_mtr_params::input_color_proto_mask
>  	 */
>  	enum rte_color *dscp_table;
> -
> +	/** Meter input color based on VLAN DEI(1bit), PCP(3 bits) protocol
> +	 * fields.
> +	 *
> +	 * Valid when *input_color_proto_mask* set to any of the following
> +	 * RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN,
> +	 * RTE_MTR_COLOR_IN_PROTO_INNER_VLAN
> +	 *
> +	 * When non-NULL: it points to a pre-allocated and pre-populated
> +	 * table with exactly 16 elements providing the input color for
> +	 * each value of the DEI(1bit), PCP(3 bits) input packet field.
> +	 *
> +	 * When NULL: it is equivalent to setting this parameter to an
> +	 * all-green populated table (i.e. table with
> +	 * all the 16 elements set to green color). The color blind mode
> +	 * is configured by setting *use_prev_mtr_color* to 0 and
> +	 * *vlan_table* to either NULL or to an all-green populated table.
> +	 *
> +	 * When *use_prev_mtr_color* is non-zero value
> +	 * or when *vlan_table* contains at least one yellow or
> +	 * red color element, then the color aware mode is configured.
> +	 *
> +	 * @see enum
> rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN
> +	 * @see enum
> rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_INNER_VLAN
> +	 * @see struct rte_mtr_params::input_color_proto_mask
> +	 */
> +	enum rte_color *vlan_table;
>  	/** Non-zero to enable the meter, zero to disable the meter at the
> time
>  	 * of MTR object creation. Ignored when the meter profile indicated by
>  	 * *meter_profile_id* is set to NONE.
> @@ -261,6 +345,25 @@ struct rte_mtr_params {
> 
>  	/** Meter policy ID. @see rte_mtr_meter_policy_add() */
>  	uint32_t meter_policy_id;
> +
> +	/** Set of input color protocols to be enabled.
> +	 *
> +	 * Set value to zero to configure as color blind mode.
> +	 *
> +	 * When multiple bits set then rte_mtr_color_in_protocol_priority_set()
> +	 * shall be used to set the priority, in the order, in which protocol
> +	 * to be used to find the inpput color.
> +	 *
> +	 * @see enum rte_mtr_color_in_protocol
> +	 * @see rte_mtr_color_in_protocol_priority_set()
> +	 */
> +	uint64_t input_color_proto_mask;

We should not have this as an input parameter at all, please remove this field. This mask is implicitly created by the user by calling the rte_mtr_color_in_protocol_priority_set() API function. If this function is called for a given method, then that method is enabled with the given priority; when this function is not called for a given method, then that method is disabled.

> +
> +	/** Input color to be set for the input packet when none of the
> +	 * enabled input color methods is applicable to the input packet.
> +	 * Ignored when this when *input_color_proto_mask* set to zero.
> +	 */
> +	enum rte_color default_input_color;
>  };
> 
>  /**
> @@ -417,6 +520,16 @@ struct rte_mtr_capabilities {
>  	 * @see enum rte_mtr_stats_type
>  	 */
>  	uint64_t stats_mask;
> +
> +	/** Set of supported input color protocol.
> +	 * @see enum rte_mtr_color_in_protocol
> +	 */
> +	uint64_t input_color_proto_mask;

Agree.

> +
> +	/** When non-zero, it indicates that driver supports separate
> +	 * input color table for given ethdev port.
> +	 */
> +	int separate_input_color_table_per_port;

The input color tables are actually configured per meter object, do we also need a "separate_input_color_table_per_meter" capability flag?

>  };
> 
>  /**
> @@ -832,6 +945,59 @@ rte_mtr_meter_dscp_table_update(uint16_t port_id,
>  	enum rte_color *dscp_table,
>  	struct rte_mtr_error *error);
> 
> +/**
> + * MTR object VLAN table update
> + *
> + * @param[in] port_id
> + *   The port identifier of the Ethernet device.
> + * @param[in] mtr_id
> + *   MTR object ID. Needs to be valid.
> + * @param[in] vlan_table
> + *   When non-NULL: it points to a pre-allocated and pre-populated table with
> + *   exactly 16 elements providing the input color for each value of the
> + *   each value of the DEI(1bit), PCP(3 bits) input packet field.
> + *   When NULL: it is equivalent to setting this parameter to an "all-green"
> + *   populated table (i.e. table with all the 16 elements set to green color).
> + * @param[out] error
> + *   Error details. Filled in only on error, when not NULL.
> + * @return
> + *   0 on success, non-zero error code otherwise.
> + */
> +__rte_experimental
> +int
> +rte_mtr_meter_vlan_table_update(uint16_t port_id, uint32_t mtr_id,
> +				enum rte_color *vlan_table,
> +				struct rte_mtr_error *error);
> +/**
> + * Set the priority for input color protocol
> + *
> + * When multiple bits set in struct rte_mtr_params::input_color_proto_mask
> + * then this API shall be used to set the priority, in the order, in
> + * which protocol to be used to find the input color.

As stated above, we should remove struct rte_mtr_params::input_color_proto_mask, as it is build implicitly by calling this function.

I suggest reiterating the explanation from above:
More than one method can be enabled for a given meter. Even if enabled, a method might not be applicable to each input packet, in case the associated protocol header is not present in the packet. The highest priority method that is both enabled for the meter and also applicable for the current input packet wins; if none is both enabled and applicable, the default input color is used. @see function rte_mtr_color_in_protocol_priority_set()

> + *
> + * @param[in] port_id
> + *   The port identifier of the Ethernet device.
> + * @param[in] mtr_id
> + *   MTR object ID. Needs to be valid.
> + * @param[in] proto
> + *   Input color protocol to apply priority.
> + *   MTR object's *input_color_proto_mask* should be configured
> + *   with proto value.
> + * @param[in] priority
> + *   Input color protocol priority. Value zero indicates
> + *   the highest priority.

Agree.

> + * @param[out] error
> + *   Error details. Filled in only on error, when not NULL.
> + * @return
> + *   0 on success, non-zero error code otherwise.
> + *
> + * @see rte_mtr_params::input_color_proto_mask
> + */
> +__rte_experimental
> +int
> +rte_mtr_color_in_protocol_priority_set(uint16_t port_id, uint32_t mtr_id,
> +	enum rte_mtr_color_in_protocol proto, uint32_t priority,
> +	struct rte_mtr_error *error);
>  /**
>   * MTR object enabled statistics counters update
>   *

<snip>

Regards,
Cristian
  

On Tue, Apr 26, 2022 at 3:49 PM Ray Kinsella <mdr@ashroe.eu> wrote:
>
>
> jerinj@marvell.com writes:
>
> > From: Jerin Jacob <jerinj@marvell.com>
> >
> > Currently, meter object supports only DSCP based on input color table,
> > The patch enhance that to support VLAN based input color table,
> > color table based on inner field for the tunnel use case, and
> > support for fallback color per meter if packet based on a different field.
> >
> > All of the above features are exposed through capability and added
> > additional capability to specify the implementation supports
> > more than one input color table per ethdev port.
> >
> > Suggested-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > ---
> > v4..v3:
> >
> > - Aligned with community meeting call which is documented in
> > https://patches.dpdk.org/project/dpdk/patch/20220301085824.1041009-1-skori@marvell.com/
> > as last message. With following exception,
> > - Used RTE_MTR_COLOR_IN_*_DSCP instead of RTE_MTR_COLOR_IN_*_IP as
> > there is already dscp_table and rte_mtr_meter_dscp_table_update() API.
> > Changing above symbols break existing application for no good.
> > - Updated 22.07 release notes
> > - Remove testpmd changes from series to finalize the API spec first and
> >   then we can send testpmd changes.
> >
> > v3..v2:
> >
> > - Fix input color flags as a bitmask
> > - Add definitions for newly added API
> >
> > v2..v1:
> > - Fix seperate typo
> >
> > v1..RFC:
> >
> > Address the review comments by Cristian at
> > https://patches.dpdk.org/project/dpdk/patch/20210820082401.3778736-1-jerinj@marvell.com/

> >
> > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> > index 20391ab29e..cccbf6dee5 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -279,6 +279,10 @@ EXPERIMENTAL {
> >       rte_flow_async_action_handle_create;
> >       rte_flow_async_action_handle_destroy;
> >       rte_flow_async_action_handle_update;
> > +
> > +     # added in 22.07
>
> Symbols are not in alphabetical order.

Thanks. Will fix it in  v5

>
> > +     rte_mtr_meter_vlan_table_update;
> > +     rte_mtr_color_in_protocol_priority_set;
> >  };
> >
> >  INTERNAL {
>
>
> --
> Regards, Ray K
  

On Tue, Apr 26, 2022 at 5:38 PM Dumitrescu, Cristian
<cristian.dumitrescu@intel.com> wrote:
>
> Hi Jerin,

Hi Cristian

>
> Thank you for implementing according to our agreement, I am happy to see that we are converging.

:-)

>
> Here are some comments below:
>
> <snip>
>
> > diff --git a/lib/ethdev/rte_mtr.h b/lib/ethdev/rte_mtr.h
> > index 40df0888c8..76ffbcf724 100644
> > --- a/lib/ethdev/rte_mtr.h
> > +++ b/lib/ethdev/rte_mtr.h
> > @@ -213,6 +213,52 @@ struct rte_mtr_meter_policy_params {
> >       const struct rte_flow_action *actions[RTE_COLORS];
> >  };
> >
> > +/**
> > + * Input color protocol method
>
> I suggest adding some more explanations here:
> More than one method can be enabled for a given meter. Even if enabled, a method might not be applicable to each input packet, in case the associated protocol header is not present in the packet. The highest priority method that is both enabled for the meter and also applicable for the current input packet wins; if none is both enabled and applicable, the default input color is used. @see function rte_mtr_color_in_protocol_priority_set()

OK.

>
> > + */
> > +enum rte_mtr_color_in_protocol {
> > +     /**
> > +      * If the input packet has at least one VLAN label, its input color is
> > +      * detected by the outermost VLAN DEI(1bit), PCP(3 bits)
> > +      * indexing into the struct rte_mtr_params::vlan_table.
> > +      * Otherwise, the *default_input_color* is applied.
> > +      *
>
> The statement "Otherwise, the *default_input_color* is applied" is incorrect IMO and should be removed, as multiple methods might be enabled and also applicable to a specific input packet, in which case the highest priority method wins, as opposed to the default input color.
>
> I suggest a simplification "Enable the detection of the packet input color based on the outermost VLAN header fields DEI (1 bit) and PCP (3 bits). These fields are used as index into the VLAN table"

OK

>
> > +      * @see struct rte_mtr_params::default_input_color
> > +      * @see struct rte_mtr_params::vlan_table
> > +      */
> > +     RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN = RTE_BIT64(0),
> > +     /**
> > +      * If the input packet has at least one VLAN label, its input color is
> > +      * detected by the innermost VLAN DEI(1bit), PCP(3 bits)
> > +      * indexing into the struct rte_mtr_params::vlan_table.
> > +      * Otherwise, the *default_input_color* is applied.
> > +      *
> > +      * @see struct rte_mtr_params::default_input_color
> > +      * @see struct rte_mtr_params::vlan_table
> > +      */
>
> Same simplification suggested here.

OK

>
> > +     RTE_MTR_COLOR_IN_PROTO_INNER_VLAN = RTE_BIT64(1),
> > +     /**
> > +      * If the input packet is IPv4 or IPv6, its input color is detected by
> > +      * the outermost DSCP field indexing into the
> > +      * struct rte_mtr_params::dscp_table.
> > +      * Otherwise, the *default_input_color* is applied.
> > +      *
> > +      * @see struct rte_mtr_params::default_input_color
> > +      * @see struct rte_mtr_params::dscp_table
> > +      */
>
> Same simplification suggested here.

OK

>
> > +     RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP = RTE_BIT64(2),
>
> I am OK to keep DSCP for the name of the table instead of renaming the table, as you suggested, but this method name should reflect the protocol, not the field: RTE_MTR_COLOR_IN_PROTO_OUTER_IP.

OK

>
> > +     /**
> > +      * If the input packet is IPv4 or IPv6, its input color is detected by
> > +      * the innermost DSCP field indexing into the
> > +      * struct rte_mtr_params::dscp_table.
> > +      * Otherwise, the *default_input_color* is applied.
> > +      *
> > +      * @see struct rte_mtr_params::default_input_color
> > +      * @see struct rte_mtr_params::dscp_table
> > +      */
>
> Same simplification suggested here.

OK

>
> > +     RTE_MTR_COLOR_IN_PROTO_INNER_DSCP = RTE_BIT64(3),
>
> I am OK to keep DSCP for the name of the table instead of renaming the table, as you suggested, but this method name should reflect the protocol, not the field: RTE_MTR_COLOR_IN_PROTO_INNER_IP.

OK

>
> > +
> >  /**
> >   * Parameters for each traffic metering & policing object
> >   *
> > @@ -233,20 +279,58 @@ struct rte_mtr_params {
> >        */
> >       int use_prev_mtr_color;
> >
> > -     /** Meter input color. When non-NULL: it points to a pre-allocated and
> > +     /** Meter input color based on IP DSCP protocol field.
> > +      *
> > +      * Valid when *input_color_proto_mask* set to any of the following
> > +      * RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP,
> > +      * RTE_MTR_COLOR_IN_PROTO_INNER_DSCP
> > +      *
> > +      * When non-NULL: it points to a pre-allocated and
> >        * pre-populated table with exactly 64 elements providing the input
> >        * color for each value of the IPv4/IPv6 Differentiated Services Code
> > -      * Point (DSCP) input packet field. When NULL: it is equivalent to
> > -      * setting this parameter to an all-green populated table (i.e. table
> > -      * with all the 64 elements set to green color). The color blind mode
> > -      * is configured by setting *use_prev_mtr_color* to 0 and *dscp_table*
> > -      * to either NULL or to an all-green populated table. When
> > -      * *use_prev_mtr_color* is non-zero value or when *dscp_table*
> > contains
> > -      * at least one yellow or red color element, then the color aware mode
> > -      * is configured.
> > +      * Point (DSCP) input packet field.
> > +      *
> > +      * When NULL: it is equivalent to setting this parameter to an all-green
> > +      * populated table (i.e. table with all the 64 elements set to green
> > +      * color). The color blind mode is configured by setting
> > +      * *use_prev_mtr_color* to 0 and *dscp_table* to either NULL or to an
> > +      * all-green populated table.
> > +      *
> > +      * When *use_prev_mtr_color* is non-zero value or when
> > *dscp_table*
> > +      * contains at least one yellow or red color element, then the color
> > +      * aware mode is configured.
> > +      *
> > +      * @see enum
> > rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_OUTER_DSCP
> > +      * @see enum
> > rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_INNER_DSCP
> > +      * @see struct rte_mtr_params::input_color_proto_mask
> >        */
> >       enum rte_color *dscp_table;
> > -
> > +     /** Meter input color based on VLAN DEI(1bit), PCP(3 bits) protocol
> > +      * fields.
> > +      *
> > +      * Valid when *input_color_proto_mask* set to any of the following
> > +      * RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN,
> > +      * RTE_MTR_COLOR_IN_PROTO_INNER_VLAN
> > +      *
> > +      * When non-NULL: it points to a pre-allocated and pre-populated
> > +      * table with exactly 16 elements providing the input color for
> > +      * each value of the DEI(1bit), PCP(3 bits) input packet field.
> > +      *
> > +      * When NULL: it is equivalent to setting this parameter to an
> > +      * all-green populated table (i.e. table with
> > +      * all the 16 elements set to green color). The color blind mode
> > +      * is configured by setting *use_prev_mtr_color* to 0 and
> > +      * *vlan_table* to either NULL or to an all-green populated table.
> > +      *
> > +      * When *use_prev_mtr_color* is non-zero value
> > +      * or when *vlan_table* contains at least one yellow or
> > +      * red color element, then the color aware mode is configured.
> > +      *
> > +      * @see enum
> > rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN
> > +      * @see enum
> > rte_mtr_color_in_protocol::RTE_MTR_COLOR_IN_PROTO_INNER_VLAN
> > +      * @see struct rte_mtr_params::input_color_proto_mask
> > +      */
> > +     enum rte_color *vlan_table;
> >       /** Non-zero to enable the meter, zero to disable the meter at the
> > time
> >        * of MTR object creation. Ignored when the meter profile indicated by
> >        * *meter_profile_id* is set to NONE.
> > @@ -261,6 +345,25 @@ struct rte_mtr_params {
> >
> >       /** Meter policy ID. @see rte_mtr_meter_policy_add() */
> >       uint32_t meter_policy_id;
> > +
> > +     /** Set of input color protocols to be enabled.
> > +      *
> > +      * Set value to zero to configure as color blind mode.
> > +      *
> > +      * When multiple bits set then rte_mtr_color_in_protocol_priority_set()
> > +      * shall be used to set the priority, in the order, in which protocol
> > +      * to be used to find the inpput color.
> > +      *
> > +      * @see enum rte_mtr_color_in_protocol
> > +      * @see rte_mtr_color_in_protocol_priority_set()
> > +      */
> > +     uint64_t input_color_proto_mask;
>
> We should not have this as an input parameter at all, please remove this field. This mask is implicitly created by the user by calling the rte_mtr_color_in_protocol_priority_set() API function. If this function is called for a given method, then that method is enabled with the given priority; when this function is not called for a given method, then that method is disabled.

OK. Since we had rte_mtr_color_in_protocol_priority_set(), I was
thinking it is only setting "priority".

I will change to rte_mtr_color_in_protocol_set() and add API to _get() as well.


>
> > +
> > +     /** Input color to be set for the input packet when none of the
> > +      * enabled input color methods is applicable to the input packet.
> > +      * Ignored when this when *input_color_proto_mask* set to zero.
> > +      */
> > +     enum rte_color default_input_color;
> >  };
> >
> >  /**
> > @@ -417,6 +520,16 @@ struct rte_mtr_capabilities {
> >        * @see enum rte_mtr_stats_type
> >        */
> >       uint64_t stats_mask;
> > +
> > +     /** Set of supported input color protocol.
> > +      * @see enum rte_mtr_color_in_protocol
> > +      */
> > +     uint64_t input_color_proto_mask;
>
> Agree.
>
> > +
> > +     /** When non-zero, it indicates that driver supports separate
> > +      * input color table for given ethdev port.
> > +      */
> > +     int separate_input_color_table_per_port;
>
> The input color tables are actually configured per meter object, do we also need a "separate_input_color_table_per_meter" capability flag?

Yes. We have a similar limitation.


>
> >  };
> >
> >  /**
> > @@ -832,6 +945,59 @@ rte_mtr_meter_dscp_table_update(uint16_t port_id,
> >       enum rte_color *dscp_table,
> >       struct rte_mtr_error *error);
> >
> > +/**
> > + * MTR object VLAN table update
> > + *
> > + * @param[in] port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param[in] mtr_id
> > + *   MTR object ID. Needs to be valid.
> > + * @param[in] vlan_table
> > + *   When non-NULL: it points to a pre-allocated and pre-populated table with
> > + *   exactly 16 elements providing the input color for each value of the
> > + *   each value of the DEI(1bit), PCP(3 bits) input packet field.
> > + *   When NULL: it is equivalent to setting this parameter to an "all-green"
> > + *   populated table (i.e. table with all the 16 elements set to green color).
> > + * @param[out] error
> > + *   Error details. Filled in only on error, when not NULL.
> > + * @return
> > + *   0 on success, non-zero error code otherwise.
> > + */
> > +__rte_experimental
> > +int
> > +rte_mtr_meter_vlan_table_update(uint16_t port_id, uint32_t mtr_id,
> > +                             enum rte_color *vlan_table,
> > +                             struct rte_mtr_error *error);
> > +/**
> > + * Set the priority for input color protocol
> > + *
> > + * When multiple bits set in struct rte_mtr_params::input_color_proto_mask
> > + * then this API shall be used to set the priority, in the order, in
> > + * which protocol to be used to find the input color.
>
> As stated above, we should remove struct rte_mtr_params::input_color_proto_mask, as it is build implicitly by calling this function.
>
> I suggest reiterating the explanation from above:
> More than one method can be enabled for a given meter. Even if enabled, a method might not be applicable to each input packet, in case the associated protocol header is not present in the packet. The highest priority method that is both enabled for the meter and also applicable for the current input packet wins; if none is both enabled and applicable, the default input color is used. @see function rte_mtr_color_in_protocol_priority_set()

OK

>
> > + *
> > + * @param[in] port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param[in] mtr_id
> > + *   MTR object ID. Needs to be valid.
> > + * @param[in] proto
> > + *   Input color protocol to apply priority.
> > + *   MTR object's *input_color_proto_mask* should be configured
> > + *   with proto value.
> > + * @param[in] priority
> > + *   Input color protocol priority. Value zero indicates
> > + *   the highest priority.
>
> Agree.
>
> > + * @param[out] error
> > + *   Error details. Filled in only on error, when not NULL.
> > + * @return
> > + *   0 on success, non-zero error code otherwise.
> > + *
> > + * @see rte_mtr_params::input_color_proto_mask
> > + */
> > +__rte_experimental
> > +int
> > +rte_mtr_color_in_protocol_priority_set(uint16_t port_id, uint32_t mtr_id,
> > +     enum rte_mtr_color_in_protocol proto, uint32_t priority,
> > +     struct rte_mtr_error *error);
> >  /**
> >   * MTR object enabled statistics counters update
> >   *
>
> <snip>
>
> Regards,
> Cristian