Skip to contentSkip to conversation

Support mlx5 flow steering with RAW data

PATCH In review Leon Romanovsky 4 participants 19 messages Jul 8, 2018 – Jul 11, 2018

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Support mlx5 flow steering with RAW data

From: Leon Romanovsky <[email protected]>

Quoted 1 line

From Yishai:

This series introduces vendor create and destroy flow methods on the uverbs flow object by using the KABI infra-structure.

It's done in a way that enables the driver to get its specific device attributes in a raw data to match its underlay specification while still using the generic ib_flow object for cleanup and code sharing.

In addition, a specific mlx5 matcher object and its create/destroy methods were introduced. This object matches the underlay flow steering mask specification and is used as part of mlx5 create flow input data.

This series supports IB_QP/TIR as its flow steering destination as applicable today via the ib_create_flow API, however, it adds also an option to work with DEVX object which its destination can be both TIR and flow table.

Few changes were done in the mlx5 core layer to support forward compatible for the device specification raw data and to support flow table when the DEVX destination is used.

As part of this series the default IB destroy handler (i.e. uverbs_destroy_def_handler()) was exposed from IB core to be used by the drivers and existing code was refactored to use it.

Thanks

Yishai Hadas (9):
net/mlx5: Add forward compatible support for the FTE match data net/mlx5: Add support for flow table destination number
IB: Enable uverbs_destroy_def_handler to be used by drivers IB/mlx5: Introduce flow steering matcher object
IB: Consider ib_flow creation by the KABI infrastructure IB/mlx5: Introduce vendor create and destroy flow methods IB/mlx5: Support adding flow steering rule by raw data IB/mlx5: Add support for a flow table destination IB/mlx5: Expose vendor flow trees

 drivers/infiniband/core/uverbs.h                   |   3 -
 drivers/infiniband/core/uverbs_cmd.c               |   4 +
 drivers/infiniband/core/uverbs_std_types.c         |   6 +-
 drivers/infiniband/hw/mlx5/Makefile                |   1 +
 drivers/infiniband/hw/mlx5/devx.c                  |  40 ++--
 drivers/infiniband/hw/mlx5/flow.c                  | 249 +++++++++++++++++++++
 drivers/infiniband/hw/mlx5/main.c                  | 230 +++++++++++++++++--
 drivers/infiniband/hw/mlx5/mlx5_ib.h               |  31 +++
 .../mellanox/mlx5/core/diag/fs_tracepoint.c        |   3 +
 drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c   |  21 +-
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c  |  81 +------
 include/linux/mlx5/fs.h                            |   1 +
 include/linux/mlx5/mlx5_ifc.h                      |   1 +
 include/rdma/ib_verbs.h                            |  18 ++
 include/rdma/uverbs_named_ioctl.h                  |  29 ++-
 include/uapi/rdma/mlx5_user_ioctl_cmds.h           |  50 ++++-
 16 files changed, 633 insertions(+), 135 deletions(-)
 create mode 100644 drivers/infiniband/hw/mlx5/flow.c
#1 lore raw

PATCH 1/9 net/mlx5: Add forward compatible support for the FTE match data

From: Yishai Hadas <[email protected]>

Use the PRM size including the reserved when working with the FTE match data.

This comes to support forward compatibility for cases that current reserved data will be exposed by the firmware and could be used by an application by DEVX without changing the kernel.

Also drop some driver checks around the match criteria leaving the work for firmware to enable forward compatibility for future bits there.

---
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 77 +----------------------
 1 file changed, 1 insertion(+), 76 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c +1 -76
index 49a75d31185e..eba113cf1117 100644
@@ -309,89 +309,17 @@ static struct fs_prio *find_prio(struct mlx5_flow_namespace *ns,
 	return NULL;
 }
 
-static bool check_last_reserved(const u32 *match_criteria)
-{
-	char *match_criteria_reserved =
-		MLX5_ADDR_OF(fte_match_param, match_criteria, MLX5_FTE_MATCH_PARAM_RESERVED);
-
-	return	!match_criteria_reserved[0] &&
-		!memcmp(match_criteria_reserved, match_criteria_reserved + 1,
-			MLX5_FLD_SZ_BYTES(fte_match_param,
-					  MLX5_FTE_MATCH_PARAM_RESERVED) - 1);
-}
-
-static bool check_valid_mask(u8 match_criteria_enable, const u32 *match_criteria)
-{
-	if (match_criteria_enable & ~(
-		(1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_OUTER_HEADERS)   |
-		(1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_MISC_PARAMETERS) |
-		(1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_INNER_HEADERS) |
-		(1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_MISC_PARAMETERS_2)))
-		return false;
-
-	if (!(match_criteria_enable &
-	      1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_OUTER_HEADERS)) {
-		char *fg_type_mask = MLX5_ADDR_OF(fte_match_param,
-						  match_criteria, outer_headers);
-
-		if (fg_type_mask[0] ||
-		    memcmp(fg_type_mask, fg_type_mask + 1,
-			   MLX5_ST_SZ_BYTES(fte_match_set_lyr_2_4) - 1))
-			return false;
-	}
-
-	if (!(match_criteria_enable &
-	      1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_MISC_PARAMETERS)) {
-		char *fg_type_mask = MLX5_ADDR_OF(fte_match_param,
-						  match_criteria, misc_parameters);
-
-		if (fg_type_mask[0] ||
-		    memcmp(fg_type_mask, fg_type_mask + 1,
-			   MLX5_ST_SZ_BYTES(fte_match_set_misc) - 1))
-			return false;
-	}
-
-	if (!(match_criteria_enable &
-	      1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_INNER_HEADERS)) {
-		char *fg_type_mask = MLX5_ADDR_OF(fte_match_param,
-						  match_criteria, inner_headers);
-
-		if (fg_type_mask[0] ||
-		    memcmp(fg_type_mask, fg_type_mask + 1,
-			   MLX5_ST_SZ_BYTES(fte_match_set_lyr_2_4) - 1))
-			return false;
-	}
-
-	if (!(match_criteria_enable &
-	      1 << MLX5_CREATE_FLOW_GROUP_IN_MATCH_CRITERIA_ENABLE_MISC_PARAMETERS_2)) {
-		char *fg_type_mask = MLX5_ADDR_OF(fte_match_param,
-						  match_criteria, misc_parameters_2);
-
-		if (fg_type_mask[0] ||
-		    memcmp(fg_type_mask, fg_type_mask + 1,
-			   MLX5_ST_SZ_BYTES(fte_match_set_misc2) - 1))
-			return false;
-	}
-
-	return check_last_reserved(match_criteria);
-}
-
 static bool check_valid_spec(const struct mlx5_flow_spec *spec)
 {
 	int i;
 
-	if (!check_valid_mask(spec->match_criteria_enable, spec->match_criteria)) {
-		pr_warn("mlx5_core: Match criteria given mismatches match_criteria_enable\n");
-		return false;
-	}
-
 	for (i = 0; i < MLX5_ST_SZ_DW_MATCH_PARAM; i++)
 		if (spec->match_value[i] & ~spec->match_criteria[i]) {
 			pr_warn("mlx5_core: match_value differs from match_criteria\n");
 			return false;
 		}
 
-	return check_last_reserved(spec->match_value);
+	return true;
 }
 
 static struct mlx5_flow_root_namespace *find_root(struct fs_node *node)
