* [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).