| Message ID | 1538648247-21003-5-git-send-email-yishaih@mellanox.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | mlx5: Extend flow steering support | expand |
On Thu, Oct 04, 2018 at 01:17:26PM +0300, Yishai Hadas wrote: > From: Mark Bloch <markb@mellanox.com> > > Use flags defined under enum ibv_flow_flags, this will allow creating > egress matcher. > > Signed-off-by: Mark Bloch <markb@mellanox.com> > Signed-off-by: Yishai Hadas <yishaih@mellanox.com> > providers/mlx5/verbs.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c > index de731ce..41060bc 100644 > +++ b/providers/mlx5/verbs.c > @@ -3665,7 +3665,7 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, > { > DECLARE_COMMAND_BUFFER(cmd, MLX5_IB_OBJECT_FLOW_MATCHER, > MLX5_IB_METHOD_FLOW_MATCHER_CREATE, > - 4); > + 5); > struct mlx5dv_flow_matcher *flow_matcher; > struct ib_uverbs_attr *handle; > int ret; > @@ -3695,6 +3695,9 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, > fill_attr_in_enum(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE, > IBV_FLOW_ATTR_NORMAL, &attr->priority, > sizeof(attr->priority)); > + if (attr->flags) > + fill_attr_const_in(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_FLAGS, > + attr->flags); Uhh.. This is 'enum ib_flow_flags' in the kernel, and it is not in a uapi header. This was missed when the kernel patches were accepted. Please send a kernel patch to fix it.. Jason
On 10/4/2018 6:27 PM, Jason Gunthorpe wrote: > On Thu, Oct 04, 2018 at 01:17:26PM +0300, Yishai Hadas wrote: >> From: Mark Bloch <markb@mellanox.com> >> >> Use flags defined under enum ibv_flow_flags, this will allow creating >> egress matcher. >> >> Signed-off-by: Mark Bloch <markb@mellanox.com> >> Signed-off-by: Yishai Hadas <yishaih@mellanox.com> >> providers/mlx5/verbs.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c >> index de731ce..41060bc 100644 >> +++ b/providers/mlx5/verbs.c >> @@ -3665,7 +3665,7 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, >> { >> DECLARE_COMMAND_BUFFER(cmd, MLX5_IB_OBJECT_FLOW_MATCHER, >> MLX5_IB_METHOD_FLOW_MATCHER_CREATE, >> - 4); >> + 5); >> struct mlx5dv_flow_matcher *flow_matcher; >> struct ib_uverbs_attr *handle; >> int ret; >> @@ -3695,6 +3695,9 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, >> fill_attr_in_enum(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE, >> IBV_FLOW_ATTR_NORMAL, &attr->priority, >> sizeof(attr->priority)); >> + if (attr->flags) >> + fill_attr_const_in(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_FLAGS, >> + attr->flags); > > Uhh.. > > This is 'enum ib_flow_flags' in the kernel, and it is not in a uapi > header. This was missed when the kernel patches were accepted. > This enum is defined in verb.h and its values are exposed to application long time ago from there, how can that be changed ? > Please send a kernel patch to fix it.. No change was done for that enum as part of this series, it's some legacy enum in both kernel and user which is used as input as done also for ibv_create_flow().
On Thu, Oct 04, 2018 at 07:00:30PM +0300, Yishai Hadas wrote: > On 10/4/2018 6:27 PM, Jason Gunthorpe wrote: > > On Thu, Oct 04, 2018 at 01:17:26PM +0300, Yishai Hadas wrote: > > > From: Mark Bloch <markb@mellanox.com> > > > > > > Use flags defined under enum ibv_flow_flags, this will allow creating > > > egress matcher. > > > > > > Signed-off-by: Mark Bloch <markb@mellanox.com> > > > Signed-off-by: Yishai Hadas <yishaih@mellanox.com> > > > providers/mlx5/verbs.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c > > > index de731ce..41060bc 100644 > > > +++ b/providers/mlx5/verbs.c > > > @@ -3665,7 +3665,7 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, > > > { > > > DECLARE_COMMAND_BUFFER(cmd, MLX5_IB_OBJECT_FLOW_MATCHER, > > > MLX5_IB_METHOD_FLOW_MATCHER_CREATE, > > > - 4); > > > + 5); > > > struct mlx5dv_flow_matcher *flow_matcher; > > > struct ib_uverbs_attr *handle; > > > int ret; > > > @@ -3695,6 +3695,9 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, > > > fill_attr_in_enum(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE, > > > IBV_FLOW_ATTR_NORMAL, &attr->priority, > > > sizeof(attr->priority)); > > > + if (attr->flags) > > > + fill_attr_const_in(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_FLAGS, > > > + attr->flags); > > > > Uhh.. > > > > This is 'enum ib_flow_flags' in the kernel, and it is not in a uapi > > header. This was missed when the kernel patches were accepted. > > > > This enum is defined in verb.h and its values are exposed to application > long time ago from there, how can that be changed ? It can be changed to use defines from libibverbs/verbs_api.h as other enums are doing now. > > Please send a kernel patch to fix it.. > > No change was done for that enum as part of this series, it's some legacy > enum in both kernel and user which is used as input as done also for > ibv_create_flow(). When the attribute as added it should have been checked if the refered enum is in a upai header. Please move the enum to the uapi header since it is part of the uapi Jason
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c index de731ce..41060bc 100644 --- a/providers/mlx5/verbs.c +++ b/providers/mlx5/verbs.c @@ -3665,7 +3665,7 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, { DECLARE_COMMAND_BUFFER(cmd, MLX5_IB_OBJECT_FLOW_MATCHER, MLX5_IB_METHOD_FLOW_MATCHER_CREATE, - 4); + 5); struct mlx5dv_flow_matcher *flow_matcher; struct ib_uverbs_attr *handle; int ret; @@ -3695,6 +3695,9 @@ mlx5dv_create_flow_matcher(struct ibv_context *context, fill_attr_in_enum(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_TYPE, IBV_FLOW_ATTR_NORMAL, &attr->priority, sizeof(attr->priority)); + if (attr->flags) + fill_attr_const_in(cmd, MLX5_IB_ATTR_FLOW_MATCHER_FLOW_FLAGS, + attr->flags); ret = execute_ioctl(context, cmd); if (ret)