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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxx>

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@xxxxxxxxxxxx>
Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
  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
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux