ethdev: remove init_color from METER_MARK action

Message ID 20230808100036.215477-1-getelson@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: remove init_color from METER_MARK action |

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/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS

Commit Message

Gregory Etelson Aug. 8, 2023, 10 a.m. UTC
  Indirect list API defines 2 types of action update:
• Action mutable context is always shared between all flows
  that referenced indirect actions list handle.
  Action mutable context can be changed by explicit invocation
  of indirect handle update function.
• Flow mutable context is private to a flow.
  Flow mutable context can be updated by indirect list handle
  flow rule configuration.

`METER_MARK::init_color` is flow resource.
Current flows implementation placed `init_color` in the
`rte_flow_action_meter_mark` making it action level resource.

The patch removes `init_color` from the `rte_flow_action_meter_mark`
structure.

API change:
The patch removed:
• struct rte_flow_action_meter_mark::init_color

• struct rte_flow_update_meter_mark::init_color_valid

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 app/test-pmd/cmdline_flow.c     |  8 ---
 app/test-pmd/config.c           |  1 -
 drivers/net/mlx5/mlx5_flow_hw.c | 97 ++++++++++++++-------------------
 lib/ethdev/rte_flow.h           |  6 +-
 4 files changed, 43 insertions(+), 69 deletions(-)
  

Comments

Ferruh Yigit Oct. 10, 2023, 9:55 a.m. UTC | #1
On 8/8/2023 11:00 AM, Gregory Etelson wrote:
> Indirect list API defines 2 types of action update:
> • Action mutable context is always shared between all flows
>   that referenced indirect actions list handle.
>   Action mutable context can be changed by explicit invocation
>   of indirect handle update function.
> • Flow mutable context is private to a flow.
>   Flow mutable context can be updated by indirect list handle
>   flow rule configuration.
> 
> `METER_MARK::init_color` is flow resource.
> Current flows implementation placed `init_color` in the
> `rte_flow_action_meter_mark` making it action level resource.
> 
> The patch removes `init_color` from the `rte_flow_action_meter_mark`
> structure.
> 
> API change:
> The patch removed:
> • struct rte_flow_action_meter_mark::init_color
> 
> • struct rte_flow_update_meter_mark::init_color_valid
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> 


Ori, et al.,
mechanics of the patches looks good, and mlx is the only user,
if there is no objection it will be merged in a few days.
  
Ori Kam Oct. 12, 2023, 9:16 a.m. UTC | #2
Hi Gregory,

