RE: [PATCH 2/3] net/mlx5: add jump FDB Rx flag

[Suanming Mou <suanmingm@nvidia.com>]

> -----Original Message-----
> From: Bing Zhao <bingz@nvidia.com>
> Sent: Wednesday, February 26, 2025 6:38 PM
> To: Suanming Mou <suanmingm@nvidia.com>; Dariusz Sosnowski
> <dsosnowski@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Ori
> Kam <orika@nvidia.com>; Matan Azrad <matan@nvidia.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>
> Subject: RE: [PATCH 2/3] net/mlx5: add jump FDB Rx flag
> 
> Hi
> 
> > -----Original Message-----
> > From: Suanming Mou <suanmingm@nvidia.com>
> > Sent: Tuesday, February 25, 2025 8:45 AM
> > To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Slava Ovsiienko
> > <viacheslavo@nvidia.com>; Bing Zhao <bingz@nvidia.com>; Ori Kam
> > <orika@nvidia.com>; Matan Azrad <matan@nvidia.com>
> > Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>
> > Subject: [PATCH 2/3] net/mlx5: add jump FDB Rx flag
> >
> > When jump FDB Rx is supported, flow will be able to jump from FDB Tx
> > to FDB Rx, in that case the dest action in FDB Rx table should support
> > FDB Tx as well.
> >
> > Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> > ---
> >  drivers/common/mlx5/mlx5_devx_cmds.c | 8 ++++++++
> > drivers/common/mlx5/mlx5_devx_cmds.h | 1 +
> >  drivers/net/mlx5/linux/mlx5_os.c     | 1 +
> >  drivers/net/mlx5/mlx5.h              | 1 +
> >  drivers/net/mlx5/mlx5_flow_hw.c      | 8 +++++++-
> >  5 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c
> > b/drivers/common/mlx5/mlx5_devx_cmds.c
> > index f504b29f31..eb8553e8ad 100644
> > --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> > +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> > @@ -924,6 +924,7 @@ mlx5_devx_cmd_query_hca_attr(void *ctx,
> >  	uint32_t out[MLX5_ST_SZ_DW(query_hca_cap_out)] = {0};
> >  	bool hca_cap_2_sup;
> >  	uint64_t general_obj_types_supported = 0;
> > +	uint64_t stc_action_type_127_64;
> >  	void *hcattr;
> >  	int rc, i;
> >
> > @@ -1352,6 +1353,13 @@ mlx5_devx_cmd_query_hca_attr(void *ctx,
> >  		attr->fdb_unified_en = MLX5_GET(wqe_based_flow_table_cap,
> >  						hcattr,
> >  						fdb_unified_en);
> > +		stc_action_type_127_64 =
> MLX5_GET64(wqe_based_flow_table_cap,
> > +						    hcattr,
> > +						    stc_action_type_127_64);
> > +		if (stc_action_type_127_64 &
> > +		   (1 <<
> > (MLX5_IFC_STC_ACTION_TYPE_JUMP_FLOW_TABLE_FDB_RX_BIT_INDEX -
> > +			  MLX5_IFC_STC_ACTION_TYPE_BIT_64_INDEX)))
> > +			attr->jump_fdb_rx_en = true;
> 
> I assume integer value 1 is enough, since it is a bit field, no need to use bool
> type.
Will change it to 1.

> 
> >  	}
> >  	/* Query HCA attribute for ROCE. */
> >  	if (attr->roce) {
> > diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h
> > b/drivers/common/mlx5/mlx5_devx_cmds.h
> > index 8de4210fb2..6c726a0d46 100644
> > --- a/drivers/common/mlx5/mlx5_devx_cmds.h
> > +++ b/drivers/common/mlx5/mlx5_devx_cmds.h
> > @@ -326,6 +326,7 @@ struct mlx5_hca_attr {
> >  	uint32_t lag_rx_port_affinity:1;
> >  	uint32_t wqe_based_flow_table_sup:1;
> >  	uint32_t fdb_unified_en:1;
> > +	uint32_t jump_fdb_rx_en:1;
> >  	uint8_t max_header_modify_pattern_length;
> >  	uint64_t system_image_guid;
> >  	uint32_t log_max_conn_track_offload:5; diff --git
> > a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
> > index 9410211e3b..4e64026137 100644
> > --- a/drivers/net/mlx5/linux/mlx5_os.c
> > +++ b/drivers/net/mlx5/linux/mlx5_os.c
> > @@ -1718,6 +1718,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
> >  		priv->unified_fdb_en = !!priv->master;
> >  		DRV_LOG(DEBUG, "port %u: unified FDB %s enabled.",
> >  			eth_dev->data->port_id, priv->unified_fdb_en ? "is" :
> > "isn't");
> > +		priv->jump_fdb_rx_en = sh->cdev-
> > >config.hca_attr.jump_fdb_rx_en;
> 
> Can we add some INFO or DEBUG level log to indicate this to the user?
Sure, and I think here we have another improvement as if unified_fdb is not enabled, jump_fdb_rx_en does not make any sense. Will update.

> 
> >  		if (priv->sh->config.dv_esw_en) {
> >  			uint32_t usable_bits;
> >  			uint32_t required_bits;
> > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> > f73f6e63ff..545ba48b3c 100644
> > --- a/drivers/net/mlx5/mlx5.h
> > +++ b/drivers/net/mlx5/mlx5.h
> > @@ -1987,6 +1987,7 @@ struct mlx5_priv {
> >  	uint32_t num_lag_ports:4; /* Number of ports can be bonded. */
> >  	uint32_t tunnel_enabled:1; /* If tunnel offloading is enabled on
> > rxqs. */
> >  	uint32_t unified_fdb_en:1; /* Unified FDB flag per port. */
> > +	uint32_t jump_fdb_rx_en:1; /* Jump from FDB Tx to FDB Rx flag per
> > +port. */
> >  	uint16_t domain_id; /* Switch domain identifier. */
> >  	uint16_t vport_id; /* Associated VF vport index (if any). */
> >  	uint32_t vport_meta_tag; /* Used for vport index match ove VF LAG.
> > */ diff --git a/drivers/net/mlx5/mlx5_flow_hw.c
> > b/drivers/net/mlx5/mlx5_flow_hw.c index f0888dbf0e..83f55ed3e8 100644
> > --- a/drivers/net/mlx5/mlx5_flow_hw.c
> > +++ b/drivers/net/mlx5/mlx5_flow_hw.c
> > @@ -9322,6 +9322,7 @@ flow_hw_grp_create_cb(void *tool_ctx, void
> *cb_ctx)
> >  	struct mlx5_flow_group *grp_data;
> >  	struct mlx5dr_table *tbl = NULL;
> >  	struct mlx5dr_action *jump;
> > +	uint32_t hws_flags;
> >  	uint32_t idx = 0;
> >  	MKSTR(matcher_name, "%s_%s_%u_%u_matcher_list",
> >  	      attr->transfer ? "FDB" : "NIC", attr->egress ? "egress" :
> > "ingress", @@ -9342,10 +9343,15 @@ flow_hw_grp_create_cb(void
> > *tool_ctx, void *cb_ctx)
> >  		goto error;
> >  	grp_data->tbl = tbl;
> >  	if (attr->group) {
> > +		hws_flags = mlx5_hw_act_dest_table_flag[dr_tbl_attr.type];
> > +		/* For case of jump from FDB Tx to FDB Rx as it is supported
> > now. */
> > +		if (priv->jump_fdb_rx_en &&
> > +		    dr_tbl_attr.type == MLX5DR_TABLE_TYPE_FDB_RX)
> > +			hws_flags |= MLX5DR_ACTION_FLAG_HWS_FDB_TX;
> >  		/* Jump action be used by non-root table. */
> >  		jump = mlx5dr_action_create_dest_table
> >  			(priv->dr_ctx, tbl,
> > -			 mlx5_hw_act_dest_table_flag[dr_tbl_attr.type]);
> > +			 hws_flags);
> >  		if (!jump)
> >  			goto error;
> >  		grp_data->jump.hws_action = jump;
> > --
> > 2.34.1