@@ -1158,9 +1086,6 @@ struct mlx5_flow_group *mlx5_create_flow_group(struct mlx5_flow_table *ft,
 	struct mlx5_flow_group *fg;
 	int err;
 
-	if (!check_valid_mask(match_criteria_enable, match_criteria))
-		return ERR_PTR(-EINVAL);
-
 	if (ft->autogroup.active)
 		return ERR_PTR(-EPERM);
 
Signature · 1 line
2.14.4
#2 lore raw

PATCH 2/9 net/mlx5: Add support for flow table destination number

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

Add support to set a destination from a flow table number. This functionality will be used in downstream patches from this series by the DEVX stuff.

---
 .../mellanox/mlx5/core/diag/fs_tracepoint.c         |  3 +++
 drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c    | 21 ++++++++++++++-------
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c   |  4 +++-
 include/linux/mlx5/fs.h                             |  1 +
 include/linux/mlx5/mlx5_ifc.h                       |  1 +
 5 files changed, 22 insertions(+), 8 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c +3 -0
index b3820a34e773..0f11fff32a9b 100644
@@ -240,6 +240,9 @@ const char *parse_fs_dst(struct trace_seq *p,
 	case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
 		trace_seq_printf(p, "ft=%p\n", dst->ft);
 		break;
+	case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
+		trace_seq_printf(p, "ft_num=%u\n", dst->ft_num);
+		break;
 	case MLX5_FLOW_DESTINATION_TYPE_TIR:
 		trace_seq_printf(p, "tir=%u\n", dst->tir_num);
 		break;
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c +14 -7
index 5a00deff5457..a98584576c2e 100644
@@ -363,17 +363,21 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
 
 		list_for_each_entry(dst, &fte->node.children, node.list) {
 			unsigned int id;
+			unsigned int type;
 
 			if (dst->dest_attr.type == MLX5_FLOW_DESTINATION_TYPE_COUNTER)
 				continue;
 
-			MLX5_SET(dest_format_struct, in_dests, destination_type,
-				 dst->dest_attr.type);
-			if (dst->dest_attr.type ==
-			    MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE) {
+			type = dst->dest_attr.type;
+			switch (type) {
+			case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
+				id = dst->dest_attr.ft_num;
+				type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+				break;
+			case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
 				id = dst->dest_attr.ft->id;
-			} else if (dst->dest_attr.type ==
-				   MLX5_FLOW_DESTINATION_TYPE_VPORT) {
+				break;
+			case MLX5_FLOW_DESTINATION_TYPE_VPORT:
 				id = dst->dest_attr.vport.num;
 				MLX5_SET(dest_format_struct, in_dests,
 					 destination_eswitch_owner_vhca_id_valid,
@@ -381,9 +385,12 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
 				MLX5_SET(dest_format_struct, in_dests,
 					 destination_eswitch_owner_vhca_id,
 					 dst->dest_attr.vport.vhca_id);
-			} else {
+				break;
+			default:
 				id = dst->dest_attr.tir_num;
 			}
+
+			MLX5_SET(dest_format_struct, in_dests, destination_type, type);
 			MLX5_SET(dest_format_struct, in_dests, destination_id, id);
 			in_dests += MLX5_ST_SZ_BYTES(dest_format_struct);
 			list_size++;
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c +3 -1
index eba113cf1117..69aa298a0b1c 100644
@@ -1356,7 +1356,9 @@ static bool mlx5_flow_dests_cmp(struct mlx5_flow_destination *d1,
 		    (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE &&
 		     d1->ft == d2->ft) ||
 		    (d1->type == MLX5_FLOW_DESTINATION_TYPE_TIR &&
-		     d1->tir_num == d2->tir_num))
+		     d1->tir_num == d2->tir_num) ||
+		    (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM &&
+		     d1->ft_num == d2->ft_num))
 			return true;
 	}
 
include/linux/mlx5/fs.h +1 -0
index 757b4a30281e..a45febcf6b51 100644
@@ -89,6 +89,7 @@ struct mlx5_flow_destination {
 	enum mlx5_flow_destination_type	type;
 	union {
 		u32			tir_num;
+		u32			ft_num;
 		struct mlx5_flow_table	*ft;
 		struct mlx5_fc		*counter;
 		struct {
include/linux/mlx5/mlx5_ifc.h +1 -0
index 44a6ce01c3bb..fb89cc519703 100644
@@ -1181,6 +1181,7 @@ enum mlx5_flow_destination_type {
 
 	MLX5_FLOW_DESTINATION_TYPE_PORT         = 0x99,
 	MLX5_FLOW_DESTINATION_TYPE_COUNTER      = 0x100,
+	MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM = 0x101,
 };
 
 struct mlx5_ifc_dest_format_struct_bits {
Signature · 1 line
2.14.4
#3 lore raw

Re: [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number

Quoted 35 lines · Leon Romanovsky

From: Yishai Hadas <[email protected]>

Add support to set a destination from a flow table number. This functionality will be used in downstream patches from this series by the DEVX stuff.

---
 .../mellanox/mlx5/core/diag/fs_tracepoint.c         |  3 +++
 drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c    | 21 ++++++++++++++-------
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c   |  4 +++-
 include/linux/mlx5/fs.h                             |  1 +
 include/linux/mlx5/mlx5_ifc.h                       |  1 +
 5 files changed, 22 insertions(+), 8 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c +3 -0
index b3820a34e773..0f11fff32a9b 100644
@@ -240,6 +240,9 @@ const char *parse_fs_dst(struct trace_seq *p,
        case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
                trace_seq_printf(p, "ft=%p\n", dst->ft);
                break;
+       case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
+               trace_seq_printf(p, "ft_num=%u\n", dst->ft_num);
+               break;
        case MLX5_FLOW_DESTINATION_TYPE_TIR:
                trace_seq_printf(p, "tir=%u\n", dst->tir_num);
                break;
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c +0 -0
index 5a00deff5457..a98584576c2e 100644

drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev, · line 367

Quoted 5 lines
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c +1 -0
@@ -363,17 +363,21 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
 
                list_for_each_entry(dst, &fte->node.children, node.list) {
                        unsigned int id;
+                       unsigned int type;
 

reversed xmas tree, preferably just do:

    unsigned int id, type = dst->dest_attr.type;

    // use type directly instead of dst->dest_attr.type below ..
Quoted 78 lines
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c +13 -7
                        if (dst->dest_attr.type == MLX5_FLOW_DESTINATION_TYPE_COUNTER)
                                continue;
 
-                       MLX5_SET(dest_format_struct, in_dests, destination_type,
-                                dst->dest_attr.type);
-                       if (dst->dest_attr.type ==
-                           MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE) {
+                       type = dst->dest_attr.type;
+                       switch (type) {
+                       case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
+                               id = dst->dest_attr.ft_num;
+                               type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+                               break;
+                       case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
                                id = dst->dest_attr.ft->id;
-                       } else if (dst->dest_attr.type ==
-                                  MLX5_FLOW_DESTINATION_TYPE_VPORT) {
+                               break;
+                       case MLX5_FLOW_DESTINATION_TYPE_VPORT:
                                id = dst->dest_attr.vport.num;
                                MLX5_SET(dest_format_struct, in_dests,
                                         destination_eswitch_owner_vhca_id_valid,
@@ -381,9 +385,12 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
                                MLX5_SET(dest_format_struct, in_dests,
                                         destination_eswitch_owner_vhca_id,
                                         dst->dest_attr.vport.vhca_id);
-                       } else {
+                               break;
+                       default:
                                id = dst->dest_attr.tir_num;
                        }
+
+                       MLX5_SET(dest_format_struct, in_dests, destination_type, type);
                        MLX5_SET(dest_format_struct, in_dests, destination_id, id);
                        in_dests += MLX5_ST_SZ_BYTES(dest_format_struct);
                        list_size++;
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c +3 -1
index eba113cf1117..69aa298a0b1c 100644
@@ -1356,7 +1356,9 @@ static bool mlx5_flow_dests_cmp(struct mlx5_flow_destination *d1,
                    (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE &&
                     d1->ft == d2->ft) ||
                    (d1->type == MLX5_FLOW_DESTINATION_TYPE_TIR &&
-                    d1->tir_num == d2->tir_num))
+                    d1->tir_num == d2->tir_num) ||
+                   (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM &&
+                    d1->ft_num == d2->ft_num))
                        return true;
        }
 
include/linux/mlx5/fs.h +1 -0
index 757b4a30281e..a45febcf6b51 100644
@@ -89,6 +89,7 @@ struct mlx5_flow_destination {
        enum mlx5_flow_destination_type type;
        union {
                u32                     tir_num;
+               u32                     ft_num;
                struct mlx5_flow_table  *ft;
                struct mlx5_fc          *counter;
                struct {
include/linux/mlx5/mlx5_ifc.h +1 -1
index 44a6ce01c3bb..fb89cc519703 100644
@@ -1181,6 +1181,7 @@ enum mlx5_flow_destination_type {
 
        MLX5_FLOW_DESTINATION_TYPE_PORT         = 0x99,
        MLX5_FLOW_DESTINATION_TYPE_COUNTER      = 0x100,
+       MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM = 0x101,
 };
 
 struct mlx5_ifc_dest_format_struct_bits {
--

2.14.4

#4 lore raw

Re: [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number

Quoted 44 lines · Saeed Mahameed
Quoted 35 lines · Leon Romanovsky

From: Yishai Hadas <[email protected]>

Add support to set a destination from a flow table number. This functionality will be used in downstream patches from this series by the DEVX stuff.

---
  .../mellanox/mlx5/core/diag/fs_tracepoint.c         |  3 +++
  drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c    | 21 ++++++++++++++-------
  drivers/net/ethernet/mellanox/mlx5/core/fs_core.c   |  4 +++-
  include/linux/mlx5/fs.h                             |  1 +
  include/linux/mlx5/mlx5_ifc.h                       |  1 +
  5 files changed, 22 insertions(+), 8 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c +3 -0
index b3820a34e773..0f11fff32a9b 100644
@@ -240,6 +240,9 @@ const char *parse_fs_dst(struct trace_seq *p,
         case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
                 trace_seq_printf(p, "ft=%p\n", dst->ft);
                 break;
+       case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
+               trace_seq_printf(p, "ft_num=%u\n", dst->ft_num);
+               break;
         case MLX5_FLOW_DESTINATION_TYPE_TIR:
                 trace_seq_printf(p, "tir=%u\n", dst->tir_num);
                 break;
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c +0 -0
index 5a00deff5457..a98584576c2e 100644

drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev, · line 367

Quoted 5 lines
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c +1 -0
@@ -363,17 +363,21 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
 
                 list_for_each_entry(dst, &fte->node.children, node.list) {
                         unsigned int id;
+                       unsigned int type;
 

reversed xmas tree, preferably just do:

Quoted 4 lines
     unsigned int id, type = dst->dest_attr.type;

     // use type directly instead of dst->dest_attr.type below ..

OK, V1 will handle this as you suggested here.

Quoted 83 lines
Quoted 76 lines
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c +13 -7
                         if (dst->dest_attr.type == MLX5_FLOW_DESTINATION_TYPE_COUNTER)
                                 continue;
 
-                       MLX5_SET(dest_format_struct, in_dests, destination_type,
-                                dst->dest_attr.type);
-                       if (dst->dest_attr.type ==
-                           MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE) {
+                       type = dst->dest_attr.type;
+                       switch (type) {
+                       case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM:
+                               id = dst->dest_attr.ft_num;
+                               type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+                               break;
+                       case MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE:
                                 id = dst->dest_attr.ft->id;
-                       } else if (dst->dest_attr.type ==
-                                  MLX5_FLOW_DESTINATION_TYPE_VPORT) {
+                               break;
+                       case MLX5_FLOW_DESTINATION_TYPE_VPORT:
                                 id = dst->dest_attr.vport.num;
                                 MLX5_SET(dest_format_struct, in_dests,
                                          destination_eswitch_owner_vhca_id_valid,
@@ -381,9 +385,12 @@ static int mlx5_cmd_set_fte(struct mlx5_core_dev *dev,
                                 MLX5_SET(dest_format_struct, in_dests,
                                          destination_eswitch_owner_vhca_id,
                                          dst->dest_attr.vport.vhca_id);
-                       } else {
+                               break;
+                       default:
                                 id = dst->dest_attr.tir_num;
                         }
+
+                       MLX5_SET(dest_format_struct, in_dests, destination_type, type);
                         MLX5_SET(dest_format_struct, in_dests, destination_id, id);
                         in_dests += MLX5_ST_SZ_BYTES(dest_format_struct);
                         list_size++;
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c +3 -1
index eba113cf1117..69aa298a0b1c 100644
@@ -1356,7 +1356,9 @@ static bool mlx5_flow_dests_cmp(struct mlx5_flow_destination *d1,
                     (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE &&
                      d1->ft == d2->ft) ||
                     (d1->type == MLX5_FLOW_DESTINATION_TYPE_TIR &&
-                    d1->tir_num == d2->tir_num))
+                    d1->tir_num == d2->tir_num) ||
+                   (d1->type == MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM &&
+                    d1->ft_num == d2->ft_num))
                         return true;
         }
 
include/linux/mlx5/fs.h +1 -0
index 757b4a30281e..a45febcf6b51 100644
@@ -89,6 +89,7 @@ struct mlx5_flow_destination {
         enum mlx5_flow_destination_type type;
         union {
                 u32                     tir_num;
+               u32                     ft_num;
                 struct mlx5_flow_table  *ft;
                 struct mlx5_fc          *counter;
                 struct {
include/linux/mlx5/mlx5_ifc.h +1 -1
index 44a6ce01c3bb..fb89cc519703 100644
@@ -1181,6 +1181,7 @@ enum mlx5_flow_destination_type {
 
         MLX5_FLOW_DESTINATION_TYPE_PORT         = 0x99,
         MLX5_FLOW_DESTINATION_TYPE_COUNTER      = 0x100,
+       MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM = 0x101,
  };
 
  struct mlx5_ifc_dest_format_struct_bits {
--
Quoted 2 lines

2.14.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw

PATCH 3/9 IB: Enable uverbs_destroy_def_handler to be used by drivers

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

Enable uverbs_destroy_def_handler to be used by drivers and replace current code to use it.

---
 drivers/infiniband/core/uverbs.h           |  3 ---
 drivers/infiniband/core/uverbs_std_types.c |  1 +
 drivers/infiniband/hw/mlx5/devx.c          | 18 ++----------------
 include/rdma/ib_verbs.h                    |  3 +++
 4 files changed, 6 insertions(+), 19 deletions(-)
drivers/infiniband/core/uverbs.h +0 -3
index cbb727f0959f..1d06e6b4e0cd 100644
@@ -247,9 +247,6 @@ void ib_uverbs_detach_umcast(struct ib_qp *qp,
 
 void create_udata(struct uverbs_attr_bundle *ctx, struct ib_udata *udata);
 long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
-int uverbs_destroy_def_handler(struct ib_device *ib_dev,
-			       struct ib_uverbs_file *file,
-			       struct uverbs_attr_bundle *attrs);
 
 struct ib_uverbs_flow_spec {
 	union {
drivers/infiniband/core/uverbs_std_types.c +1 -0
index 912519fda3ba..718c8430d364 100644
@@ -215,6 +215,7 @@ int uverbs_destroy_def_handler(struct ib_device *ib_dev,
 {
 	return 0;
 }
+EXPORT_SYMBOL(uverbs_destroy_def_handler);
 
 void create_udata(struct uverbs_attr_bundle *ctx, struct ib_udata *udata)
 {
drivers/infiniband/hw/mlx5/devx.c +2 -16
index 192844bf6016..60ac1fbe940e 100644
@@ -682,13 +682,6 @@ static int devx_obj_cleanup(struct ib_uobject *uobject,
 	return ret;
 }
 
-static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_DESTROY)(struct ib_device *ib_dev,
-				    struct ib_uverbs_file *file,
-				    struct uverbs_attr_bundle *attrs)
-{
-	return 0;
-}
-
 static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(struct ib_device *ib_dev,
 				   struct ib_uverbs_file *file,
 				   struct uverbs_attr_bundle *attrs)
@@ -961,13 +954,6 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_UMEM_REG)(struct ib_device *ib_dev
 	return err;
 }
 
-static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_UMEM_DEREG)(struct ib_device *ib_dev,
-				   struct ib_uverbs_file *file,
-				   struct uverbs_attr_bundle *attrs)
-{
-	return 0;
-}
-
 static int devx_umem_cleanup(struct ib_uobject *uobject,
 			     enum rdma_remove_reason why)
 {
@@ -1003,7 +989,7 @@ DECLARE_UVERBS_NAMED_METHOD(
 			    UVERBS_ATTR_TYPE(u32),
 			    UA_MANDATORY));
 
-DECLARE_UVERBS_NAMED_METHOD(
+DECLARE_UVERBS_NAMED_METHOD_DESTROY(
 	MLX5_IB_METHOD_DEVX_UMEM_DEREG,
 	UVERBS_ATTR_IDR(MLX5_IB_ATTR_DEVX_UMEM_DEREG_HANDLE,
 			MLX5_IB_OBJECT_DEVX_UMEM,
@@ -1056,7 +1042,7 @@ DECLARE_UVERBS_NAMED_METHOD(
 		UVERBS_ATTR_MIN_SIZE(MLX5_ST_SZ_BYTES(general_obj_out_cmd_hdr)),
 		UA_MANDATORY));
 
-DECLARE_UVERBS_NAMED_METHOD(
+DECLARE_UVERBS_NAMED_METHOD_DESTROY(
 	MLX5_IB_METHOD_DEVX_OBJ_DESTROY,
 	UVERBS_ATTR_IDR(MLX5_IB_ATTR_DEVX_OBJ_DESTROY_HANDLE,
 			MLX5_IB_OBJECT_DEVX_OBJ,
include/rdma/ib_verbs.h +3 -0
index 031d121190fd..01b9d5a54368 100644
@@ -4160,4 +4160,7 @@ void rdma_roce_rescan_device(struct ib_device *ibdev);
 
 struct ib_ucontext *ib_uverbs_get_ucontext(struct ib_uverbs_file *ufile);
 
+int uverbs_destroy_def_handler(struct ib_device *ib_dev,
+			       struct ib_uverbs_file *file,
+			       struct uverbs_attr_bundle *attrs);
 #endif /* IB_VERBS_H */
Signature · 1 line
2.14.4
#6 lore raw

PATCH 4/9 IB/mlx5: Introduce flow steering matcher object

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

Introduce flow steering matcher object and its create and destroy methods.

This matcher object holds some mlx5 specific driver properties that matches the underlay device specification when an mlx5 flow steering group is created.

It will be used in downstream patches to be part of mlx5 specific create flow method.

---
 drivers/infiniband/hw/mlx5/Makefile      |   1 +
 drivers/infiniband/hw/mlx5/flow.c        | 132 +++++++++++++++++++++++++++++++
 drivers/infiniband/hw/mlx5/mlx5_ib.h     |  11 +++
 include/uapi/rdma/mlx5_user_ioctl_cmds.h |  33 +++++++-
 4 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 drivers/infiniband/hw/mlx5/flow.c
drivers/infiniband/hw/mlx5/Makefile +1 -0
index 577e4c418bae..b8e4b15e2674 100644
@@ -4,3 +4,4 @@ mlx5_ib-y :=	main.o cq.o doorbell.o qp.o mem.o srq.o mr.o ah.o mad.o gsi.o ib_vi
 mlx5_ib-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += odp.o
 mlx5_ib-$(CONFIG_MLX5_ESWITCH) += ib_rep.o
 mlx5_ib-$(CONFIG_INFINIBAND_USER_ACCESS) += devx.o
+mlx5_ib-$(CONFIG_INFINIBAND_USER_ACCESS) += flow.o
drivers/infiniband/hw/mlx5/flow.c new file +132 -0

132 lines not on this page · raw message

drivers/infiniband/hw/mlx5/mlx5_ib.h +11 -0
index 67e86c8304a2..2e26a1354889 100644
@@ -46,6 +46,7 @@
 #include <rdma/ib_user_verbs.h>
 #include <rdma/mlx5-abi.h>
 #include <rdma/uverbs_ioctl.h>
+#include <rdma/mlx5_user_ioctl_cmds.h>
 
 #define mlx5_ib_dbg(dev, format, arg...)				\
 pr_debug("%s:%s:%d:(pid %d): " format, (dev)->ib_dev.name, __func__,	\
@@ -179,6 +180,16 @@ struct mlx5_ib_flow_handler {
 	struct ib_counters		*ibcounters;
 };
 
+struct mlx5_ib_flow_matcher {
+	struct mlx5_ib_match_params matcher_mask;
+	u32			mask_len;
+	enum mlx5_ib_flow_type	flow_type;
+	u16			priority;
+	struct mlx5_core_dev	*mdev;
+	atomic_t		usecnt;
+	u8			match_criteria_enable;
+};
+
 struct mlx5_ib_flow_db {
 	struct mlx5_ib_flow_prio	prios[MLX5_IB_NUM_FLOW_FT];
 	struct mlx5_ib_flow_prio	sniffer[MLX5_IB_NUM_SNIFFER_FTS];
include/uapi/rdma/mlx5_user_ioctl_cmds.h +32 -1
index 1a05bb4b0b34..233d5d140179 100644
@@ -33,6 +33,7 @@
 #ifndef MLX5_USER_IOCTL_CMDS_H
 #define MLX5_USER_IOCTL_CMDS_H
 
+#include <linux/types.h>
 #include <rdma/ib_user_ioctl_cmds.h>
 
 enum mlx5_ib_create_flow_action_attrs {
@@ -112,10 +113,40 @@ enum mlx5_ib_devx_umem_methods {
 	MLX5_IB_METHOD_DEVX_UMEM_DEREG,
 };
 
-enum mlx5_ib_devx_objects {
+enum mlx5_ib_objects {
 	MLX5_IB_OBJECT_DEVX = (1U << UVERBS_ID_NS_SHIFT),
 	MLX5_IB_OBJECT_DEVX_OBJ,
 	MLX5_IB_OBJECT_DEVX_UMEM,
+	MLX5_IB_OBJECT_FLOW_MATCHER,
+};
+
+enum mlx5_ib_flow_matcher_create_attrs {
+	MLX5_IB_ATTR_FLOW_MATCHER_CREATE_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
+	MLX5_IB_ATTR_FLOW_MATCHER_MATCH_MASK,
+	MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE,
+	MLX5_IB_ATTR_FLOW_MATCHER_MATCH_CRITERIA,
+};
+
+enum mlx5_ib_flow_matcher_destroy_attrs {
+	MLX5_IB_ATTR_FLOW_MATCHER_DESTROY_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
+};
+
+enum mlx5_ib_flow_matcher_methods {
+	MLX5_IB_METHOD_FLOW_MATCHER_CREATE = (1U << UVERBS_ID_NS_SHIFT),
+	MLX5_IB_METHOD_FLOW_MATCHER_DESTROY,
+};
+
+#define MLX5_IB_DW_MATCH_PARAM 0x80
+
+struct mlx5_ib_match_params {
+	__u32	match_params[MLX5_IB_DW_MATCH_PARAM];
+};
+
+enum mlx5_ib_flow_type {
+	MLX5_IB_FLOW_TYPE_NORMAL,
+	MLX5_IB_FLOW_TYPE_SNIFFER,
+	MLX5_IB_FLOW_TYPE_ALL_DEFAULT,
+	MLX5_IB_FLOW_TYPE_MC_DEFAULT,
 };
 
 #endif
Signature · 1 line
2.14.4
#7 lore raw

Re: [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object

Quoted 50 lines · Leon Romanovsky

From: Yishai Hadas <[email protected]>

Introduce flow steering matcher object and its create and destroy methods.

This matcher object holds some mlx5 specific driver properties that matches the underlay device specification when an mlx5 flow steering group is created.

It will be used in downstream patches to be part of mlx5 specific create flow method.

 drivers/infiniband/hw/mlx5/Makefile      |   1 +
 drivers/infiniband/hw/mlx5/flow.c        | 132 +++++++++++++++++++++++++++++++
 drivers/infiniband/hw/mlx5/mlx5_ib.h     |  11 +++
 include/uapi/rdma/mlx5_user_ioctl_cmds.h |  33 +++++++-
 4 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 drivers/infiniband/hw/mlx5/flow.c
drivers/infiniband/hw/mlx5/Makefile +2 -0
index 577e4c418bae..b8e4b15e2674 100644
+++ b/drivers/infiniband/hw/mlx5/Makefile
@@ -4,3 +4,4 @@ mlx5_ib-y :=	main.o cq.o doorbell.o qp.o mem.o srq.o mr.o ah.o mad.o gsi.o ib_vi
 mlx5_ib-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += odp.o
 mlx5_ib-$(CONFIG_MLX5_ESWITCH) += ib_rep.o
 mlx5_ib-$(CONFIG_INFINIBAND_USER_ACCESS) += devx.o
+mlx5_ib-$(CONFIG_INFINIBAND_USER_ACCESS) += flow.o
drivers/infiniband/hw/mlx5/flow.c new file +17 -0
new file mode 100644
index 000000000000..99409e516c7f
+++ b/drivers/infiniband/hw/mlx5/flow.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/*
+ * Copyright (c) 2018, Mellanox Technologies inc.  All rights reserved.
+ */
+
+#include <rdma/ib_user_verbs.h>
+#include <rdma/ib_verbs.h>
+#include <rdma/uverbs_types.h>
+#include <rdma/uverbs_ioctl.h>
+#include <rdma/mlx5_user_ioctl_cmds.h>
+#include <rdma/ib_umem.h>
+#include <linux/mlx5/driver.h>
+#include <linux/mlx5/fs.h>
+#include "mlx5_ib.h"
+
+#define UVERBS_MODULE_NAME mlx5_ib

drivers/infiniband/hw/mlx5/flow.c line 26

Quoted 10 lines
drivers/infiniband/hw/mlx5/flow.c new file +10 -0
+#include <rdma/uverbs_named_ioctl.h>
+
+static const struct uverbs_attr_spec mlx5_ib_flow_type[] = {
+	[MLX5_IB_FLOW_TYPE_NORMAL] = {
+		.type = UVERBS_ATTR_TYPE_PTR_IN,
+		UVERBS_ATTR_TYPE(u16),
+	},
+	[MLX5_IB_FLOW_TYPE_SNIFFER] = {
+		/* No need to specify any data */
+		.type = UVERBS_ATTR_TYPE_PTR_IN,

I think this deserves a macro rather than a repeated comment.. Especially since we now have have a different version in uverbs_flow_action_esp_replay:

                .type = UVERBS_ATTR_TYPE_PTR_IN,
                /* No need to specify any data */
                UVERBS_ATTR_SIZE(0, 0),

Something simple like:

#define UVERBS_ATTR_NO_DATA() UVERBS_ATTR_SIZE(0, 0)

drivers/infiniband/hw/mlx5/flow.c line 45

Quoted 8 lines
drivers/infiniband/hw/mlx5/flow.c new file +8 -0
+static int flow_matcher_cleanup(struct ib_uobject *uobject,
+				enum rdma_remove_reason why)
+{
+	struct mlx5_ib_flow_matcher *obj = uobject->object;
+	int ret = atomic_read(&obj->usecnt) ? -EBUSY : 0;
+
+	if (ib_is_destroy_retryable(ret, why, uobject))
+		return ret;

This is ib_destroy_usecnt() now

drivers/infiniband/hw/mlx5/flow.c line 55

Quoted 5 lines
drivers/infiniband/hw/mlx5/flow.c new file +5 -0
+static int UVERBS_HANDLER(MLX5_IB_METHOD_FLOW_MATCHER_CREATE)(struct ib_device *ib_dev,
+				   struct ib_uverbs_file *file,
+				   struct uverbs_attr_bundle *attrs)
+{
+	struct mlx5_ib_dev *dev = to_mdev(ib_dev);

I have a patch changing these - when working with uobj's the ib_dev argument should not be used, the ib_dev must come from the ucontext instead.

drivers/infiniband/hw/mlx5/flow.c line 61

Quoted 6 lines
drivers/infiniband/hw/mlx5/flow.c new file +6 -0
+	void *cmd_in = uverbs_attr_get_alloced_ptr(attrs,
+						   MLX5_IB_ATTR_FLOW_MATCHER_MATCH_MASK);
+	struct ib_uobject *uobj;
+	struct mlx5_ib_flow_matcher *obj;
+	int mask_len;
+	int err;

So this is to be written as

	struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs,
                         MLX5_IB_ATTR_FLOW_MATCHER_CREATE_HANDLE);
	struct mlx5_ib_dev *dev = to_mdev(uobj->context->device);

drivers/infiniband/hw/mlx5/flow.c line 69

Quoted 8 lines
drivers/infiniband/hw/mlx5/flow.c new file +8 -0
+
+	obj = kzalloc(sizeof(struct mlx5_ib_flow_matcher), GFP_KERNEL);
+	if (!obj)
+		return -ENOMEM;
+
+	obj->mask_len = uverbs_attr_get_len(attrs,
+					    MLX5_IB_ATTR_FLOW_MATCHER_MATCH_MASK);
+	memcpy(obj->matcher_mask.match_params, cmd_in, obj->mask_len);

As noted before, memcpying an alloced_ptr doesn't make sense, use the copy from user version instead.

Jason

#8 lore raw

Re: [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object

Quoted 66 lines · Jason Gunthorpe
Quoted 50 lines · Leon Romanovsky

From: Yishai Hadas <[email protected]>

Introduce flow steering matcher object and its create and destroy methods.

This matcher object holds some mlx5 specific driver properties that matches the underlay device specification when an mlx5 flow steering group is created.

It will be used in downstream patches to be part of mlx5 specific create flow method.

  drivers/infiniband/hw/mlx5/Makefile      |   1 +
  drivers/infiniband/hw/mlx5/flow.c        | 132 +++++++++++++++++++++++++++++++
  drivers/infiniband/hw/mlx5/mlx5_ib.h     |  11 +++
  include/uapi/rdma/mlx5_user_ioctl_cmds.h |  33 +++++++-
  4 files changed, 176 insertions(+), 1 deletion(-)

create mode 100644 drivers/infiniband/hw/mlx5/flow.c

drivers/infiniband/hw/mlx5/Makefile +2 -0
index 577e4c418bae..b8e4b15e2674 100644
+++ b/drivers/infiniband/hw/mlx5/Makefile
@@ -4,3 +4,4 @@ mlx5_ib-y :=	main.o cq.o doorbell.o qp.o mem.o srq.o mr.o ah.o mad.o gsi.o ib_vi
  mlx5_ib-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += odp.o
  mlx5_ib-$(CONFIG_MLX5_ESWITCH) += ib_rep.o
  mlx5_ib-$(CONFIG_INFINIBAND_USER_ACCESS) += devx.o
+mlx5_ib-$(CONFIG_INFINIBAND_USER_ACCESS) += flow.o
drivers/infiniband/hw/mlx5/flow.c new file +17 -0
new file mode 100644
index 000000000000..99409e516c7f
+++ b/drivers/infiniband/hw/mlx5/flow.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/*
+ * Copyright (c) 2018, Mellanox Technologies inc.  All rights reserved.
+ */
+
+#include <rdma/ib_user_verbs.h>
+#include <rdma/ib_verbs.h>
+#include <rdma/uverbs_types.h>
+#include <rdma/uverbs_ioctl.h>
+#include <rdma/mlx5_user_ioctl_cmds.h>
+#include <rdma/ib_umem.h>
+#include <linux/mlx5/driver.h>
+#include <linux/mlx5/fs.h>
+#include "mlx5_ib.h"
+
+#define UVERBS_MODULE_NAME mlx5_ib

drivers/infiniband/hw/mlx5/flow.c line 26

Quoted 10 lines
drivers/infiniband/hw/mlx5/flow.c new file +10 -0
+#include <rdma/uverbs_named_ioctl.h>
+
+static const struct uverbs_attr_spec mlx5_ib_flow_type[] = {
+	[MLX5_IB_FLOW_TYPE_NORMAL] = {
+		.type = UVERBS_ATTR_TYPE_PTR_IN,
+		UVERBS_ATTR_TYPE(u16),
+	},
+	[MLX5_IB_FLOW_TYPE_SNIFFER] = {
+		/* No need to specify any data */
+		.type = UVERBS_ATTR_TYPE_PTR_IN,

I think this deserves a macro rather than a repeated comment.. Especially since we now have have a different version in uverbs_flow_action_esp_replay:

Quoted 7 lines
                 .type = UVERBS_ATTR_TYPE_PTR_IN,
                 /* No need to specify any data */
                 UVERBS_ATTR_SIZE(0, 0),

Something simple like:

#define UVERBS_ATTR_NO_DATA() UVERBS_ATTR_SIZE(0, 0)

OK

Quoted 12 lines

drivers/infiniband/hw/mlx5/flow.c line 45

Quoted 9 lines
drivers/infiniband/hw/mlx5/flow.c new file +8 -0
+static int flow_matcher_cleanup(struct ib_uobject *uobject,
+				enum rdma_remove_reason why)
+{
+	struct mlx5_ib_flow_matcher *obj = uobject->object;
+	int ret = atomic_read(&obj->usecnt) ? -EBUSY : 0;
+
+	if (ib_is_destroy_retryable(ret, why, uobject))
+		return ret;

This is ib_destroy_usecnt() now

OK

Quoted 10 lines

drivers/infiniband/hw/mlx5/flow.c line 55

Quoted 5 lines
drivers/infiniband/hw/mlx5/flow.c new file +5 -0
+static int UVERBS_HANDLER(MLX5_IB_METHOD_FLOW_MATCHER_CREATE)(struct ib_device *ib_dev,
+				   struct ib_uverbs_file *file,
+				   struct uverbs_attr_bundle *attrs)
+{
+	struct mlx5_ib_dev *dev = to_mdev(ib_dev);

I have a patch changing these - when working with uobj's the ib_dev argument should not be used, the ib_dev must come from the ucontext instead.

OK, V1 will take the ib_dev from the uobj.

However, does it mean that you are going to change the signatures of all the handlers to *not* get ib_dev ? can you please clarify why it must come from the uobj->ucontext ?

Quoted 12 lines

drivers/infiniband/hw/mlx5/flow.c line 61

Quoted 6 lines
drivers/infiniband/hw/mlx5/flow.c new file +6 -0
+	void *cmd_in = uverbs_attr_get_alloced_ptr(attrs,
+						   MLX5_IB_ATTR_FLOW_MATCHER_MATCH_MASK);
+	struct ib_uobject *uobj;
+	struct mlx5_ib_flow_matcher *obj;
+	int mask_len;
+	int err;

So this is to be written as

	struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs,
                          MLX5_IB_ATTR_FLOW_MATCHER_CREATE_HANDLE);
	struct mlx5_ib_dev *dev = to_mdev(uobj->context->device);
Quoted 12 lines

drivers/infiniband/hw/mlx5/flow.c line 69

Quoted 8 lines
drivers/infiniband/hw/mlx5/flow.c new file +8 -0
+
+	obj = kzalloc(sizeof(struct mlx5_ib_flow_matcher), GFP_KERNEL);
+	if (!obj)
+		return -ENOMEM;
+
+	obj->mask_len = uverbs_attr_get_len(attrs,
+					    MLX5_IB_ATTR_FLOW_MATCHER_MATCH_MASK);
+	memcpy(obj->matcher_mask.match_params, cmd_in, obj->mask_len);

As noted before, memcpying an alloced_ptr doesn't make sense, use the copy from user version instead.

OK

#9 lore raw

Re: [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object

Quoted 11 lines · Yishai Hadas
Quoted 10 lines

drivers/infiniband/hw/mlx5/flow.c line 55

Quoted 5 lines
drivers/infiniband/hw/mlx5/flow.c new file +5 -0
+static int UVERBS_HANDLER(MLX5_IB_METHOD_FLOW_MATCHER_CREATE)(struct ib_device *ib_dev,
+				   struct ib_uverbs_file *file,
+				   struct uverbs_attr_bundle *attrs)
+{
+	struct mlx5_ib_dev *dev = to_mdev(ib_dev);

I have a patch changing these - when working with uobj's the ib_dev argument should not be used, the ib_dev must come from the ucontext instead.

Quoted 4 lines

OK, V1 will take the ib_dev from the uobj.

However, does it mean that you are going to change the signatures of all the handlers to *not* get ib_dev ?

Yes.

Jason

#10 lore raw

PATCH 5/9 IB: Consider ib_flow creation by the KABI infrastructure

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

This patch considers the case that ib_flow is created by some device driver with its specific parameters using the KABI infrastructure.

In that case both QP and ib_uflow_resources might not be applicable. Downstream patches from this series use the above functionality.

---
 drivers/infiniband/core/uverbs_cmd.c       | 4 ++++
 drivers/infiniband/core/uverbs_std_types.c | 5 +++--
 include/rdma/ib_verbs.h                    | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +4 -0
index 45fa241df67c..60e6ee5bb5eb 100644
@@ -2748,6 +2748,9 @@ void ib_uverbs_flow_resources_free(struct ib_uflow_resources *uflow_res)
 {
 	unsigned int i;
 
+	if (!uflow_res)
+		return;
+
 	for (i = 0; i < uflow_res->collection_num; i++)
 		atomic_dec(&uflow_res->collection[i]->usecnt);
 
@@ -3567,6 +3570,7 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
 	}
 	atomic_inc(&qp->usecnt);
 	flow_id->qp = qp;
+	flow_id->device = qp->device;
 	flow_id->uobject = uobj;
 	uobj->object = flow_id;
 	uflow = container_of(uobj, typeof(*uflow), uobject);
drivers/infiniband/core/uverbs_std_types.c +3 -2
index 718c8430d364..c1e0492cc78a 100644
@@ -54,9 +54,10 @@ static int uverbs_free_flow(struct ib_uobject *uobject,
 	struct ib_qp *qp = flow->qp;
 	int ret;
 
-	ret = qp->device->destroy_flow(flow);
+	ret = flow->device->destroy_flow(flow);
 	if (!ret) {
-		atomic_dec(&qp->usecnt);
+		if (qp)
+			atomic_dec(&qp->usecnt);
 		ib_uverbs_flow_resources_free(uflow->resources);
 	}
 
include/rdma/ib_verbs.h +1 -0
index 01b9d5a54368..482d07d79656 100644
@@ -2100,6 +2100,7 @@ struct ib_flow_attr {
 
 struct ib_flow {
 	struct ib_qp		*qp;
+	struct ib_device	*device;
 	struct ib_uobject	*uobject;
 };
 
Signature · 1 line
2.14.4
#11 lore raw

PATCH 6/9 IB/mlx5: Introduce vendor create and destroy flow methods

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

Introduce vendor create and destroy flow methods on the uverbs flow object.

This enables the driver to get its specific device attributes to match the underlay specification while still using the generic ib_flow object for cleanup and code sharing.

The IB object's attributes are set via ib_set_flow() helper function.

The specific implementation for the given specification is added in downstream patches.

---
 drivers/infiniband/hw/mlx5/devx.c        |  22 +++++++
 drivers/infiniband/hw/mlx5/flow.c        | 108 +++++++++++++++++++++++++++++++
 drivers/infiniband/hw/mlx5/main.c        |   9 +++
 drivers/infiniband/hw/mlx5/mlx5_ib.h     |  15 +++++
 include/rdma/ib_verbs.h                  |  14 ++++
 include/rdma/uverbs_named_ioctl.h        |  29 +++++----
 include/uapi/rdma/mlx5_user_ioctl_cmds.h |  17 +++++
 7 files changed, 202 insertions(+), 12 deletions(-)
drivers/infiniband/hw/mlx5/devx.c +22 -0
index 60ac1fbe940e..32c7d8c71d85 100644
@@ -89,6 +89,28 @@ void mlx5_ib_devx_destroy(struct mlx5_ib_dev *dev,
 	mlx5_cmd_exec(dev->mdev, in, sizeof(in), out, sizeof(out));
 }
 
+bool mlx5_ib_devx_is_flow_dest(void *obj, int *dest_id, int *dest_type)
+{
+	struct devx_obj *devx_obj = obj;
+	u16 opcode = MLX5_GET(general_obj_in_cmd_hdr, devx_obj->dinbox, opcode);
+
+	switch (opcode) {
+	case MLX5_CMD_OP_DESTROY_TIR:
+		*dest_type = MLX5_FLOW_DESTINATION_TYPE_TIR;
+		*dest_id = MLX5_GET(general_obj_in_cmd_hdr, devx_obj->dinbox,
+				    obj_id);
+		return true;
+
+	case MLX5_CMD_OP_DESTROY_FLOW_TABLE:
+		*dest_type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+		*dest_id = MLX5_GET(destroy_flow_table_in, devx_obj->dinbox,
+				    table_id);
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int devx_is_valid_obj_id(struct devx_obj *obj, const void *in)
 {
 	u16 opcode = MLX5_GET(general_obj_in_cmd_hdr, in, opcode);
drivers/infiniband/hw/mlx5/flow.c +108 -0

120 lines not on this page · raw message

drivers/infiniband/hw/mlx5/main.c +9 -0
index f86b5ad2dd43..3f1d3540edaa 100644
@@ -3644,6 +3644,15 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
 	return ERR_PTR(err);
 }
 
+struct mlx5_ib_flow_handler *
+mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
+			struct mlx5_ib_flow_matcher *fs_matcher,
+			void *cmd_in, int inlen, int dest_id,
+			int dest_type)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
 static u32 mlx5_ib_flow_action_flags_to_accel_xfrm_flags(u32 mlx5_flags)
 {
 	u32 flags = 0;
drivers/infiniband/hw/mlx5/mlx5_ib.h +15 -0
index 2e26a1354889..0398c88f6707 100644
@@ -1236,6 +1236,12 @@ int mlx5_ib_devx_create(struct mlx5_ib_dev *dev,
 void mlx5_ib_devx_destroy(struct mlx5_ib_dev *dev,
 			  struct mlx5_ib_ucontext *context);
 const struct uverbs_object_tree_def *mlx5_ib_get_devx_tree(void);
+struct mlx5_ib_flow_handler *mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
+						     struct mlx5_ib_flow_matcher *fs_matcher,
+						     void *cmd_in,
+						     int inlen, int dest_id,
+						     int dest_type);
+bool mlx5_ib_devx_is_flow_dest(void *obj, int *dest_id, int *dest_type);
 #else
 static inline int
 mlx5_ib_devx_create(struct mlx5_ib_dev *dev,
@@ -1244,6 +1250,15 @@ static inline void mlx5_ib_devx_destroy(struct mlx5_ib_dev *dev,
 					struct mlx5_ib_ucontext *context) {}
 static inline const struct uverbs_object_tree_def *
 mlx5_ib_get_devx_tree(void) { return NULL; }
+static inline struct mlx5_ib_flow_handler *
+mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
+			struct mlx5_ib_flow_matcher *fs_matcher,
+			void *cmd_in,
+			int inlen, int dest_id,
+			int dest_type) { return -EOPNOTSUPP; };
+static inline bool
+mlx5_ib_devx_is_flow_dest(void *obj, int *dest_id,
+			  int *dest_type) { return false; };
 #endif
 static inline void init_query_mad(struct ib_smp *mad)
 {
include/rdma/ib_verbs.h +14 -0
index 482d07d79656..d249cba3c5cc 100644
@@ -4151,6 +4151,20 @@ ib_get_vector_affinity(struct ib_device *device, int comp_vector)
 
 }
 
+static inline void ib_set_flow(struct ib_uobject *uobj, struct ib_flow *ibflow,
+			       struct ib_qp *qp, struct ib_device *device)
+{
+	uobj->object = ibflow;
+	ibflow->uobject = uobj;
+
+	if (qp) {
+		atomic_inc(&qp->usecnt);
+		ibflow->qp = qp;
+	}
+
+	ibflow->device = device;
+}
+
 /**
  * rdma_roce_rescan_device - Rescan all of the network devices in the system
  * and add their gids, as needed, to the relevant RoCE devices.
include/rdma/uverbs_named_ioctl.h +17 -12
index 2eb1767042af..119748166335 100644
@@ -97,22 +97,14 @@
 		.methods = &UVERBS_OBJECT_METHODS(_object_id)                  \
 	}
 
-/* Used by drivers to declare a complete parsing tree for a single method that
- * differs only in having additional driver specific attributes.
+/* Used by drivers to declare a complete parsing tree for new methods
  */
-#define ADD_UVERBS_ATTRIBUTES_SIMPLE(_name, _object_id, _method_id, ...)       \
-	static const struct uverbs_attr_def *const UVERBS_METHOD_ATTRS(        \
-		_method_id)[] = { __VA_ARGS__ };                               \
-	static const struct uverbs_method_def UVERBS_METHOD(_method_id) = {    \
-		.id = _method_id,                                              \
-		.num_attrs = ARRAY_SIZE(UVERBS_METHOD_ATTRS(_method_id)),      \
-		.attrs = &UVERBS_METHOD_ATTRS(_method_id),                     \
-	};                                                                     \
+#define ADD_UVERBS_METHODS(_name, _object_id, ...)                             \
 	static const struct uverbs_method_def *const UVERBS_OBJECT_METHODS(    \
-		_object_id)[] = { &UVERBS_METHOD(_method_id) };                \
+		_object_id)[] = { __VA_ARGS__ };                               \
 	static const struct uverbs_object_def _name##_struct = {               \
 		.id = _object_id,                                              \
-		.num_methods = 1,                                              \
+		.num_methods = ARRAY_SIZE(UVERBS_OBJECT_METHODS(_object_id)),  \
 		.methods = &UVERBS_OBJECT_METHODS(_object_id)                  \
 	};                                                                     \
 	static const struct uverbs_object_def *const _name##_ptrs[] = {        \
@@ -123,4 +115,17 @@
 		.objects = &_name##_ptrs,                                      \
 	}
 
+/* Used by drivers to declare a complete parsing tree for a single method that
+ * differs only in having additional driver specific attributes.
+ */
+#define ADD_UVERBS_ATTRIBUTES_SIMPLE(_name, _object_id, _method_id, ...)       \
+	static const struct uverbs_attr_def *const UVERBS_METHOD_ATTRS(        \
+		_method_id)[] = { __VA_ARGS__ };                               \
+	static const struct uverbs_method_def UVERBS_METHOD(_method_id) = {    \
+		.id = _method_id,                                              \
+		.num_attrs = ARRAY_SIZE(UVERBS_METHOD_ATTRS(_method_id)),      \
+		.attrs = &UVERBS_METHOD_ATTRS(_method_id),                     \
+	};                                                                     \
+	ADD_UVERBS_METHODS(_name, _object_id, _method_id)
+
 #endif
include/uapi/rdma/mlx5_user_ioctl_cmds.h +17 -0
index 233d5d140179..9c51801b9e64 100644
@@ -149,4 +149,21 @@ enum mlx5_ib_flow_type {
 	MLX5_IB_FLOW_TYPE_MC_DEFAULT,
 };
 
+enum mlx5_ib_create_flow_attrs {
+	MLX5_IB_ATTR_CREATE_FLOW_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
+	MLX5_IB_ATTR_CREATE_FLOW_MATCH_VALUE,
+	MLX5_IB_ATTR_CREATE_FLOW_DEST_QP,
+	MLX5_IB_ATTR_CREATE_FLOW_DEST_DEVX,
+	MLX5_IB_ATTR_CREATE_FLOW_MATCHER,
+};
+
+enum mlx5_ib_destoy_flow_attrs {
+	MLX5_IB_ATTR_DESTROY_FLOW_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
+};
+
+enum mlx5_ib_flow_methods {
+	MLX5_IB_METHOD_CREATE_FLOW = (1U << UVERBS_ID_NS_SHIFT),
+	MLX5_IB_METHOD_DESTROY_FLOW,
+};
+
 #endif
Signature · 1 line
2.14.4
#12 lore raw

Re: [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods

drivers/infiniband/hw/mlx5/flow.c static const struct uverbs_attr_spec mlx5_ib_flow_type[] = { · line 42

Quoted 5 lines · Leon Romanovsky
drivers/infiniband/hw/mlx5/flow.c +5 -0
+static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(struct ib_device *ib_dev,
+						      struct ib_uverbs_file *file,
+						      struct uverbs_attr_bundle *attrs)
+{
+	struct mlx5_ib_dev *dev = to_mdev(ib_dev);

Same comment as before, the dev needs to come from uboj->context->device

Quoted 29 lines
include/rdma/uverbs_named_ioctl.h +6 -12
-/* Used by drivers to declare a complete parsing tree for a single method that
- * differs only in having additional driver specific attributes.
+/* Used by drivers to declare a complete parsing tree for new methods
  */
-#define ADD_UVERBS_ATTRIBUTES_SIMPLE(_name, _object_id, _method_id, ...)       \
-	static const struct uverbs_attr_def *const UVERBS_METHOD_ATTRS(        \
-		_method_id)[] = { __VA_ARGS__ };                               \
-	static const struct uverbs_method_def UVERBS_METHOD(_method_id) = {    \
-		.id = _method_id,                                              \
-		.num_attrs = ARRAY_SIZE(UVERBS_METHOD_ATTRS(_method_id)),      \
-		.attrs = &UVERBS_METHOD_ATTRS(_method_id),                     \
-	};                                                                     \
+#define ADD_UVERBS_METHODS(_name, _object_id, ...)                             \
 	static const struct uverbs_method_def *const UVERBS_OBJECT_METHODS(    \
-		_object_id)[] = { &UVERBS_METHOD(_method_id) };                \
+		_object_id)[] = { __VA_ARGS__ };                               \
 	static const struct uverbs_object_def _name##_struct = {               \
 		.id = _object_id,                                              \
-		.num_methods = 1,                                              \
+		.num_methods = ARRAY_SIZE(UVERBS_OBJECT_METHODS(_object_id)),  \
 		.methods = &UVERBS_OBJECT_METHODS(_object_id)                  \
 	};                                                                     \
 	static const struct uverbs_object_def *const _name##_ptrs[] = {        \
@@ -123,4 +115,17 @@
 		.objects = &_name##_ptrs,                                      \
 	}
 
+/* Used by drivers to declare a complete parsing tree for a single method that
+ * differs only in having additional driver specific attributes.

include/rdma/uverbs_named_ioctl.h line 129

Quoted 10 lines
include/rdma/uverbs_named_ioctl.h +10 -0
+ */
+#define ADD_UVERBS_ATTRIBUTES_SIMPLE(_name, _object_id, _method_id, ...)       \
+	static const struct uverbs_attr_def *const UVERBS_METHOD_ATTRS(        \
+		_method_id)[] = { __VA_ARGS__ };                               \
+	static const struct uverbs_method_def UVERBS_METHOD(_method_id) = {    \
+		.id = _method_id,                                              \
+		.num_attrs = ARRAY_SIZE(UVERBS_METHOD_ATTRS(_method_id)),      \
+		.attrs = &UVERBS_METHOD_ATTRS(_method_id),                     \
+	};                                                                     \
+	ADD_UVERBS_METHODS(_name, _object_id, _method_id)

Wow. How does that even compile? Oh I see, the only two users are passing in a 0 constant which the compiler will understand as NULL without a warning.

I guess this is an instant crash at runtime?

Should be:

ADD_UVERBS_METHODS(_name, _object_id, &UVERBS_METHOD(_method_id)

Jason

#13 lore raw

Re: [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods

Quoted 10 lines · Jason Gunthorpe

drivers/infiniband/hw/mlx5/flow.c static const struct uverbs_attr_spec mlx5_ib_flow_type[] = { · line 42

Quoted 5 lines · Leon Romanovsky
drivers/infiniband/hw/mlx5/flow.c +5 -0
+static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(struct ib_device *ib_dev,
+						      struct ib_uverbs_file *file,
+						      struct uverbs_attr_bundle *attrs)
+{
+	struct mlx5_ib_dev *dev = to_mdev(ib_dev);

Same comment as before, the dev needs to come from uboj->context->device

OK

Quoted 27 lines
Quoted 27 lines
include/rdma/uverbs_named_ioctl.h +4 -12
-/* Used by drivers to declare a complete parsing tree for a single method that
- * differs only in having additional driver specific attributes.
+/* Used by drivers to declare a complete parsing tree for new methods
   */
-#define ADD_UVERBS_ATTRIBUTES_SIMPLE(_name, _object_id, _method_id, ...)       \
-	static const struct uverbs_attr_def *const UVERBS_METHOD_ATTRS(        \
-		_method_id)[] = { __VA_ARGS__ };                               \
-	static const struct uverbs_method_def UVERBS_METHOD(_method_id) = {    \
-		.id = _method_id,                                              \
-		.num_attrs = ARRAY_SIZE(UVERBS_METHOD_ATTRS(_method_id)),      \
-		.attrs = &UVERBS_METHOD_ATTRS(_method_id),                     \
-	};                                                                     \
+#define ADD_UVERBS_METHODS(_name, _object_id, ...)                             \
  	static const struct uverbs_method_def *const UVERBS_OBJECT_METHODS(    \
-		_object_id)[] = { &UVERBS_METHOD(_method_id) };                \
+		_object_id)[] = { __VA_ARGS__ };                               \
  	static const struct uverbs_object_def _name##_struct = {               \
  		.id = _object_id,                                              \
-		.num_methods = 1,                                              \
+		.num_methods = ARRAY_SIZE(UVERBS_OBJECT_METHODS(_object_id)),  \
  		.methods = &UVERBS_OBJECT_METHODS(_object_id)                  \
  	};                                                                     \
  	static const struct uverbs_object_def *const _name##_ptrs[] = {        \
@@ -123,4 +115,17 @@
  		.objects = &_name##_ptrs,                                      \
  	}
 
Quoted 17 lines

include/rdma/uverbs_named_ioctl.h line 129

Quoted 12 lines
include/rdma/uverbs_named_ioctl.h +12 -0
+/* Used by drivers to declare a complete parsing tree for a single method that
+ * differs only in having additional driver specific attributes.
+ */
+#define ADD_UVERBS_ATTRIBUTES_SIMPLE(_name, _object_id, _method_id, ...)       \
+	static const struct uverbs_attr_def *const UVERBS_METHOD_ATTRS(        \
+		_method_id)[] = { __VA_ARGS__ };                               \
+	static const struct uverbs_method_def UVERBS_METHOD(_method_id) = {    \
+		.id = _method_id,                                              \
+		.num_attrs = ARRAY_SIZE(UVERBS_METHOD_ATTRS(_method_id)),      \
+		.attrs = &UVERBS_METHOD_ATTRS(_method_id),                     \
+	};                                                                     \
+	ADD_UVERBS_METHODS(_name, _object_id, _method_id)

Wow. How does that even compile? Oh I see, the only two users are passing in a 0 constant which the compiler will understand as NULL without a warning.

Yes

Quoted 2 lines

I guess this is an instant crash at runtime?

The merger code apparently skipped this method upon merging the trees and EOPNOTSUPP was returned from the parser layer. I have double checked it internally with our verification, the DM test was really failed but no crash was introduced.

Quoted 4 lines

Should be:

ADD_UVERBS_METHODS(_name, _object_id, &UVERBS_METHOD(_method_id)

Correct, will be part of V1.

Thanks,
Yishai

#14 lore raw

PATCH 7/9 IB/mlx5: Support adding flow steering rule by raw data

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

Add support to set a public flow steering rule which its destination is a TIR by using raw specification data.

The logic follows the verbs API but instead of using ib_spec(s) the raw input data is used.

---
 drivers/infiniband/hw/mlx5/main.c    | 216 ++++++++++++++++++++++++++++++++---
 drivers/infiniband/hw/mlx5/mlx5_ib.h |   2 +
 2 files changed, 201 insertions(+), 17 deletions(-)
drivers/infiniband/hw/mlx5/main.c +199 -17
index 3f1d3540edaa..614d0f69886e 100644
@@ -2981,11 +2981,11 @@ static void counters_clear_description(struct ib_counters *counters)
 
 static int mlx5_ib_destroy_flow(struct ib_flow *flow_id)
 {
-	struct mlx5_ib_dev *dev = to_mdev(flow_id->qp->device);
 	struct mlx5_ib_flow_handler *handler = container_of(flow_id,
 							  struct mlx5_ib_flow_handler,
 							  ibflow);
 	struct mlx5_ib_flow_handler *iter, *tmp;
+	struct mlx5_ib_dev *dev = handler->dev;
 
 	mutex_lock(&dev->flow_db->lock);
 
@@ -3003,6 +3003,8 @@ static int mlx5_ib_destroy_flow(struct ib_flow *flow_id)
 		counters_clear_description(handler->ibcounters);
 
 	mutex_unlock(&dev->flow_db->lock);
+	if (handler->flow_matcher)
+		atomic_dec(&handler->flow_matcher->usecnt);
 	kfree(handler);
 
 	return 0;
@@ -3023,6 +3025,26 @@ enum flow_table_type {
 
 #define MLX5_FS_MAX_TYPES	 6
 #define MLX5_FS_MAX_ENTRIES	 BIT(16)
+
+static struct mlx5_ib_flow_prio *_get_prio(struct mlx5_flow_namespace *ns,
+					   struct mlx5_ib_flow_prio *prio,
+					   int priority,
+					   int num_entries, int num_groups)
+{
+	struct mlx5_flow_table *ft;
+
+	ft = mlx5_create_auto_grouped_flow_table(ns, priority,
+						 num_entries,
+						 num_groups,
+						 0, 0);
+	if (IS_ERR(ft))
+		return ERR_CAST(ft);
+
+	prio->flow_table = ft;
+	prio->refcount = 0;
+	return prio;
+}
+
 static struct mlx5_ib_flow_prio *get_flow_table(struct mlx5_ib_dev *dev,
 						struct ib_flow_attr *flow_attr,
 						enum flow_table_type ft_type)
@@ -3035,7 +3057,6 @@ static struct mlx5_ib_flow_prio *get_flow_table(struct mlx5_ib_dev *dev,
 	int num_entries;
 	int num_groups;
 	int priority;
-	int err = 0;
 
 	max_table_size = BIT(MLX5_CAP_FLOWTABLE_NIC_RX(dev->mdev,
 						       log_max_ft_size));
@@ -3085,21 +3106,10 @@ static struct mlx5_ib_flow_prio *get_flow_table(struct mlx5_ib_dev *dev,
 		return ERR_PTR(-ENOMEM);
 
 	ft = prio->flow_table;
-	if (!ft) {
-		ft = mlx5_create_auto_grouped_flow_table(ns, priority,
-							 num_entries,
-							 num_groups,
-							 0, 0);
-
-		if (!IS_ERR(ft)) {
-			prio->refcount = 0;
-			prio->flow_table = ft;
-		} else {
-			err = PTR_ERR(ft);
-		}
-	}
+	if (!ft)
+		return _get_prio(ns, prio, priority, num_entries, num_groups);
 
-	return err ? ERR_PTR(err) : prio;
+	return prio;
 }
 
 static void set_underlay_qp(struct mlx5_ib_dev *dev,
@@ -3358,6 +3368,7 @@ static struct mlx5_ib_flow_handler *_create_flow_rule(struct mlx5_ib_dev *dev,
 
 	ft_prio->refcount++;
 	handler->prio = ft_prio;
+	handler->dev = dev;
 
 	ft_prio->flow_table = ft;
 free:
@@ -3644,13 +3655,184 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
 	return ERR_PTR(err);
 }
 
+static struct mlx5_ib_flow_prio *_get_flow_table(struct mlx5_ib_dev *dev,
+						 int priority, bool mcast)
+{
+	int max_table_size;
+	struct mlx5_flow_namespace *ns = NULL;
+	struct mlx5_ib_flow_prio *prio;
+
+	max_table_size = BIT(MLX5_CAP_FLOWTABLE_NIC_RX(dev->mdev,
+			     log_max_ft_size));
+	if (max_table_size < MLX5_FS_MAX_ENTRIES)
+		return ERR_PTR(-ENOMEM);
+
+	if (mcast)
+		priority = MLX5_IB_FLOW_MCAST_PRIO;
+	else
+		priority = ib_prio_to_core_prio(priority, false);
+
+	ns = mlx5_get_flow_namespace(dev->mdev, MLX5_FLOW_NAMESPACE_BYPASS);
+	if (!ns)
+		return ERR_PTR(-ENOTSUPP);
+
+	prio = &dev->flow_db->prios[priority];
+
+	if (prio->flow_table)
+		return prio;
+
+	return _get_prio(ns, prio, priority, MLX5_FS_MAX_ENTRIES,
+			 MLX5_FS_MAX_TYPES);
+}
+
+static struct mlx5_ib_flow_handler *
+_create_raw_flow_rule(struct mlx5_ib_dev *dev,
+		      struct mlx5_ib_flow_prio *ft_prio,
+		      struct mlx5_flow_destination *dst,
+		      struct mlx5_ib_flow_matcher  *fs_matcher,
+		      void *cmd_in, int inlen)
+{
+	struct mlx5_ib_flow_handler *handler;
+	struct mlx5_flow_act flow_act = {.flow_tag = MLX5_FS_DEFAULT_FLOW_TAG};
+	struct mlx5_flow_spec *spec;
+	struct mlx5_flow_table *ft = ft_prio->flow_table;
+	int err = 0;
+
+	spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
+	handler = kzalloc(sizeof(*handler), GFP_KERNEL);
+	if (!handler || !spec) {
+		err = -ENOMEM;
+		goto free;
+	}
+
+	INIT_LIST_HEAD(&handler->list);
+
+	memcpy(spec->match_value, cmd_in, inlen);
+	memcpy(spec->match_criteria, fs_matcher->matcher_mask.match_params,
+	       fs_matcher->mask_len);
+	spec->match_criteria_enable = fs_matcher->match_criteria_enable;
+
+	flow_act.action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+	handler->rule = mlx5_add_flow_rules(ft, spec,
+					    &flow_act, dst, 1);
+
+	if (IS_ERR(handler->rule)) {
+		err = PTR_ERR(handler->rule);
+		goto free;
+	}
+
+	ft_prio->refcount++;
+	handler->prio = ft_prio;
+	handler->dev = dev;
+	ft_prio->flow_table = ft;
+
+free:
+	if (err)
+		kfree(handler);
+	kvfree(spec);
+	return err ? ERR_PTR(err) : handler;
+}
+
+static bool raw_fs_is_multicast(struct mlx5_ib_flow_matcher *fs_matcher,
+				void *match_v)
+{
+	void *match_c;
+	void *match_v_set_lyr_2_4, *match_c_set_lyr_2_4;
+	void *dmac, *dmac_mask;
+	void *ipv4, *ipv4_mask;
+
+	if (!(fs_matcher->match_criteria_enable &
+	      (1 << MATCH_CRITERIA_ENABLE_OUTER_BIT)))
+		return false;
+
+	match_c = fs_matcher->matcher_mask.match_params;
+	match_v_set_lyr_2_4 = MLX5_ADDR_OF(fte_match_param, match_v,
+					   outer_headers);
+	match_c_set_lyr_2_4 = MLX5_ADDR_OF(fte_match_param, match_c,
+					   outer_headers);
+
+	dmac = MLX5_ADDR_OF(fte_match_set_lyr_2_4, match_v_set_lyr_2_4,
+			    dmac_47_16);
+	dmac_mask = MLX5_ADDR_OF(fte_match_set_lyr_2_4, match_c_set_lyr_2_4,
+				 dmac_47_16);
+
+	if (is_multicast_ether_addr(dmac) &&
+	    is_multicast_ether_addr(dmac_mask))
+		return true;
+
+	ipv4 = MLX5_ADDR_OF(fte_match_set_lyr_2_4, match_v_set_lyr_2_4,
+			    dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
+
+	ipv4_mask = MLX5_ADDR_OF(fte_match_set_lyr_2_4, match_c_set_lyr_2_4,
+				 dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
+
+	if (ipv4_is_multicast(*(__be32 *)(ipv4)) &&
+	    ipv4_is_multicast(*(__be32 *)(ipv4_mask)))
+		return true;
+
+	return false;
+}
+
 struct mlx5_ib_flow_handler *
 mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
 			struct mlx5_ib_flow_matcher *fs_matcher,
 			void *cmd_in, int inlen, int dest_id,
 			int dest_type)
 {
-	return ERR_PTR(-EOPNOTSUPP);
+	struct mlx5_flow_destination *dst;
+	struct mlx5_ib_flow_prio *ft_prio;
+	int priority = fs_matcher->priority;
+	struct mlx5_ib_flow_handler *handler;
+	bool mcast;
+	int err;
+
+	if (fs_matcher->flow_type != MLX5_IB_FLOW_TYPE_NORMAL)
+		return ERR_PTR(-EOPNOTSUPP);
+
+	if (fs_matcher->priority > MLX5_IB_FLOW_LAST_PRIO)
+		return ERR_PTR(-ENOMEM);
+
+	if (dest_type != MLX5_FLOW_DESTINATION_TYPE_TIR)
+		return ERR_PTR(-ENOTSUPP);
+
+	dst = kzalloc(sizeof(*dst), GFP_KERNEL);
+	if (!dst)
+		return ERR_PTR(-ENOMEM);
+
+	mcast = raw_fs_is_multicast(fs_matcher, cmd_in);
+	mutex_lock(&dev->flow_db->lock);
+
+	ft_prio = _get_flow_table(dev, priority, mcast);
+	if (IS_ERR(ft_prio)) {
+		err = PTR_ERR(ft_prio);
+		goto unlock;
+	}
+
+	dst->type = dest_type;
+	dst->tir_num = dest_id;
+	handler = _create_raw_flow_rule(dev, ft_prio, dst, fs_matcher, cmd_in,
+					inlen);
+
+	if (IS_ERR(handler)) {
+		err = PTR_ERR(handler);
+		goto destroy_ft;
+	}
+
+	mutex_unlock(&dev->flow_db->lock);
+	atomic_inc(&fs_matcher->usecnt);
+	handler->flow_matcher = fs_matcher;
+
+	kfree(dst);
+
+	return handler;
+
+destroy_ft:
+	put_flow_table(dev, ft_prio, false);
+unlock:
+	mutex_unlock(&dev->flow_db->lock);
+	kfree(dst);
+
+	return ERR_PTR(err);
 }
 
 static u32 mlx5_ib_flow_action_flags_to_accel_xfrm_flags(u32 mlx5_flags)
drivers/infiniband/hw/mlx5/mlx5_ib.h +2 -0
index 0398c88f6707..4a66c9f036cc 100644
@@ -178,6 +178,8 @@ struct mlx5_ib_flow_handler {
 	struct mlx5_ib_flow_prio	*prio;
 	struct mlx5_flow_handle		*rule;
 	struct ib_counters		*ibcounters;
+	struct mlx5_ib_dev		*dev;
+	struct mlx5_ib_flow_matcher	*flow_matcher;
 };
 
 struct mlx5_ib_flow_matcher {
Signature · 1 line
2.14.4
#15 lore raw

PATCH 8/9 IB/mlx5: Add support for a flow table destination

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

Add support to set a destination that is a flow table, this can come from the DEVX destination.

---
 drivers/infiniband/hw/mlx5/main.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
drivers/infiniband/hw/mlx5/main.c +8 -5
index 614d0f69886e..68c574d8a0ee 100644
@@ -3792,9 +3792,6 @@ mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
 	if (fs_matcher->priority > MLX5_IB_FLOW_LAST_PRIO)
 		return ERR_PTR(-ENOMEM);
 
-	if (dest_type != MLX5_FLOW_DESTINATION_TYPE_TIR)
-		return ERR_PTR(-ENOTSUPP);
-
 	dst = kzalloc(sizeof(*dst), GFP_KERNEL);
 	if (!dst)
 		return ERR_PTR(-ENOMEM);
@@ -3808,8 +3805,14 @@ mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
 		goto unlock;
 	}
 
-	dst->type = dest_type;
-	dst->tir_num = dest_id;
+	if (dest_type == MLX5_FLOW_DESTINATION_TYPE_TIR) {
+		dst->type = dest_type;
+		dst->tir_num = dest_id;
+	} else {
+		dst->type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE_NUM;
+		dst->ft_num = dest_id;
+	}
+
 	handler = _create_raw_flow_rule(dev, ft_prio, dst, fs_matcher, cmd_in,
 					inlen);
 
Signature · 1 line
2.14.4
#16 lore raw

PATCH 9/9 IB/mlx5: Expose vendor flow trees

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

Expose mlx5 flow trees to be used by upper layers.

---
 drivers/infiniband/hw/mlx5/flow.c    | 9 +++++++++
 drivers/infiniband/hw/mlx5/main.c    | 4 +++-
 drivers/infiniband/hw/mlx5/mlx5_ib.h | 3 +++
 3 files changed, 15 insertions(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/flow.c +9 -0
index 7a8018d591a5..3884861123df 100644
@@ -238,3 +238,12 @@ DECLARE_UVERBS_NAMED_OBJECT(MLX5_IB_OBJECT_FLOW_MATCHER,
 DECLARE_UVERBS_OBJECT_TREE(flow_objects,
 			&UVERBS_OBJECT(MLX5_IB_OBJECT_FLOW_MATCHER));
 
+int mlx5_ib_get_flow_trees(const struct uverbs_object_tree_def **root)
+{
+	int i = 0;
+
+	root[i++] = &flow_objects;
+	root[i++] = &mlx5_ib_fs;
+
+	return i;
+}
drivers/infiniband/hw/mlx5/main.c +3 -1
index 68c574d8a0ee..d9cf5e41d821 100644
@@ -5528,7 +5528,7 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE(
 			   UVERBS_ATTR_TYPE(u64),
 			   UA_MANDATORY));
 
-#define NUM_TREES	3
+#define NUM_TREES	5
 static int populate_specs_root(struct mlx5_ib_dev *dev)
 {
 	const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] = {
@@ -5548,6 +5548,8 @@ static int populate_specs_root(struct mlx5_ib_dev *dev)
 	    !WARN_ON(num_trees >= ARRAY_SIZE(default_root)))
 		default_root[num_trees++] = mlx5_ib_get_devx_tree();
 
+	num_trees += mlx5_ib_get_flow_trees(default_root + num_trees);
+
 	dev->ib_dev.driver_specs_root =
 		uverbs_alloc_spec_tree(num_trees, default_root);
 
drivers/infiniband/hw/mlx5/mlx5_ib.h +3 -0
index 4a66c9f036cc..3147f0ea2e2e 100644
@@ -1244,6 +1244,7 @@ struct mlx5_ib_flow_handler *mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
 						     int inlen, int dest_id,
 						     int dest_type);
 bool mlx5_ib_devx_is_flow_dest(void *obj, int *dest_id, int *dest_type);
+int mlx5_ib_get_flow_trees(const struct uverbs_object_tree_def **root);
 #else
 static inline int
 mlx5_ib_devx_create(struct mlx5_ib_dev *dev,
@@ -1261,6 +1262,8 @@ mlx5_ib_raw_fs_rule_add(struct mlx5_ib_dev *dev,
 static inline bool
 mlx5_ib_devx_is_flow_dest(void *obj, int *dest_id,
 			  int *dest_type) { return false; };
+static inline int
+mlx5_ib_get_flow_trees(const struct uverbs_object_tree_def **root) { return 0; };
 #endif
 static inline void init_query_mad(struct ib_smp *mad)
 {
Signature · 1 line
2.14.4
#17 lore raw
In reply to Leon Romanovsky
Quoted 24 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Quoted 1 line

From Yishai:

This series introduces vendor create and destroy flow methods on the uverbs flow object by using the KABI infra-structure.

It's done in a way that enables the driver to get its specific device attributes in a raw data to match its underlay specification while still using the generic ib_flow object for cleanup and code sharing.

In addition, a specific mlx5 matcher object and its create/destroy methods were introduced. This object matches the underlay flow steering mask specification and is used as part of mlx5 create flow input data.

This series supports IB_QP/TIR as its flow steering destination as applicable today via the ib_create_flow API, however, it adds also an option to work with DEVX object which its destination can be both TIR and flow table.

Few changes were done in the mlx5 core layer to support forward compatible for the device specification raw data and to support flow table when the DEVX destination is used.

Quoted 7 lines

As part of this series the default IB destroy handler (i.e. uverbs_destroy_def_handler()) was exposed from IB core to be used by the drivers and existing code was refactored to use it.

Thanks

IB: Enable uverbs_destroy_def_handler to be used by drivers

I applied this one

Quoted 9 lines

Yishai Hadas (9):
net/mlx5: Add forward compatible support for the FTE match data net/mlx5: Add support for flow table destination number IB/mlx5: Introduce flow steering matcher object
IB: Consider ib_flow creation by the KABI infrastructure IB/mlx5: Introduce vendor create and destroy flow methods IB/mlx5: Support adding flow steering rule by raw data IB/mlx5: Add support for a flow table destination IB/mlx5: Expose vendor flow trees

The rest will need to be resent after the comments are addressed.

Thanks,
Jason

#18 lore raw
Quoted 45 lines · Jason Gunthorpe
Quoted 24 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Quoted 1 line

From Yishai:

This series introduces vendor create and destroy flow methods on the uverbs flow object by using the KABI infra-structure.

It's done in a way that enables the driver to get its specific device attributes in a raw data to match its underlay specification while still using the generic ib_flow object for cleanup and code sharing.

In addition, a specific mlx5 matcher object and its create/destroy methods were introduced. This object matches the underlay flow steering mask specification and is used as part of mlx5 create flow input data.

This series supports IB_QP/TIR as its flow steering destination as applicable today via the ib_create_flow API, however, it adds also an option to work with DEVX object which its destination can be both TIR and flow table.

Few changes were done in the mlx5 core layer to support forward compatible for the device specification raw data and to support flow table when the DEVX destination is used.

Quoted 7 lines

As part of this series the default IB destroy handler (i.e. uverbs_destroy_def_handler()) was exposed from IB core to be used by the drivers and existing code was refactored to use it.

Thanks

IB: Enable uverbs_destroy_def_handler to be used by drivers

I applied this one

Quoted 9 lines

Yishai Hadas (9):
net/mlx5: Add forward compatible support for the FTE match data net/mlx5: Add support for flow table destination number IB/mlx5: Introduce flow steering matcher object
IB: Consider ib_flow creation by the KABI infrastructure IB/mlx5: Introduce vendor create and destroy flow methods IB/mlx5: Support adding flow steering rule by raw data IB/mlx5: Add support for a flow table destination IB/mlx5: Expose vendor flow trees

Quoted 1 line

The rest will need to be resent after the comments are addressed.

Jason, Saeed

Thanks for the comments, we will fix and resubmit.

Quoted 3 lines

Thanks,
Jason