netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH rdma-next v1 0/4] Add steering support for default miss
@ 2020-05-04  5:30 Leon Romanovsky
  2020-05-04  5:30 ` [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code Leon Romanovsky
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Leon Romanovsky @ 2020-05-04  5:30 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, linux-rdma, Maor Gottlieb, Mark Bloch,
	Mark Zhang, netdev, Saeed Mahameed

From: Leon Romanovsky <leonro@mellanox.com>

Changelog
v1:
 * Rebased on latest rdma-next
 * Removed attr_is_valid() check from flags
v0: https://lore.kernel.org/linux-rdma/20200413135220.934007-1-leon@kernel.org

-------------------------------------------------------------------------
Hi,

This code from Naor refactors the fs_core and adds steering support
for default miss functionality.

Thanks

Maor Gottlieb (4):
  {IB/net}/mlx5: Simplify don't trap code
  net/mlx5: Add support in forward to namespace
  RDMA/mlx5: Refactor DV create flow
  RDMA/mlx5: Add support in steering default miss

 drivers/infiniband/hw/mlx5/flow.c             | 136 +++++++++++-------
 drivers/infiniband/hw/mlx5/main.c             |  55 ++-----
 .../net/ethernet/mellanox/mlx5/core/fs_core.c | 123 +++++++++++-----
 .../net/ethernet/mellanox/mlx5/core/fs_core.h |   2 +
 include/linux/mlx5/fs.h                       |   1 +
 include/uapi/rdma/mlx5_user_ioctl_cmds.h      |   5 +
 6 files changed, 192 insertions(+), 130 deletions(-)

--
2.26.2


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

* [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code
  2020-05-04  5:30 [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
@ 2020-05-04  5:30 ` Leon Romanovsky
  2020-05-08 19:58   ` Jason Gunthorpe
  2020-05-04  5:30 ` [PATCH mlx5-next v1 2/4] net/mlx5: Add support in forward to namespace Leon Romanovsky
  2020-05-13  7:05 ` [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
  2 siblings, 1 reply; 9+ messages in thread
From: Leon Romanovsky @ 2020-05-04  5:30 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Maor Gottlieb, linux-rdma, Mark Bloch, Mark Zhang, netdev,
	Saeed Mahameed

From: Maor Gottlieb <maorg@mellanox.com>

The fs_core already supports creation of rules with multiple
actions/destinations. Refactor fs_core to handle the case
when don't trap rule is created with destination. Adapt the
calling code in the driver.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Reviewed-by: Mark Zhang <markz@mellanox.com>
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c             | 46 +++--------
 .../net/ethernet/mellanox/mlx5/core/fs_core.c | 79 +++++++++++--------
 2 files changed, 55 insertions(+), 70 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 0b8cc219e085..5a366f286043 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -3699,12 +3699,13 @@ static struct mlx5_ib_flow_handler *_create_flow_rule(struct mlx5_ib_dev *dev,
 		if (!dest_num)
 			rule_dst = NULL;
 	} else {
+		if (flow_attr->flags & IB_FLOW_ATTR_FLAGS_DONT_TRAP)
+			flow_act.action |=
+				MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
 		if (is_egress)
 			flow_act.action |= MLX5_FLOW_CONTEXT_ACTION_ALLOW;
-		else
-			flow_act.action |=
-				dest_num ?  MLX5_FLOW_CONTEXT_ACTION_FWD_DEST :
-					MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
+		else if (dest_num)
+			flow_act.action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
 	}
 
 	if ((spec->flow_context.flags & FLOW_CONTEXT_HAS_TAG)  &&
@@ -3748,30 +3749,6 @@ static struct mlx5_ib_flow_handler *create_flow_rule(struct mlx5_ib_dev *dev,
 	return _create_flow_rule(dev, ft_prio, flow_attr, dst, 0, NULL);
 }
 
-static struct mlx5_ib_flow_handler *create_dont_trap_rule(struct mlx5_ib_dev *dev,
-							  struct mlx5_ib_flow_prio *ft_prio,
-							  struct ib_flow_attr *flow_attr,
-							  struct mlx5_flow_destination *dst)
-{
-	struct mlx5_ib_flow_handler *handler_dst = NULL;
-	struct mlx5_ib_flow_handler *handler = NULL;
-
-	handler = create_flow_rule(dev, ft_prio, flow_attr, NULL);
-	if (!IS_ERR(handler)) {
-		handler_dst = create_flow_rule(dev, ft_prio,
-					       flow_attr, dst);
-		if (IS_ERR(handler_dst)) {
-			mlx5_del_flow_rules(handler->rule);
-			ft_prio->refcount--;
-			kfree(handler);
-			handler = handler_dst;
-		} else {
-			list_add(&handler_dst->list, &handler->list);
-		}
-	}
-
-	return handler;
-}
 enum {
 	LEFTOVERS_MC,
 	LEFTOVERS_UC,
@@ -3975,15 +3952,10 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
 	}
 
 	if (flow_attr->type == IB_FLOW_ATTR_NORMAL) {
-		if (flow_attr->flags & IB_FLOW_ATTR_FLAGS_DONT_TRAP)  {
-			handler = create_dont_trap_rule(dev, ft_prio,
-							flow_attr, dst);
-		} else {
-			underlay_qpn = (mqp->flags & IB_QP_CREATE_SOURCE_QPN) ?
-					mqp->underlay_qpn : 0;
-			handler = _create_flow_rule(dev, ft_prio, flow_attr,
-						    dst, underlay_qpn, ucmd);
-		}
+		underlay_qpn = (mqp->flags & IB_QP_CREATE_SOURCE_QPN) ?
+			mqp->underlay_qpn : 0;
+		handler = _create_flow_rule(dev, ft_prio, flow_attr,
+					    dst, underlay_qpn, ucmd);
 	} else if (flow_attr->type == IB_FLOW_ATTR_ALL_DEFAULT ||
 		   flow_attr->type == IB_FLOW_ATTR_MC_DEFAULT) {
 		handler = create_leftovers_rule(dev, ft_prio, flow_attr,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index d5defe09339a..9afe942f7aa2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -254,7 +254,7 @@ static void del_sw_flow_group(struct fs_node *node);
 static void del_sw_fte(struct fs_node *node);
 static void del_sw_prio(struct fs_node *node);
 static void del_sw_ns(struct fs_node *node);
-/* Delete rule (destination) is special case that 
+/* Delete rule (destination) is special case that
  * requires to lock the FTE for all the deletion process.
  */
 static void del_sw_hw_rule(struct fs_node *node);
@@ -1899,48 +1899,61 @@ mlx5_add_flow_rules(struct mlx5_flow_table *ft,
 {
 	struct mlx5_flow_root_namespace *root = find_root(&ft->node);
 	static const struct mlx5_flow_spec zero_spec = {};
-	struct mlx5_flow_destination gen_dest = {};
+	struct mlx5_flow_destination *gen_dest = NULL;
 	struct mlx5_flow_table *next_ft = NULL;
 	struct mlx5_flow_handle *handle = NULL;
 	u32 sw_action = flow_act->action;
 	struct fs_prio *prio;
+	int i;
 
 	if (!spec)
 		spec = &zero_spec;
 
-	fs_get_obj(prio, ft->node.parent);
-	if (flow_act->action == MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO) {
-		if (!fwd_next_prio_supported(ft))
-			return ERR_PTR(-EOPNOTSUPP);
-		if (num_dest)
-			return ERR_PTR(-EINVAL);
-		mutex_lock(&root->chain_lock);
-		next_ft = find_next_chained_ft(prio);
-		if (next_ft) {
-			gen_dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
-			gen_dest.ft = next_ft;
-			dest = &gen_dest;
-			num_dest = 1;
-			flow_act->action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
-		} else {
-			mutex_unlock(&root->chain_lock);
-			return ERR_PTR(-EOPNOTSUPP);
-		}
-	}
+	if (!(sw_action & MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO))
+		return _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
 
-	handle = _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
+	if (!fwd_next_prio_supported(ft))
+		return ERR_PTR(-EOPNOTSUPP);
 
-	if (sw_action == MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO) {
-		if (!IS_ERR_OR_NULL(handle) &&
-		    (list_empty(&handle->rule[0]->next_ft))) {
-			mutex_lock(&next_ft->lock);
-			list_add(&handle->rule[0]->next_ft,
-				 &next_ft->fwd_rules);
-			mutex_unlock(&next_ft->lock);
-			handle->rule[0]->sw_action = MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
-		}
-		mutex_unlock(&root->chain_lock);
-	}
+	mutex_lock(&root->chain_lock);
+	fs_get_obj(prio, ft->node.parent);
+	next_ft = find_next_chained_ft(prio);
+	if (!next_ft) {
+		handle = ERR_PTR(-EOPNOTSUPP);
+		goto unlock;
+	}
+
+	gen_dest = kcalloc(num_dest + 1, sizeof(*dest),
+			   GFP_KERNEL);
+	if (!gen_dest) {
+		handle = ERR_PTR(-ENOMEM);
+		goto unlock;
+	}
+	for (i = 0; i < num_dest; i++)
+		gen_dest[i] = dest[i];
+	gen_dest[i].type =
+		MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+	gen_dest[i].ft = next_ft;
+	dest = gen_dest;
+	num_dest++;
+	flow_act->action &=
+		~MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
+	flow_act->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+	handle = _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
+	if (IS_ERR_OR_NULL(handle))
+		goto unlock;
+
+	if (list_empty(&handle->rule[num_dest - 1]->next_ft)) {
+		mutex_lock(&next_ft->lock);
+		list_add(&handle->rule[num_dest - 1]->next_ft,
+			 &next_ft->fwd_rules);
+		mutex_unlock(&next_ft->lock);
+		handle->rule[num_dest - 1]->sw_action =
+			MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
+	}
+unlock:
+	mutex_unlock(&root->chain_lock);
+	kfree(gen_dest);
 	return handle;
 }
 EXPORT_SYMBOL(mlx5_add_flow_rules);
-- 
2.26.2


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

* [PATCH mlx5-next v1 2/4] net/mlx5: Add support in forward to namespace
  2020-05-04  5:30 [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
  2020-05-04  5:30 ` [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code Leon Romanovsky
@ 2020-05-04  5:30 ` Leon Romanovsky
  2020-05-13  7:05 ` [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
  2 siblings, 0 replies; 9+ messages in thread
From: Leon Romanovsky @ 2020-05-04  5:30 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Maor Gottlieb, linux-rdma, Mark Bloch, Mark Zhang, netdev,
	Saeed Mahameed

From: Maor Gottlieb <maorg@mellanox.com>

Currently, fs_core supports rule of forward the traffic
to continue matching in the next priority, now we add support
to forward the traffic matching in the next namespace.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Reviewed-by: Mark Bloch <markb@mellanox.com>
Reviewed-by: Mark Zhang <markz@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 .../net/ethernet/mellanox/mlx5/core/fs_core.c | 56 ++++++++++++++++---
 .../net/ethernet/mellanox/mlx5/core/fs_core.h |  2 +
 include/linux/mlx5/fs.h                       |  1 +
 3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index 9afe942f7aa2..b297bdbeaf50 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -384,6 +384,12 @@ static struct fs_prio *find_prio(struct mlx5_flow_namespace *ns,
 	return NULL;
 }
 
+static bool is_fwd_next_action(u32 action)
+{
+	return action & (MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO |
+			 MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_NS);
+}
+
 static bool check_valid_spec(const struct mlx5_flow_spec *spec)
 {
 	int i;
@@ -502,7 +508,7 @@ static void del_sw_hw_rule(struct fs_node *node)
 	fs_get_obj(rule, node);
 	fs_get_obj(fte, rule->node.parent);
 	trace_mlx5_fs_del_rule(rule);
-	if (rule->sw_action == MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO) {
+	if (is_fwd_next_action(rule->sw_action)) {
 		mutex_lock(&rule->dest_attr.ft->lock);
 		list_del(&rule->next_ft);
 		mutex_unlock(&rule->dest_attr.ft->lock);
@@ -826,6 +832,36 @@ static struct mlx5_flow_table *find_prev_chained_ft(struct fs_prio *prio)
 	return find_closest_ft(prio, true);
 }
 
+static struct fs_prio *find_fwd_ns_prio(struct mlx5_flow_root_namespace *root,
+					struct mlx5_flow_namespace *ns)
+{
+	struct mlx5_flow_namespace *root_ns = &root->ns;
+	struct fs_prio *iter_prio;
+	struct fs_prio *prio;
+
+	fs_get_obj(prio, ns->node.parent);
+	list_for_each_entry(iter_prio, &root_ns->node.children, node.list) {
+		if (iter_prio == prio &&
+		    !list_is_last(&prio->node.children, &iter_prio->node.list))
+			return list_next_entry(iter_prio, node.list);
+	}
+	return NULL;
+}
+
+static struct mlx5_flow_table *find_next_fwd_ft(struct mlx5_flow_table *ft,
+						struct mlx5_flow_act *flow_act)
+{
+	struct mlx5_flow_root_namespace *root = find_root(&ft->node);
+	struct fs_prio *prio;
+
+	if (flow_act->action & MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_NS)
+		prio = find_fwd_ns_prio(root, ft->ns);
+	else
+		fs_get_obj(prio, ft->node.parent);
+
+	return (prio) ? find_next_chained_ft(prio) : NULL;
+}
+
 static int connect_fts_in_prio(struct mlx5_core_dev *dev,
 			       struct fs_prio *prio,
 			       struct mlx5_flow_table *ft)
@@ -976,6 +1012,10 @@ static int connect_fwd_rules(struct mlx5_core_dev *dev,
 	list_splice_init(&old_next_ft->fwd_rules, &new_next_ft->fwd_rules);
 	mutex_unlock(&old_next_ft->lock);
 	list_for_each_entry(iter, &new_next_ft->fwd_rules, next_ft) {
+		if ((iter->sw_action & MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_NS) &&
+		    iter->ft->ns == new_next_ft->ns)
+			continue;
+
 		err = _mlx5_modify_rule_destination(iter, &dest);
 		if (err)
 			pr_err("mlx5_core: failed to modify rule to point on flow table %d\n",
@@ -1077,6 +1117,7 @@ static struct mlx5_flow_table *__mlx5_create_flow_table(struct mlx5_flow_namespa
 	next_ft = unmanaged ? ft_attr->next_ft :
 			      find_next_chained_ft(fs_prio);
 	ft->def_miss_action = ns->def_miss_action;
+	ft->ns = ns;
 	err = root->cmds->create_flow_table(root, ft, log_table_sz, next_ft);
 	if (err)
 		goto free_ft;
@@ -1903,21 +1944,19 @@ mlx5_add_flow_rules(struct mlx5_flow_table *ft,
 	struct mlx5_flow_table *next_ft = NULL;
 	struct mlx5_flow_handle *handle = NULL;
 	u32 sw_action = flow_act->action;
-	struct fs_prio *prio;
 	int i;
 
 	if (!spec)
 		spec = &zero_spec;
 
-	if (!(sw_action & MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO))
+	if (!is_fwd_next_action(sw_action))
 		return _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
 
 	if (!fwd_next_prio_supported(ft))
 		return ERR_PTR(-EOPNOTSUPP);
 
 	mutex_lock(&root->chain_lock);
-	fs_get_obj(prio, ft->node.parent);
-	next_ft = find_next_chained_ft(prio);
+	next_ft = find_next_fwd_ft(ft, flow_act);
 	if (!next_ft) {
 		handle = ERR_PTR(-EOPNOTSUPP);
 		goto unlock;
@@ -1937,7 +1976,8 @@ mlx5_add_flow_rules(struct mlx5_flow_table *ft,
 	dest = gen_dest;
 	num_dest++;
 	flow_act->action &=
-		~MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
+		~(MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO |
+		  MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_NS);
 	flow_act->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
 	handle = _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
 	if (IS_ERR_OR_NULL(handle))
@@ -1948,8 +1988,8 @@ mlx5_add_flow_rules(struct mlx5_flow_table *ft,
 		list_add(&handle->rule[num_dest - 1]->next_ft,
 			 &next_ft->fwd_rules);
 		mutex_unlock(&next_ft->lock);
-		handle->rule[num_dest - 1]->sw_action =
-			MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
+		handle->rule[num_dest - 1]->sw_action = sw_action;
+		handle->rule[num_dest - 1]->ft = ft;
 	}
 unlock:
 	mutex_unlock(&root->chain_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h
index 508108c58dae..825b662f809b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.h
@@ -138,6 +138,7 @@ struct fs_node {
 
 struct mlx5_flow_rule {
 	struct fs_node				node;
+	struct mlx5_flow_table			*ft;
 	struct mlx5_flow_destination		dest_attr;
 	/* next_ft should be accessed under chain_lock and only of
 	 * destination type is FWD_NEXT_fT.
@@ -175,6 +176,7 @@ struct mlx5_flow_table {
 	u32				flags;
 	struct rhltable			fgs_hash;
 	enum mlx5_flow_table_miss_action def_miss_action;
+	struct mlx5_flow_namespace	*ns;
 };
 
 struct mlx5_ft_underlay_qp {
diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
index e2d13e074067..6c5aa0a21425 100644
--- a/include/linux/mlx5/fs.h
+++ b/include/linux/mlx5/fs.h
@@ -42,6 +42,7 @@ enum {
 	MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO	= 1 << 16,
 	MLX5_FLOW_CONTEXT_ACTION_ENCRYPT	= 1 << 17,
 	MLX5_FLOW_CONTEXT_ACTION_DECRYPT	= 1 << 18,
+	MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_NS	= 1 << 19,
 };
 
 enum {
-- 
2.26.2


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

* Re: [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code
  2020-05-04  5:30 ` [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code Leon Romanovsky
@ 2020-05-08 19:58   ` Jason Gunthorpe
  2020-05-08 20:29     ` Mark Bloch
  2020-05-10  8:17     ` Leon Romanovsky
  0 siblings, 2 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2020-05-08 19:58 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Bloch, Mark Zhang,
	netdev, Saeed Mahameed

On Mon, May 04, 2020 at 08:30:09AM +0300, Leon Romanovsky wrote:
> +	flow_act->action &=
> +		~MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
> +	flow_act->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> +	handle = _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
> +	if (IS_ERR_OR_NULL(handle))
> +		goto unlock;

I never like seeing IS_ERR_OR_NULL()..

In this case I see callers of mlx5_add_flow_rules() that crash if it
returns NULL, so this can't be right.

Also, I don't see an obvious place where _mlx5_add_flow_rules()
returns NULL, does it?

Jason

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

* Re: [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code
  2020-05-08 19:58   ` Jason Gunthorpe
@ 2020-05-08 20:29     ` Mark Bloch
  2020-05-10  8:17     ` Leon Romanovsky
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Bloch @ 2020-05-08 20:29 UTC (permalink / raw)
  To: Jason Gunthorpe, Leon Romanovsky
  Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Zhang, netdev,
	Saeed Mahameed



On 5/8/2020 12:58, Jason Gunthorpe wrote:
> On Mon, May 04, 2020 at 08:30:09AM +0300, Leon Romanovsky wrote:
>> +	flow_act->action &=
>> +		~MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
>> +	flow_act->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
>> +	handle = _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
>> +	if (IS_ERR_OR_NULL(handle))
>> +		goto unlock;
> 
> I never like seeing IS_ERR_OR_NULL()..
> 
> In this case I see callers of mlx5_add_flow_rules() that crash if it
> returns NULL, so this can't be right.
> 
> Also, I don't see an obvious place where _mlx5_add_flow_rules()
> returns NULL, does it?

It seems you are right. b3638e1a76648 ("net/mlx5_core: Introduce forward to next priority action")
added that code and it seems from the start it was wrong.

Looking at the code it looks like we always use IS_ERR() to check the result
of mlx5_add_flow_rules() except in: mlx5e_tc_add_nic_flow() which should also
be fixed.

Thanks Jason.

> 
> Jason
> 

Mark

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

* Re: [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code
  2020-05-08 19:58   ` Jason Gunthorpe
  2020-05-08 20:29     ` Mark Bloch
@ 2020-05-10  8:17     ` Leon Romanovsky
  2020-05-12 20:14       ` Jason Gunthorpe
  1 sibling, 1 reply; 9+ messages in thread
From: Leon Romanovsky @ 2020-05-10  8:17 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Bloch, Mark Zhang,
	netdev, Saeed Mahameed

On Fri, May 08, 2020 at 04:58:38PM -0300, Jason Gunthorpe wrote:
> On Mon, May 04, 2020 at 08:30:09AM +0300, Leon Romanovsky wrote:
> > +	flow_act->action &=
> > +		~MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
> > +	flow_act->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> > +	handle = _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
> > +	if (IS_ERR_OR_NULL(handle))
> > +		goto unlock;
>
> I never like seeing IS_ERR_OR_NULL()..
>
> In this case I see callers of mlx5_add_flow_rules() that crash if it
> returns NULL, so this can't be right.
>
> Also, I don't see an obvious place where _mlx5_add_flow_rules()
> returns NULL, does it?

You are right, I'll replace this IS_ERR_OR_NULL() to be IS_ERR() once
will take it to mlx5-next.

Is it ok?

Thanks

>
> Jason

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

* Re: [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code
  2020-05-10  8:17     ` Leon Romanovsky
@ 2020-05-12 20:14       ` Jason Gunthorpe
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2020-05-12 20:14 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Bloch, Mark Zhang,
	netdev, Saeed Mahameed

On Sun, May 10, 2020 at 11:17:14AM +0300, Leon Romanovsky wrote:
> On Fri, May 08, 2020 at 04:58:38PM -0300, Jason Gunthorpe wrote:
> > On Mon, May 04, 2020 at 08:30:09AM +0300, Leon Romanovsky wrote:
> > > +	flow_act->action &=
> > > +		~MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO;
> > > +	flow_act->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> > > +	handle = _mlx5_add_flow_rules(ft, spec, flow_act, dest, num_dest);
> > > +	if (IS_ERR_OR_NULL(handle))
> > > +		goto unlock;
> >
> > I never like seeing IS_ERR_OR_NULL()..
> >
> > In this case I see callers of mlx5_add_flow_rules() that crash if it
> > returns NULL, so this can't be right.
> >
> > Also, I don't see an obvious place where _mlx5_add_flow_rules()
> > returns NULL, does it?
> 
> You are right, I'll replace this IS_ERR_OR_NULL() to be IS_ERR() once
> will take it to mlx5-next.
> 
> Is it ok?

Okay, looks fine, let me know the shared branch commit

Jason

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

* Re: [PATCH rdma-next v1 0/4] Add steering support for default miss
  2020-05-04  5:30 [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
  2020-05-04  5:30 ` [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code Leon Romanovsky
  2020-05-04  5:30 ` [PATCH mlx5-next v1 2/4] net/mlx5: Add support in forward to namespace Leon Romanovsky
@ 2020-05-13  7:05 ` Leon Romanovsky
  2020-05-13 18:57   ` Jason Gunthorpe
  2 siblings, 1 reply; 9+ messages in thread
From: Leon Romanovsky @ 2020-05-13  7:05 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: linux-rdma, Maor Gottlieb, Mark Bloch, Mark Zhang, netdev,
	Saeed Mahameed

On Mon, May 04, 2020 at 08:30:08AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog
> v1:
>  * Rebased on latest rdma-next
>  * Removed attr_is_valid() check from flags
> v0: https://lore.kernel.org/linux-rdma/20200413135220.934007-1-leon@kernel.org
>
> -------------------------------------------------------------------------
> Hi,
>
> This code from Naor refactors the fs_core and adds steering support
> for default miss functionality.
>
> Thanks
>
> Maor Gottlieb (4):
>   {IB/net}/mlx5: Simplify don't trap code
>   net/mlx5: Add support in forward to namespace

Applied to mlx5-next with change of IS_ERR_OR_NULL().

19386660212d net/mlx5: Add support in forward to namespace
8e14c75c999a {IB/net}/mlx5: Simplify don't trap code

Thanks

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

* Re: [PATCH rdma-next v1 0/4] Add steering support for default miss
  2020-05-13  7:05 ` [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
@ 2020-05-13 18:57   ` Jason Gunthorpe
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2020-05-13 18:57 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, linux-rdma, Maor Gottlieb, Mark Bloch, Mark Zhang,
	netdev, Saeed Mahameed

On Wed, May 13, 2020 at 10:05:59AM +0300, Leon Romanovsky wrote:
> On Mon, May 04, 2020 at 08:30:08AM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Changelog
> > v1:
> >  * Rebased on latest rdma-next
> >  * Removed attr_is_valid() check from flags
> > v0: https://lore.kernel.org/linux-rdma/20200413135220.934007-1-leon@kernel.org
> >
> > Hi,
> >
> > This code from Naor refactors the fs_core and adds steering support
> > for default miss functionality.
> >
> > Thanks
> >
> > Maor Gottlieb (4):
> >   {IB/net}/mlx5: Simplify don't trap code
> >   net/mlx5: Add support in forward to namespace
> 
> Applied to mlx5-next with change of IS_ERR_OR_NULL().
> 
> 19386660212d net/mlx5: Add support in forward to namespace
> 8e14c75c999a {IB/net}/mlx5: Simplify don't trap code

Ok applied to for-next

Jason

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

end of thread, other threads:[~2020-05-13 18:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04  5:30 [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
2020-05-04  5:30 ` [PATCH mlx5-next v1 1/4] {IB/net}/mlx5: Simplify don't trap code Leon Romanovsky
2020-05-08 19:58   ` Jason Gunthorpe
2020-05-08 20:29     ` Mark Bloch
2020-05-10  8:17     ` Leon Romanovsky
2020-05-12 20:14       ` Jason Gunthorpe
2020-05-04  5:30 ` [PATCH mlx5-next v1 2/4] net/mlx5: Add support in forward to namespace Leon Romanovsky
2020-05-13  7:05 ` [PATCH rdma-next v1 0/4] Add steering support for default miss Leon Romanovsky
2020-05-13 18:57   ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).