> -----Original Message-----
> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Tuesday, August 8, 2023 1:01 PM
> 
> Indirect list API defines 2 types of action update:
> • Action mutable context is always shared between all flows
>   that referenced indirect actions list handle.
>   Action mutable context can be changed by explicit invocation
>   of indirect handle update function.
> • Flow mutable context is private to a flow.
>   Flow mutable context can be updated by indirect list handle
>   flow rule configuration.
> 
> `METER_MARK::init_color` is flow resource.
> Current flows implementation placed `init_color` in the
> `rte_flow_action_meter_mark` making it action level resource.
> 
> The patch removes `init_color` from the `rte_flow_action_meter_mark`
> structure.
> 
> API change:
> The patch removed:
> • struct rte_flow_action_meter_mark::init_color
> 
> • struct rte_flow_update_meter_mark::init_color_valid
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---
>  app/test-pmd/cmdline_flow.c     |  8 ---
>  app/test-pmd/config.c           |  1 -
>  drivers/net/mlx5/mlx5_flow_hw.c | 97 ++++++++++++++-------------------
>  lib/ethdev/rte_flow.h           |  6 +-
>  4 files changed, 43 insertions(+), 69 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 94827bcc4a..54daede7cb 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -575,7 +575,6 @@ enum index {
>  	ACTION_METER_POLICY,
>  	ACTION_METER_POLICY_ID2PTR,
>  	ACTION_METER_COLOR_MODE,
> -	ACTION_METER_INIT_COLOR,
>  	ACTION_METER_STATE,
>  	ACTION_OF_DEC_NW_TTL,
>  	ACTION_OF_POP_VLAN,
> @@ -2227,7 +2226,6 @@ static const enum index action_meter_mark[] = {
>  	ACTION_METER_PROFILE,
>  	ACTION_METER_POLICY,
>  	ACTION_METER_COLOR_MODE,
> -	ACTION_METER_INIT_COLOR,
>  	ACTION_METER_STATE,
>  	ACTION_NEXT,
>  	ZERO,
> @@ -6175,12 +6173,6 @@ static const struct token token_list[] = {
>  		.args = ARGS(ARGS_ENTRY(struct
> rte_flow_action_meter_mark, color_mode)),
>  		.call = parse_vc_conf,
>  	},
> -	[ACTION_METER_INIT_COLOR] = {
> -		.name = "mtr_init_color",
> -		.help = "meter initial color",
> -		.next = NEXT(action_meter_mark,
> NEXT_ENTRY(ITEM_METER_COLOR_NAME)),
> -		.args = ARGS(ARGS_ENTRY(struct
> rte_flow_action_meter_mark, init_color)),
> -	},
>  	[ACTION_METER_STATE] = {
>  		.name = "mtr_state",
>  		.help = "meter state",
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 11f3a22048..415da109dc 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -3179,7 +3179,6 @@ port_queue_action_handle_update(portid_t port_id,
>  		if (mtr_update.meter_mark.policy)
>  			mtr_update.policy_valid = 1;
>  		mtr_update.color_mode_valid = 1;
> -		mtr_update.init_color_valid = 1;
>  		mtr_update.state_valid = 1;
>  		update = &mtr_update;
>  		break;
> diff --git a/drivers/net/mlx5/mlx5_flow_hw.c
> b/drivers/net/mlx5/mlx5_flow_hw.c
> index 5395969eb0..8ddef811ee 100644
> --- a/drivers/net/mlx5/mlx5_flow_hw.c
> +++ b/drivers/net/mlx5/mlx5_flow_hw.c
> @@ -1344,8 +1344,7 @@ flow_hw_meter_mark_alloc(struct rte_eth_dev
> *dev, uint32_t queue,
>  	aso_mtr->state = (queue == MLX5_HW_INV_QUEUE) ?
>  			  ASO_METER_WAIT : ASO_METER_WAIT_ASYNC;
>  	aso_mtr->offset = mtr_id - 1;
> -	aso_mtr->init_color = (meter_mark->color_mode) ?
> -		meter_mark->init_color : RTE_COLOR_GREEN;
> +	aso_mtr->init_color = fm->color_aware ? RTE_COLORS :
> RTE_COLOR_GREEN;
>  	/* Update ASO flow meter by wqe. */
>  	if (mlx5_aso_meter_update_by_wqe(priv->sh, queue, aso_mtr,
>  					 &priv->mtr_bulk, user_data, push)) {
> @@ -1380,9 +1379,6 @@ flow_hw_meter_mark_compile(struct rte_eth_dev
> *dev,
>  	/* Compile METER_MARK action */
>  	acts[aso_mtr_pos].action = pool->action;
>  	acts[aso_mtr_pos].aso_meter.offset = aso_mtr->offset;
> -	acts[aso_mtr_pos].aso_meter.init_color =
> -		(enum mlx5dr_action_aso_meter_color)
> -		rte_col_2_mlx5_col(aso_mtr->init_color);
>  	*index = aso_mtr->fm.meter_id;
>  	return 0;
>  }
> @@ -2068,9 +2064,6 @@ flow_hw_shared_action_construct(struct
> rte_eth_dev *dev, uint32_t queue,
>  			return -1;
>  		rule_act->action = pool->action;
>  		rule_act->aso_meter.offset = aso_mtr->offset;
> -		rule_act->aso_meter.init_color =
> -			(enum mlx5dr_action_aso_meter_color)
> -			rte_col_2_mlx5_col(aso_mtr->init_color);
>  		break;
>  	case MLX5_INDIRECT_ACTION_TYPE_QUOTA:
>  		flow_hw_construct_quota(priv, rule_act, idx);
> @@ -2483,9 +2476,6 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
>  							pool->action;
>  			rule_acts[act_data->action_dst].aso_meter.offset =
>  							aso_mtr->offset;
> -			rule_acts[act_data->action_dst].aso_meter.init_color =
> -				(enum mlx5dr_action_aso_meter_color)
> -				rte_col_2_mlx5_col(aso_mtr->init_color);
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_METER_MARK:
>  			/*
> @@ -8659,6 +8649,45 @@ flow_hw_action_handle_create(struct rte_eth_dev
> *dev, uint32_t queue,
>  	return handle;
>  }
> 
> +static int
> +mlx5_flow_update_meter_mark(struct rte_eth_dev *dev, uint32_t queue,
> +			    const struct rte_flow_update_meter_mark
> *upd_meter_mark,
> +			    uint32_t idx, bool push,
> +			    struct mlx5_hw_q_job *job, struct rte_flow_error
> *error)
> +{
> +	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_aso_mtr_pool *pool = priv->hws_mpool;
> +	const struct rte_flow_action_meter_mark *meter_mark =
> &upd_meter_mark->meter_mark;
> +	struct mlx5_aso_mtr *aso_mtr = mlx5_ipool_get(pool->idx_pool, idx);
> +	struct mlx5_flow_meter_info *fm;
> +
> +	if (!aso_mtr)
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL, "Invalid meter_mark update
> index");
> +	fm = &aso_mtr->fm;
> +	if (upd_meter_mark->profile_valid)
> +		fm->profile = (struct mlx5_flow_meter_profile *)
> +			(meter_mark->profile);
> +	if (upd_meter_mark->color_mode_valid)
> +		fm->color_aware = meter_mark->color_mode;
> +	if (upd_meter_mark->state_valid)
> +		fm->is_enable = meter_mark->state;
> +	/* Update ASO flow meter by wqe. */
> +	if (mlx5_aso_meter_update_by_wqe(priv->sh, queue,
> +					 aso_mtr, &priv->mtr_bulk, job, push))
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL, "Unable to update ASO meter
> WQE");
> +	/* Wait for ASO object completion. */
> +	if (queue == MLX5_HW_INV_QUEUE &&
> +	    mlx5_aso_mtr_wait(priv->sh, MLX5_HW_INV_QUEUE, aso_mtr))
> +		return rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL, "Unable to wait for ASO meter
> CQE");
> +	return 0;
> +}
> +
>  /**
>   * Update shared action.
>   *
> @@ -8689,15 +8718,9 @@ flow_hw_action_handle_update(struct rte_eth_dev
> *dev, uint32_t queue,
>  			     struct rte_flow_error *error)
>  {
>  	struct mlx5_priv *priv = dev->data->dev_private;
> -	struct mlx5_aso_mtr_pool *pool = priv->hws_mpool;
>  	const struct rte_flow_modify_conntrack *ct_conf =
>  		(const struct rte_flow_modify_conntrack *)update;
> -	const struct rte_flow_update_meter_mark *upd_meter_mark =
> -		(const struct rte_flow_update_meter_mark *)update;
> -	const struct rte_flow_action_meter_mark *meter_mark;
>  	struct mlx5_hw_q_job *job = NULL;
> -	struct mlx5_aso_mtr *aso_mtr;
> -	struct mlx5_flow_meter_info *fm;
>  	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
>  	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
>  	uint32_t idx = act_idx & ((1u <<
> MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
> @@ -8724,44 +8747,8 @@ flow_hw_action_handle_update(struct rte_eth_dev
> *dev, uint32_t queue,
>  		break;
>  	case MLX5_INDIRECT_ACTION_TYPE_METER_MARK:
>  		aso = true;
> -		meter_mark = &upd_meter_mark->meter_mark;
> -		/* Find ASO object. */
> -		aso_mtr = mlx5_ipool_get(pool->idx_pool, idx);
> -		if (!aso_mtr) {
> -			ret = -EINVAL;
> -			rte_flow_error_set(error, EINVAL,
> -				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> -				NULL, "Invalid meter_mark update index");
> -			break;
> -		}
> -		fm = &aso_mtr->fm;
> -		if (upd_meter_mark->profile_valid)
> -			fm->profile = (struct mlx5_flow_meter_profile *)
> -							(meter_mark-
> >profile);
> -		if (upd_meter_mark->color_mode_valid)
> -			fm->color_aware = meter_mark->color_mode;
> -		if (upd_meter_mark->init_color_valid)
> -			aso_mtr->init_color = (meter_mark->color_mode) ?
> -				meter_mark->init_color : RTE_COLOR_GREEN;
> -		if (upd_meter_mark->state_valid)
> -			fm->is_enable = meter_mark->state;
> -		/* Update ASO flow meter by wqe. */
> -		if (mlx5_aso_meter_update_by_wqe(priv->sh, queue,
> -						 aso_mtr, &priv->mtr_bulk,
> job, push)) {
> -			ret = -EINVAL;
> -			rte_flow_error_set(error, EINVAL,
> -				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> -				NULL, "Unable to update ASO meter WQE");
> -			break;
> -		}
> -		/* Wait for ASO object completion. */
> -		if (queue == MLX5_HW_INV_QUEUE &&
> -		    mlx5_aso_mtr_wait(priv->sh, MLX5_HW_INV_QUEUE,
> aso_mtr)) {
> -			ret = -EINVAL;
> -			rte_flow_error_set(error, EINVAL,
> -				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> -				NULL, "Unable to wait for ASO meter CQE");
> -		}
> +		ret = mlx5_flow_update_meter_mark(dev, queue, update, idx,
> push,
> +						  job, error);
>  		break;
>  	case MLX5_INDIRECT_ACTION_TYPE_RSS:
>  		ret = flow_dv_action_update(dev, handle, update, error);
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 86ed98c562..b44e6077ad 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -4025,8 +4025,6 @@ struct rte_flow_action_meter_mark {
>  	struct rte_flow_meter_policy *policy;
>  	/** Metering mode: 0 - Color-Blind, 1 - Color-Aware. */
>  	int color_mode;
> -	/** Initial Color applied to packets in Color-Aware mode. */
> -	enum rte_color init_color;
>  	/** Metering state: 0 - Disabled, 1 - Enabled. */
>  	int state;
>  };
> @@ -4045,12 +4043,10 @@ struct rte_flow_update_meter_mark {
>  	uint32_t policy_valid:1;
>  	/** The color mode will be updated. */
>  	uint32_t color_mode_valid:1;
> -	/** The initial color will be updated. */
> -	uint32_t init_color_valid:1;
>  	/** The meter state will be updated. */
>  	uint32_t state_valid:1;
>  	/** Reserved bits for the future usage. */
> -	uint32_t reserved:27;
> +	uint32_t reserved:28;
>  };
> 
>  /**
> --
> 2.34.1

Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori
  
Ferruh Yigit Oct. 13, 2023, 7:05 p.m. UTC | #3
On 10/12/2023 10:16 AM, Ori Kam wrote:
> Hi Gregory,
> 
>> -----Original Message-----
>> From: Gregory Etelson <getelson@nvidia.com>
>> Sent: Tuesday, August 8, 2023 1:01 PM
>>
>> Indirect list API defines 2 types of action update:
>> • Action mutable context is always shared between all flows
>>   that referenced indirect actions list handle.
>>   Action mutable context can be changed by explicit invocation
>>   of indirect handle update function.
>> • Flow mutable context is private to a flow.
>>   Flow mutable context can be updated by indirect list handle
>>   flow rule configuration.
>>
>> `METER_MARK::init_color` is flow resource.
>> Current flows implementation placed `init_color` in the
>> `rte_flow_action_meter_mark` making it action level resource.
>>
>> The patch removes `init_color` from the `rte_flow_action_meter_mark`
>> structure.
>>
>> API change:
>> The patch removed:
>> • struct rte_flow_action_meter_mark::init_color
>>
>> • struct rte_flow_update_meter_mark::init_color_valid
>>
>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>> 
> 
> Acked-by: Ori Kam <orika@nvidia.com>
> 
> 

Applied to dpdk-next-net/main, thanks.
  

Patch

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 94827bcc4a..54daede7cb 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -575,7 +575,6 @@  enum index {
 	ACTION_METER_POLICY,
 	ACTION_METER_POLICY_ID2PTR,
 	ACTION_METER_COLOR_MODE,
-	ACTION_METER_INIT_COLOR,
 	ACTION_METER_STATE,
 	ACTION_OF_DEC_NW_TTL,
 	ACTION_OF_POP_VLAN,
@@ -2227,7 +2226,6 @@  static const enum index action_meter_mark[] = {
 	ACTION_METER_PROFILE,
 	ACTION_METER_POLICY,
 	ACTION_METER_COLOR_MODE,
-	ACTION_METER_INIT_COLOR,
 	ACTION_METER_STATE,
 	ACTION_NEXT,
 	ZERO,
@@ -6175,12 +6173,6 @@  static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_meter_mark, color_mode)),
 		.call = parse_vc_conf,
 	},
-	[ACTION_METER_INIT_COLOR] = {
-		.name = "mtr_init_color",
-		.help = "meter initial color",
-		.next = NEXT(action_meter_mark, NEXT_ENTRY(ITEM_METER_COLOR_NAME)),
-		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_meter_mark, init_color)),
-	},
 	[ACTION_METER_STATE] = {
 		.name = "mtr_state",
 		.help = "meter state",
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 11f3a22048..415da109dc 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3179,7 +3179,6 @@  port_queue_action_handle_update(portid_t port_id,
 		if (mtr_update.meter_mark.policy)
 			mtr_update.policy_valid = 1;
 		mtr_update.color_mode_valid = 1;
-		mtr_update.init_color_valid = 1;
 		mtr_update.state_valid = 1;
 		update = &mtr_update;
 		break;
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 5395969eb0..8ddef811ee 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1344,8 +1344,7 @@  flow_hw_meter_mark_alloc(struct rte_eth_dev *dev, uint32_t queue,
 	aso_mtr->state = (queue == MLX5_HW_INV_QUEUE) ?
 			  ASO_METER_WAIT : ASO_METER_WAIT_ASYNC;
 	aso_mtr->offset = mtr_id - 1;
-	aso_mtr->init_color = (meter_mark->color_mode) ?
-		meter_mark->init_color : RTE_COLOR_GREEN;
+	aso_mtr->init_color = fm->color_aware ? RTE_COLORS : RTE_COLOR_GREEN;
 	/* Update ASO flow meter by wqe. */
 	if (mlx5_aso_meter_update_by_wqe(priv->sh, queue, aso_mtr,
 					 &priv->mtr_bulk, user_data, push)) {
@@ -1380,9 +1379,6 @@  flow_hw_meter_mark_compile(struct rte_eth_dev *dev,
 	/* Compile METER_MARK action */
 	acts[aso_mtr_pos].action = pool->action;
 	acts[aso_mtr_pos].aso_meter.offset = aso_mtr->offset;
-	acts[aso_mtr_pos].aso_meter.init_color =
-		(enum mlx5dr_action_aso_meter_color)
-		rte_col_2_mlx5_col(aso_mtr->init_color);
 	*index = aso_mtr->fm.meter_id;
 	return 0;
 }
@@ -2068,9 +2064,6 @@  flow_hw_shared_action_construct(struct rte_eth_dev *dev, uint32_t queue,
 			return -1;
 		rule_act->action = pool->action;
 		rule_act->aso_meter.offset = aso_mtr->offset;
-		rule_act->aso_meter.init_color =
-			(enum mlx5dr_action_aso_meter_color)
-			rte_col_2_mlx5_col(aso_mtr->init_color);
 		break;
 	case MLX5_INDIRECT_ACTION_TYPE_QUOTA:
 		flow_hw_construct_quota(priv, rule_act, idx);
@@ -2483,9 +2476,6 @@  flow_hw_actions_construct(struct rte_eth_dev *dev,
 							pool->action;
 			rule_acts[act_data->action_dst].aso_meter.offset =
 							aso_mtr->offset;
-			rule_acts[act_data->action_dst].aso_meter.init_color =
-				(enum mlx5dr_action_aso_meter_color)
-				rte_col_2_mlx5_col(aso_mtr->init_color);
 			break;
 		case RTE_FLOW_ACTION_TYPE_METER_MARK:
 			/*
@@ -8659,6 +8649,45 @@  flow_hw_action_handle_create(struct rte_eth_dev *dev, uint32_t queue,
 	return handle;
 }
 
+static int
+mlx5_flow_update_meter_mark(struct rte_eth_dev *dev, uint32_t queue,
+			    const struct rte_flow_update_meter_mark *upd_meter_mark,
+			    uint32_t idx, bool push,
+			    struct mlx5_hw_q_job *job, struct rte_flow_error *error)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_aso_mtr_pool *pool = priv->hws_mpool;
+	const struct rte_flow_action_meter_mark *meter_mark = &upd_meter_mark->meter_mark;
+	struct mlx5_aso_mtr *aso_mtr = mlx5_ipool_get(pool->idx_pool, idx);
+	struct mlx5_flow_meter_info *fm;
+
+	if (!aso_mtr)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL, "Invalid meter_mark update index");
+	fm = &aso_mtr->fm;
+	if (upd_meter_mark->profile_valid)
+		fm->profile = (struct mlx5_flow_meter_profile *)
+			(meter_mark->profile);
+	if (upd_meter_mark->color_mode_valid)
+		fm->color_aware = meter_mark->color_mode;
+	if (upd_meter_mark->state_valid)
+		fm->is_enable = meter_mark->state;
+	/* Update ASO flow meter by wqe. */
+	if (mlx5_aso_meter_update_by_wqe(priv->sh, queue,
+					 aso_mtr, &priv->mtr_bulk, job, push))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL, "Unable to update ASO meter WQE");
+	/* Wait for ASO object completion. */
+	if (queue == MLX5_HW_INV_QUEUE &&
+	    mlx5_aso_mtr_wait(priv->sh, MLX5_HW_INV_QUEUE, aso_mtr))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL, "Unable to wait for ASO meter CQE");
+	return 0;
+}
+
 /**
  * Update shared action.
  *
@@ -8689,15 +8718,9 @@  flow_hw_action_handle_update(struct rte_eth_dev *dev, uint32_t queue,
 			     struct rte_flow_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_aso_mtr_pool *pool = priv->hws_mpool;
 	const struct rte_flow_modify_conntrack *ct_conf =
 		(const struct rte_flow_modify_conntrack *)update;
-	const struct rte_flow_update_meter_mark *upd_meter_mark =
-		(const struct rte_flow_update_meter_mark *)update;
-	const struct rte_flow_action_meter_mark *meter_mark;
 	struct mlx5_hw_q_job *job = NULL;
-	struct mlx5_aso_mtr *aso_mtr;
-	struct mlx5_flow_meter_info *fm;
 	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
 	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
 	uint32_t idx = act_idx & ((1u << MLX5_INDIRECT_ACTION_TYPE_OFFSET) - 1);
@@ -8724,44 +8747,8 @@  flow_hw_action_handle_update(struct rte_eth_dev *dev, uint32_t queue,
 		break;
 	case MLX5_INDIRECT_ACTION_TYPE_METER_MARK:
 		aso = true;
-		meter_mark = &upd_meter_mark->meter_mark;
-		/* Find ASO object. */
-		aso_mtr = mlx5_ipool_get(pool->idx_pool, idx);
-		if (!aso_mtr) {
-			ret = -EINVAL;
-			rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-				NULL, "Invalid meter_mark update index");
-			break;
-		}
-		fm = &aso_mtr->fm;
-		if (upd_meter_mark->profile_valid)
-			fm->profile = (struct mlx5_flow_meter_profile *)
-							(meter_mark->profile);
-		if (upd_meter_mark->color_mode_valid)
-			fm->color_aware = meter_mark->color_mode;
-		if (upd_meter_mark->init_color_valid)
-			aso_mtr->init_color = (meter_mark->color_mode) ?
-				meter_mark->init_color : RTE_COLOR_GREEN;
-		if (upd_meter_mark->state_valid)
-			fm->is_enable = meter_mark->state;
-		/* Update ASO flow meter by wqe. */
-		if (mlx5_aso_meter_update_by_wqe(priv->sh, queue,
-						 aso_mtr, &priv->mtr_bulk, job, push)) {
-			ret = -EINVAL;
-			rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-				NULL, "Unable to update ASO meter WQE");
-			break;
-		}
-		/* Wait for ASO object completion. */
-		if (queue == MLX5_HW_INV_QUEUE &&
-		    mlx5_aso_mtr_wait(priv->sh, MLX5_HW_INV_QUEUE, aso_mtr)) {
-			ret = -EINVAL;
-			rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-				NULL, "Unable to wait for ASO meter CQE");
-		}
+		ret = mlx5_flow_update_meter_mark(dev, queue, update, idx, push,
+						  job, error);
 		break;
 	case MLX5_INDIRECT_ACTION_TYPE_RSS:
 		ret = flow_dv_action_update(dev, handle, update, error);
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 86ed98c562..b44e6077ad 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4025,8 +4025,6 @@  struct rte_flow_action_meter_mark {
 	struct rte_flow_meter_policy *policy;
 	/** Metering mode: 0 - Color-Blind, 1 - Color-Aware. */
 	int color_mode;
-	/** Initial Color applied to packets in Color-Aware mode. */
-	enum rte_color init_color;
 	/** Metering state: 0 - Disabled, 1 - Enabled. */
 	int state;
 };
@@ -4045,12 +4043,10 @@  struct rte_flow_update_meter_mark {
 	uint32_t policy_valid:1;
 	/** The color mode will be updated. */
 	uint32_t color_mode_valid:1;
-	/** The initial color will be updated. */
-	uint32_t init_color_valid:1;
 	/** The meter state will be updated. */
 	uint32_t state_valid:1;
 	/** Reserved bits for the future usage. */
-	uint32_t reserved:27;
+	uint32_t reserved:28;
 };
 
 /**