All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data
@ 2018-07-08 10:24 Leon Romanovsky
  2018-07-08 10:24 ` [PATCH mlx5-next 1/9] net/mlx5: Add forward compatible support for the FTE match data Leon Romanovsky
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Leon Romanovsky <leonro@mellanox.com>

>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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH mlx5-next 1/9] net/mlx5: Add forward compatible support for the FTE match data
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-08 10:24 ` [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number Leon Romanovsky
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 77 +----------------------
 1 file changed, 1 insertion(+), 76 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index 49a75d31185e..eba113cf1117 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -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);
 
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
  2018-07-08 10:24 ` [PATCH mlx5-next 1/9] net/mlx5: Add forward compatible support for the FTE match data Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-10 22:26   ` Saeed Mahameed
  2018-07-08 10:24 ` [PATCH rdma-next 3/9] IB: Enable uverbs_destroy_def_handler to be used by drivers Leon Romanovsky
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 .../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(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
index b3820a34e773..0f11fff32a9b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
@@ -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;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
index 5a00deff5457..a98584576c2e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
@@ -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++;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index eba113cf1117..69aa298a0b1c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -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;
 	}
 
diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
index 757b4a30281e..a45febcf6b51 100644
--- a/include/linux/mlx5/fs.h
+++ b/include/linux/mlx5/fs.h
@@ -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 {
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 44a6ce01c3bb..fb89cc519703 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH rdma-next 3/9] IB: Enable uverbs_destroy_def_handler to be used by drivers
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
  2018-07-08 10:24 ` [PATCH mlx5-next 1/9] net/mlx5: Add forward compatible support for the FTE match data Leon Romanovsky
  2018-07-08 10:24 ` [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-08 10:24 ` [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object Leon Romanovsky
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 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(-)

diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index cbb727f0959f..1d06e6b4e0cd 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -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 {
diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index 912519fda3ba..718c8430d364 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -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)
 {
diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index 192844bf6016..60ac1fbe940e 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -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,
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 031d121190fd..01b9d5a54368 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -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 */
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
                   ` (2 preceding siblings ...)
  2018-07-08 10:24 ` [PATCH rdma-next 3/9] IB: Enable uverbs_destroy_def_handler to be used by drivers Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-10 17:34   ` Jason Gunthorpe
  2018-07-08 10:24 ` [PATCH rdma-next 5/9] IB: Consider ib_flow creation by the KABI infrastructure Leon Romanovsky
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 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

diff --git a/drivers/infiniband/hw/mlx5/Makefile b/drivers/infiniband/hw/mlx5/Makefile
index 577e4c418bae..b8e4b15e2674 100644
--- a/drivers/infiniband/hw/mlx5/Makefile
+++ 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
diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
new file mode 100644
index 000000000000..99409e516c7f
--- /dev/null
+++ 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
+#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,
+	},
+	[MLX5_IB_FLOW_TYPE_ALL_DEFAULT] = {
+		/* No need to specify any data */
+		.type = UVERBS_ATTR_TYPE_PTR_IN,
+	},
+	[MLX5_IB_FLOW_TYPE_MC_DEFAULT] = {
+		/* No need to specify any data */
+		.type = UVERBS_ATTR_TYPE_PTR_IN,
+	},
+};
+
+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;
+
+	kfree(obj);
+	return 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);
+	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;
+
+	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);
+	obj->flow_type = uverbs_attr_get_enum_id(attrs,
+						 MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE);
+
+	if (obj->flow_type == MLX5_IB_FLOW_TYPE_NORMAL) {
+		err = uverbs_copy_from(&obj->priority,
+				       attrs,
+				       MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE);
+		if (err)
+			goto end;
+	}
+
+	err = uverbs_copy_from(&obj->match_criteria_enable,
+			       attrs,
+			       MLX5_IB_ATTR_FLOW_MATCHER_MATCH_CRITERIA);
+	if (err)
+		goto end;
+
+	uobj = uverbs_attr_get_uobject(attrs, MLX5_IB_ATTR_FLOW_MATCHER_CREATE_HANDLE);
+	uobj->object = obj;
+	obj->mdev = dev->mdev;
+	atomic_set(&obj->usecnt, 0);
+	return 0;
+
+end:
+	kfree(obj);
+	return err;
+}
+
+DECLARE_UVERBS_NAMED_METHOD(
+	MLX5_IB_METHOD_FLOW_MATCHER_CREATE,
+	UVERBS_ATTR_IDR(MLX5_IB_ATTR_FLOW_MATCHER_CREATE_HANDLE,
+			 MLX5_IB_OBJECT_FLOW_MATCHER,
+			 UVERBS_ACCESS_NEW,
+			 UA_MANDATORY),
+	UVERBS_ATTR_PTR_IN(
+		MLX5_IB_ATTR_FLOW_MATCHER_MATCH_MASK,
+		UVERBS_ATTR_SIZE(1, sizeof(struct mlx5_ib_match_params)),
+		UA_MANDATORY,
+		UA_ALLOC_AND_COPY),
+	UVERBS_ATTR_ENUM_IN(
+		MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE,
+		mlx5_ib_flow_type,
+		UA_MANDATORY),
+	UVERBS_ATTR_PTR_IN(
+		MLX5_IB_ATTR_FLOW_MATCHER_MATCH_CRITERIA,
+		UVERBS_ATTR_TYPE(u8),
+		UA_MANDATORY));
+
+DECLARE_UVERBS_NAMED_METHOD_DESTROY(
+	MLX5_IB_METHOD_FLOW_MATCHER_DESTROY,
+	UVERBS_ATTR_IDR(MLX5_IB_ATTR_FLOW_MATCHER_DESTROY_HANDLE,
+			 MLX5_IB_OBJECT_FLOW_MATCHER,
+			 UVERBS_ACCESS_DESTROY,
+			 UA_MANDATORY));
+
+DECLARE_UVERBS_NAMED_OBJECT(MLX5_IB_OBJECT_FLOW_MATCHER,
+			UVERBS_TYPE_ALLOC_IDR(flow_matcher_cleanup),
+			&UVERBS_METHOD(MLX5_IB_METHOD_FLOW_MATCHER_CREATE),
+			&UVERBS_METHOD(MLX5_IB_METHOD_FLOW_MATCHER_DESTROY));
+
+DECLARE_UVERBS_OBJECT_TREE(flow_objects,
+			&UVERBS_OBJECT(MLX5_IB_OBJECT_FLOW_MATCHER));
+
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 67e86c8304a2..2e26a1354889 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -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];
diff --git a/include/uapi/rdma/mlx5_user_ioctl_cmds.h b/include/uapi/rdma/mlx5_user_ioctl_cmds.h
index 1a05bb4b0b34..233d5d140179 100644
--- a/include/uapi/rdma/mlx5_user_ioctl_cmds.h
+++ b/include/uapi/rdma/mlx5_user_ioctl_cmds.h
@@ -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
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH rdma-next 5/9] IB: Consider ib_flow creation by the KABI infrastructure
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
                   ` (3 preceding siblings ...)
  2018-07-08 10:24 ` [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-08 10:24 ` [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods Leon Romanovsky
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 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(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 45fa241df67c..60e6ee5bb5eb 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -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);
diff --git a/drivers/infiniband/core/uverbs_std_types.c b/drivers/infiniband/core/uverbs_std_types.c
index 718c8430d364..c1e0492cc78a 100644
--- a/drivers/infiniband/core/uverbs_std_types.c
+++ b/drivers/infiniband/core/uverbs_std_types.c
@@ -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);
 	}
 
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 01b9d5a54368..482d07d79656 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2100,6 +2100,7 @@ struct ib_flow_attr {
 
 struct ib_flow {
 	struct ib_qp		*qp;
+	struct ib_device	*device;
 	struct ib_uobject	*uobject;
 };
 
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
                   ` (4 preceding siblings ...)
  2018-07-08 10:24 ` [PATCH rdma-next 5/9] IB: Consider ib_flow creation by the KABI infrastructure Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-10 17:44   ` Jason Gunthorpe
  2018-07-08 10:24 ` [PATCH rdma-next 7/9] IB/mlx5: Support adding flow steering rule by raw data Leon Romanovsky
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 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(-)

diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index 60ac1fbe940e..32c7d8c71d85 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -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);
diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
index 99409e516c7f..7a8018d591a5 100644
--- a/drivers/infiniband/hw/mlx5/flow.c
+++ b/drivers/infiniband/hw/mlx5/flow.c
@@ -35,6 +35,81 @@ static const struct uverbs_attr_spec mlx5_ib_flow_type[] = {
 	},
 };
 
+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);
+	struct ib_uobject *uobj;
+	struct mlx5_ib_flow_handler *flow_handler;
+	struct mlx5_ib_flow_matcher *fs_matcher;
+	void *devx_obj;
+	int dest_id, dest_type;
+	void *cmd_in;
+	int inlen;
+	bool dest_devx, dest_qp;
+	struct ib_qp *qp = NULL;
+
+	if (!capable(CAP_NET_RAW))
+		return -EPERM;
+
+	dest_devx = uverbs_attr_is_valid(attrs,
+					 MLX5_IB_ATTR_CREATE_FLOW_DEST_DEVX);
+	dest_qp = uverbs_attr_is_valid(attrs,
+				       MLX5_IB_ATTR_CREATE_FLOW_DEST_QP);
+
+	if ((dest_devx && dest_qp) || (!dest_devx && !dest_qp))
+		return -EINVAL;
+
+	if (dest_devx) {
+		devx_obj = uverbs_attr_get_obj(attrs, MLX5_IB_ATTR_CREATE_FLOW_DEST_DEVX);
+		if (IS_ERR(devx_obj))
+			return PTR_ERR(devx_obj);
+
+		/* Verify that the given DEVX object is a flow
+		 * steering destination.
+		 */
+		if (!mlx5_ib_devx_is_flow_dest(devx_obj, &dest_id, &dest_type))
+			return -EINVAL;
+	} else {
+		struct mlx5_ib_qp *mqp;
+
+		qp = uverbs_attr_get_obj(attrs,
+					 MLX5_IB_ATTR_CREATE_FLOW_DEST_QP);
+		if (IS_ERR(qp))
+			return PTR_ERR(qp);
+
+		if (qp->qp_type != IB_QPT_RAW_PACKET)
+			return -EINVAL;
+
+		mqp = to_mqp(qp);
+		if (mqp->flags & MLX5_IB_QP_RSS)
+			dest_id = mqp->rss_qp.tirn;
+		else
+			dest_id = mqp->raw_packet_qp.rq.tirn;
+		dest_type = MLX5_FLOW_DESTINATION_TYPE_TIR;
+	}
+
+	if (dev->rep)
+		return -ENOTSUPP;
+
+	cmd_in = uverbs_attr_get_alloced_ptr(attrs,
+					     MLX5_IB_ATTR_CREATE_FLOW_MATCH_VALUE);
+	inlen = uverbs_attr_get_len(attrs,
+				    MLX5_IB_ATTR_CREATE_FLOW_MATCH_VALUE);
+	fs_matcher = uverbs_attr_get_obj(attrs,
+					 MLX5_IB_ATTR_CREATE_FLOW_MATCHER);
+	flow_handler = mlx5_ib_raw_fs_rule_add(dev, fs_matcher, cmd_in, inlen,
+					       dest_id, dest_type);
+	if (IS_ERR(flow_handler))
+		return PTR_ERR(flow_handler);
+
+	uobj = uverbs_attr_get_uobject(attrs, MLX5_IB_ATTR_CREATE_FLOW_HANDLE);
+	ib_set_flow(uobj, &flow_handler->ibflow, qp, ib_dev);
+
+	return 0;
+}
+
 static int flow_matcher_cleanup(struct ib_uobject *uobject,
 				enum rdma_remove_reason why)
 {
@@ -95,6 +170,39 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_FLOW_MATCHER_CREATE)(struct ib_device *
 	return err;
 }
 
+DECLARE_UVERBS_NAMED_METHOD(
+	MLX5_IB_METHOD_CREATE_FLOW,
+	UVERBS_ATTR_IDR(MLX5_IB_ATTR_CREATE_FLOW_HANDLE,
+			UVERBS_OBJECT_FLOW,
+			UVERBS_ACCESS_NEW,
+			UA_MANDATORY),
+	UVERBS_ATTR_PTR_IN(MLX5_IB_ATTR_CREATE_FLOW_MATCH_VALUE,
+			   UVERBS_ATTR_SIZE(1, sizeof(struct mlx5_ib_match_params)),
+			   UA_MANDATORY,
+			   UA_ALLOC_AND_COPY),
+	UVERBS_ATTR_IDR(MLX5_IB_ATTR_CREATE_FLOW_MATCHER,
+			MLX5_IB_OBJECT_FLOW_MATCHER,
+			UVERBS_ACCESS_READ,
+			UA_MANDATORY),
+	UVERBS_ATTR_IDR(MLX5_IB_ATTR_CREATE_FLOW_DEST_QP, UVERBS_OBJECT_QP,
+			UVERBS_ACCESS_READ),
+	UVERBS_ATTR_IDR(MLX5_IB_ATTR_CREATE_FLOW_DEST_DEVX,
+			MLX5_IB_OBJECT_DEVX_OBJ,
+			UVERBS_ACCESS_READ));
+
+
+DECLARE_UVERBS_NAMED_METHOD_DESTROY(
+	MLX5_IB_METHOD_DESTROY_FLOW,
+	UVERBS_ATTR_IDR(MLX5_IB_ATTR_CREATE_FLOW_HANDLE,
+			UVERBS_OBJECT_FLOW,
+			UVERBS_ACCESS_DESTROY,
+			UA_MANDATORY));
+
+ADD_UVERBS_METHODS(mlx5_ib_fs,
+		   UVERBS_OBJECT_FLOW,
+		   &UVERBS_METHOD(MLX5_IB_METHOD_CREATE_FLOW),
+		   &UVERBS_METHOD(MLX5_IB_METHOD_DESTROY_FLOW));
+
 DECLARE_UVERBS_NAMED_METHOD(
 	MLX5_IB_METHOD_FLOW_MATCHER_CREATE,
 	UVERBS_ATTR_IDR(MLX5_IB_ATTR_FLOW_MATCHER_CREATE_HANDLE,
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index f86b5ad2dd43..3f1d3540edaa 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -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;
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 2e26a1354889..0398c88f6707 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -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)
 {
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 482d07d79656..d249cba3c5cc 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -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.
diff --git a/include/rdma/uverbs_named_ioctl.h b/include/rdma/uverbs_named_ioctl.h
index 2eb1767042af..119748166335 100644
--- a/include/rdma/uverbs_named_ioctl.h
+++ b/include/rdma/uverbs_named_ioctl.h
@@ -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
diff --git a/include/uapi/rdma/mlx5_user_ioctl_cmds.h b/include/uapi/rdma/mlx5_user_ioctl_cmds.h
index 233d5d140179..9c51801b9e64 100644
--- a/include/uapi/rdma/mlx5_user_ioctl_cmds.h
+++ b/include/uapi/rdma/mlx5_user_ioctl_cmds.h
@@ -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
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH rdma-next 7/9] IB/mlx5: Support adding flow steering rule by raw data
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
                   ` (5 preceding siblings ...)
  2018-07-08 10:24 ` [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-08 10:24 ` [PATCH rdma-next 8/9] IB/mlx5: Add support for a flow table destination Leon Romanovsky
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c    | 216 ++++++++++++++++++++++++++++++++---
 drivers/infiniband/hw/mlx5/mlx5_ib.h |   2 +
 2 files changed, 201 insertions(+), 17 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 3f1d3540edaa..614d0f69886e 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -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)
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 0398c88f6707..4a66c9f036cc 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -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 {
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH rdma-next 8/9] IB/mlx5: Add support for a flow table destination
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
                   ` (6 preceding siblings ...)
  2018-07-08 10:24 ` [PATCH rdma-next 7/9] IB/mlx5: Support adding flow steering rule by raw data Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-08 10:24 ` [PATCH rdma-next 9/9] IB/mlx5: Expose vendor flow trees Leon Romanovsky
  2018-07-10 18:28 ` [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Jason Gunthorpe
  9 siblings, 0 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

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

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 614d0f69886e..68c574d8a0ee 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -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);
 
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH rdma-next 9/9] IB/mlx5: Expose vendor flow trees
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
                   ` (7 preceding siblings ...)
  2018-07-08 10:24 ` [PATCH rdma-next 8/9] IB/mlx5: Add support for a flow table destination Leon Romanovsky
