Hi Mark, > From: Mark Bloch <markb@xxxxxxxxxxxx> > Sent: Tuesday, October 29, 2019 19:01 > > On 10/29/19 8:50 AM, Leon Romanovsky wrote: > > From: Yevgeny Kliteynik <kliteyn@xxxxxxxxxxxx> > > > > Add support for flow steering counters action with a non-base counter > > ID (offset) for bulk counters. > > > > When creating a flow counter object, save the bulk value. > > This value is used when a flow action with a non-base counter ID is > > requested - to validate that the required offset is in the range of > > the allocated bulk. > > > > Signed-off-by: Yevgeny Kliteynik <kliteyn@xxxxxxxxxxxx> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > --- > > Changelog: > > v0 -> v1: > > https://lore.kernel.org/linux-rdma/20191029055916.7322-1-leon@kernel.o > > rg > > * Change ffs to multiply bitfmap > > * Changed uint32_t to be u32 > > * Added offset to mlx5_ib_devx_is_flow_counter() > > --- > > drivers/infiniband/hw/mlx5/devx.c | 15 +++++++++++- > > drivers/infiniband/hw/mlx5/flow.c | 30 +++++++++++++++++++++--- > > drivers/infiniband/hw/mlx5/mlx5_ib.h | 2 +- > > include/uapi/rdma/mlx5_user_ioctl_cmds.h | 1 + > > 4 files changed, 43 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/infiniband/hw/mlx5/devx.c > > b/drivers/infiniband/hw/mlx5/devx.c > > index 6b1fca91d7d3..ab7d201c91c9 100644 > > --- a/drivers/infiniband/hw/mlx5/devx.c > > +++ b/drivers/infiniband/hw/mlx5/devx.c > > @@ -100,6 +100,7 @@ struct devx_obj { > > struct mlx5_ib_devx_mr devx_mr; > > struct mlx5_core_dct core_dct; > > struct mlx5_core_cq core_cq; > > + u32 flow_counter_bulk_size; > > }; > > struct list_head event_sub; /* holds devx_event_subscription entries > > */ }; @@ -192,15 +193,20 @@ bool mlx5_ib_devx_is_flow_dest(void *obj, > > int *dest_id, int *dest_type) > > } > > } > > > > -bool mlx5_ib_devx_is_flow_counter(void *obj, u32 *counter_id) > > +bool mlx5_ib_devx_is_flow_counter(void *obj, u32 offset, u32 > > +*counter_id) > > { > > struct devx_obj *devx_obj = obj; > > u16 opcode = MLX5_GET(general_obj_in_cmd_hdr, devx_obj->dinbox, > > opcode); > > > > if (opcode == MLX5_CMD_OP_DEALLOC_FLOW_COUNTER) { > > + > > + if (offset && offset >= devx_obj->flow_counter_bulk_size) > > + return false; > > + > > *counter_id = MLX5_GET(dealloc_flow_counter_in, > > devx_obj->dinbox, > > flow_counter_id); > > + *counter_id += offset; > > return true; > > } > > > > @@ -1463,6 +1469,13 @@ static int > UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)( > > if (err) > > goto obj_free; > > > > + if (opcode == MLX5_CMD_OP_ALLOC_FLOW_COUNTER) { > > + u8 bulk = MLX5_GET(alloc_flow_counter_in, > > + cmd_in, > > + flow_counter_bulk); > > + obj->flow_counter_bulk_size = 128 * bulk; > > + } > > + > > uobj->object = obj; > > INIT_LIST_HEAD(&obj->event_sub); > > obj->ib_dev = dev; > > diff --git a/drivers/infiniband/hw/mlx5/flow.c > > b/drivers/infiniband/hw/mlx5/flow.c > > index b198ff10cde9..985be0927918 100644 > > --- a/drivers/infiniband/hw/mlx5/flow.c > > +++ b/drivers/infiniband/hw/mlx5/flow.c > > @@ -85,6 +85,7 @@ static int > UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)( > > struct mlx5_ib_dev *dev = mlx5_udata_to_mdev(&attrs->driver_udata); > > int len, ret, i; > > u32 counter_id = 0; > > + u32 *offset; > > > > if (!capable(CAP_NET_RAW)) > > return -EPERM; > > @@ -151,8 +152,27 @@ static int > UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)( > > if (len) { > > devx_obj = arr_flow_actions[0]->object; > > > > - if (!mlx5_ib_devx_is_flow_counter(devx_obj, &counter_id)) > > - return -EINVAL; > > + if (uverbs_attr_is_valid( > > + attrs, > > + > MLX5_IB_ATTR_CREATE_FLOW_ARR_COUNTERS_DEVX_OFFSET)) { > > + int num_offsets = uverbs_attr_ptr_get_array_size( > > + attrs, > > + > MLX5_IB_ATTR_CREATE_FLOW_ARR_COUNTERS_DEVX_OFFSET, > > + sizeof(u32)); > > + > > + if (num_offsets != 1) > > + return -EINVAL; > > + > > + offset = uverbs_attr_get_alloced_ptr( > > + attrs, > > + > MLX5_IB_ATTR_CREATE_FLOW_ARR_COUNTERS_DEVX_OFFSET); > > + > > + if (!mlx5_ib_devx_is_flow_counter(devx_obj, > > + *offset, > > + &counter_id)) > > + return -EINVAL; > > + } > > Have you tested this patch without passing offset? because now the > mlx5_ib_devx_is_flow_counter() is done only if > MLX5_IB_ATTR_CREATE_FLOW_ARR_COUNTERS_DEVX_OFFSET > is passed. Indeed. Thanks, v2 is on its way... -- YK > Mark >