@ 2018-07-08 10:24 ` Leon Romanovsky
  2018-07-10 18:28 ` [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Jason Gunthorpe
  9 siblings, 0 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-08 10:24 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yishai Hadas, Saeed Mahameed,
	linux-netdev

From: Yishai Hadas <yishaih@mellanox.com>

Expose mlx5 flow trees to be used by upper layers.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 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(-)

diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
index 7a8018d591a5..3884861123df 100644
--- a/drivers/infiniband/hw/mlx5/flow.c
+++ b/drivers/infiniband/hw/mlx5/flow.c
@@ -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;
+}
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 68c574d8a0ee..d9cf5e41d821 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -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);
 
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 4a66c9f036cc..3147f0ea2e2e 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -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)
 {
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object
  2018-07-08 10:24 ` [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object Leon Romanovsky
@ 2018-07-10 17:34   ` Jason Gunthorpe
  2018-07-11  9:32       ` Yishai Hadas
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Gunthorpe @ 2018-07-10 17:34 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Yishai Hadas,
	Saeed Mahameed, linux-netdev

On Sun, Jul 08, 2018 at 01:24:40PM +0300, Leon Romanovsky wrote:
> From: Yishai Hadas <yishaih@mellanox.com>
> 
> 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.
> 
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  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
> 
> diff --git a/drivers/infiniband/hw/mlx5/Makefile b/drivers/infiniband/hw/mlx5/Makefile
> 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
> diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
> 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
> +#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)

> +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

> +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.

> +	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);

> +
> +	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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods
  2018-07-08 10:24 ` [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods Leon Romanovsky
@ 2018-07-10 17:44   ` Jason Gunthorpe
  2018-07-11  9:44       ` Yishai Hadas
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Gunthorpe @ 2018-07-10 17:44 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Yishai Hadas,
	Saeed Mahameed, linux-netdev

On Sun, Jul 08, 2018 at 01:24:42PM +0300, Leon Romanovsky wrote:

> +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

> -/* 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)

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data
  2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
                   ` (8 preceding siblings ...)
  2018-07-08 10:24 ` [PATCH rdma-next 9/9] IB/mlx5: Expose vendor flow trees Leon Romanovsky
@ 2018-07-10 18:28 ` Jason Gunthorpe
  2018-07-11  5:43   ` Leon Romanovsky
  9 siblings, 1 reply; 21+ messages in thread
From: Jason Gunthorpe @ 2018-07-10 18:28 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Yishai Hadas,
	Saeed Mahameed, linux-netdev

On Sun, Jul 08, 2018 at 01:24:36PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> >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

>   IB: Enable uverbs_destroy_def_handler to be used by drivers

I applied this one

> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number
  2018-07-08 10:24 ` [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number Leon Romanovsky
@ 2018-07-10 22:26   ` Saeed Mahameed
  2018-07-11  9:47     ` Yishai Hadas
  0 siblings, 1 reply; 21+ messages in thread
From: Saeed Mahameed @ 2018-07-10 22:26 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Jason Gunthorpe, Leon Romanovsky,
	RDMA mailing list, Yishai Hadas, Saeed Mahameed, linux-netdev

On Sun, Jul 8, 2018 at 3:24 AM, Leon Romanovsky <leon@kernel.org> wrote:
> From: Yishai Hadas <yishaih@mellanox.com>
>
> 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.
>
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  .../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(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
> index b3820a34e773..0f11fff32a9b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
> @@ -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;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> index 5a00deff5457..a98584576c2e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> @@ -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 ..


>                         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++;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> index eba113cf1117..69aa298a0b1c 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> @@ -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;
>         }
>
> diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
> index 757b4a30281e..a45febcf6b51 100644
> --- a/include/linux/mlx5/fs.h
> +++ b/include/linux/mlx5/fs.h
> @@ -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 {
> diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
> index 44a6ce01c3bb..fb89cc519703 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -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
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data
  2018-07-10 18:28 ` [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Jason Gunthorpe
@ 2018-07-11  5:43   ` Leon Romanovsky
  0 siblings, 0 replies; 21+ messages in thread
From: Leon Romanovsky @ 2018-07-11  5:43 UTC (permalink / raw)
  To: Jason Gunthorpe, Saeed Mahameed
  Cc: Doug Ledford, RDMA mailing list, Yishai Hadas, linux-netdev

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

On Tue, Jul 10, 2018 at 12:28:54PM -0600, Jason Gunthorpe wrote:
> On Sun, Jul 08, 2018 at 01:24:36PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > >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
>
> >   IB: Enable uverbs_destroy_def_handler to be used by drivers
>
> I applied this one
>
> > 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.

Jason, Saeed

Thanks for the comments, we will fix and resubmit.

>
> Thanks,
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object
  2018-07-10 17:34   ` Jason Gunthorpe
@ 2018-07-11  9:32       ` Yishai Hadas
  0 siblings, 0 replies; 21+ messages in thread
From: Yishai Hadas @ 2018-07-11  9:32 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky,
	RDMA mailing list, Yishai Hadas, Saeed Mahameed, linux-netdev,
	Majd Dibbiny

On 7/10/2018 8:34 PM, Jason Gunthorpe wrote:
> On Sun, Jul 08, 2018 at 01:24:40PM +0300, Leon Romanovsky wrote:
>> From: Yishai Hadas <yishaih@mellanox.com>
>>
>> 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.
>>
>> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
>> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>>   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
>>
>> diff --git a/drivers/infiniband/hw/mlx5/Makefile b/drivers/infiniband/hw/mlx5/Makefile
>> 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
>> diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
>> 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
>> +#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)

OK

> 
>> +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
>> +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 ?


>> +	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);
> 
>> +
>> +	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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object
@ 2018-07-11  9:32       ` Yishai Hadas
  0 siblings, 0 replies; 21+ messages in thread
From: Yishai Hadas @ 2018-07-11  9:32 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky,
	RDMA mailing list, Yishai Hadas, Saeed Mahameed, linux-netdev,
	Majd Dibbiny, Yishai Hadas

On 7/10/2018 8:34 PM, Jason Gunthorpe wrote:
> On Sun, Jul 08, 2018 at 01:24:40PM +0300, Leon Romanovsky wrote:
>> From: Yishai Hadas <yishaih@mellanox.com>
>>
>> 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.
>>
>> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
>> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>>   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
>>
>> diff --git a/drivers/infiniband/hw/mlx5/Makefile b/drivers/infiniband/hw/mlx5/Makefile
>> 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
>> diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
>> 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
>> +#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)

OK

> 
>> +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
>> +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 ?


>> +	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);
> 
>> +
>> +	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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods
  2018-07-10 17:44   ` Jason Gunthorpe
@ 2018-07-11  9:44       ` Yishai Hadas
  0 siblings, 0 replies; 21+ messages in thread
From: Yishai Hadas @ 2018-07-11  9:44 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky,
	RDMA mailing list, Yishai Hadas, Saeed Mahameed, linux-netdev

On 7/10/2018 8:44 PM, Jason Gunthorpe wrote:
> On Sun, Jul 08, 2018 at 01:24:42PM +0300, Leon Romanovsky wrote:
> 
>> +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

>> -/* 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)
> 
> 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

> 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.

> Should be:
> 
> 	ADD_UVERBS_METHODS(_name, _object_id, &UVERBS_METHOD(_method_id)
> 

Correct, will be part of V1.

Thanks,
Yishai

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods
@ 2018-07-11  9:44       ` Yishai Hadas
  0 siblings, 0 replies; 21+ messages in thread
From: Yishai Hadas @ 2018-07-11  9:44 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky,
	RDMA mailing list, Yishai Hadas, Saeed Mahameed, linux-netdev,
	Yishai Hadas, Majd Dibbiny

On 7/10/2018 8:44 PM, Jason Gunthorpe wrote:
> On Sun, Jul 08, 2018 at 01:24:42PM +0300, Leon Romanovsky wrote:
> 
>> +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

>> -/* 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)
> 
> 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

> 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.

> Should be:
> 
> 	ADD_UVERBS_METHODS(_name, _object_id, &UVERBS_METHOD(_method_id)
> 

Correct, will be part of V1.

Thanks,
Yishai

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number
  2018-07-10 22:26   ` Saeed Mahameed
@ 2018-07-11  9:47     ` Yishai Hadas
  0 siblings, 0 replies; 21+ messages in thread
From: Yishai Hadas @ 2018-07-11  9:47 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: Leon Romanovsky, Doug Ledford, Jason Gunthorpe, Leon Romanovsky,
	RDMA mailing list, Yishai Hadas, Saeed Mahameed, linux-netdev,
	Majd Dibbiny

On 7/11/2018 1:26 AM, Saeed Mahameed wrote:
> On Sun, Jul 8, 2018 at 3:24 AM, Leon Romanovsky <leon@kernel.org> wrote:
>> From: Yishai Hadas <yishaih@mellanox.com>
>>
>> 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.
>>
>> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
>> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>> ---
>>   .../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(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
>> index b3820a34e773..0f11fff32a9b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.c
>> @@ -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;
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
>> index 5a00deff5457..a98584576c2e 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
>> @@ -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 ..
> 

OK, V1 will handle this as you suggested here.


>>                          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++;
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> index eba113cf1117..69aa298a0b1c 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>> @@ -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;
>>          }
>>
>> diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
>> index 757b4a30281e..a45febcf6b51 100644
>> --- a/include/linux/mlx5/fs.h
>> +++ b/include/linux/mlx5/fs.h
>> @@ -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 {
>> diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
>> index 44a6ce01c3bb..fb89cc519703 100644
>> --- a/include/linux/mlx5/mlx5_ifc.h
>> +++ b/include/linux/mlx5/mlx5_ifc.h
>> @@ -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
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object
  2018-07-11  9:32       ` Yishai Hadas
  (?)
@ 2018-07-11 12:02       ` Jason Gunthorpe
  -1 siblings, 0 replies; 21+ messages in thread
From: Jason Gunthorpe @ 2018-07-11 12:02 UTC (permalink / raw)
  To: Yishai Hadas
  Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky,
	RDMA mailing list, Yishai Hadas, Saeed Mahameed, linux-netdev,
	Majd Dibbiny

On Wed, Jul 11, 2018 at 12:32:56PM +0300, Yishai Hadas wrote:
> >>+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 ?

Yes.

Jason

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-07-11 12:02 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-08 10:24 [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Leon Romanovsky
2018-07-08 10:24 ` [PATCH mlx5-next 1/9] net/mlx5: Add forward compatible support for the FTE match data Leon Romanovsky
2018-07-08 10:24 ` [PATCH mlx5-next 2/9] net/mlx5: Add support for flow table destination number Leon Romanovsky
2018-07-10 22:26   ` Saeed Mahameed
2018-07-11  9:47     ` Yishai Hadas
2018-07-08 10:24 ` [PATCH rdma-next 3/9] IB: Enable uverbs_destroy_def_handler to be used by drivers Leon Romanovsky
2018-07-08 10:24 ` [PATCH rdma-next 4/9] IB/mlx5: Introduce flow steering matcher object Leon Romanovsky
2018-07-10 17:34   ` Jason Gunthorpe
2018-07-11  9:32     ` Yishai Hadas
2018-07-11  9:32       ` Yishai Hadas
2018-07-11 12:02       ` Jason Gunthorpe
2018-07-08 10:24 ` [PATCH rdma-next 5/9] IB: Consider ib_flow creation by the KABI infrastructure Leon Romanovsky
2018-07-08 10:24 ` [PATCH rdma-next 6/9] IB/mlx5: Introduce vendor create and destroy flow methods Leon Romanovsky
2018-07-10 17:44   ` Jason Gunthorpe
2018-07-11  9:44     ` Yishai Hadas
2018-07-11  9:44       ` Yishai Hadas
2018-07-08 10:24 ` [PATCH rdma-next 7/9] IB/mlx5: Support adding flow steering rule by raw data Leon Romanovsky
2018-07-08 10:24 ` [PATCH rdma-next 8/9] IB/mlx5: Add support for a flow table destination Leon Romanovsky
2018-07-08 10:24 ` [PATCH rdma-next 9/9] IB/mlx5: Expose vendor flow trees Leon Romanovsky
2018-07-10 18:28 ` [PATCH rdma-next 0/9] Support mlx5 flow steering with RAW data Jason Gunthorpe
2018-07-11  5:43   ` Leon Romanovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.