On Thu, May 31, 2018 at 02:49:44PM +0000, Ruhl, Michael J wrote: > >-----Original Message----- > >From: Leon Romanovsky [mailto:leon@xxxxxxxxxx] > >Sent: Thursday, May 31, 2018 9:44 AM > >To: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe > ><jgg@xxxxxxxxxxxx> > >Cc: Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux- > >rdma@xxxxxxxxxxxxxxx>; Boris Pismenny <borisp@xxxxxxxxxxxx>; Matan > >Barak <matanb@xxxxxxxxxxxx>; Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; > >Or Gerlitz <ogerlitz@xxxxxxxxxxxx>; Raed Salem <raeds@xxxxxxxxxxxx>; > >Yishai Hadas <yishaih@xxxxxxxxxxxx>; Saeed Mahameed > ><saeedm@xxxxxxxxxxxx>; linux-netdev <netdev@xxxxxxxxxxxxxxx> > >Subject: [PATCH rdma-next v3 10/14] IB/uverbs: Add support for flow > >counters > > > >From: Raed Salem <raeds@xxxxxxxxxxxx> > > > >The struct ib_uverbs_flow_spec_action_count associates > >a counters object with the flow. > > > >Post this association the flow counters can be read via > >the counters object. > > > >Reviewed-by: Yishai Hadas <yishaih@xxxxxxxxxxxx> > >Signed-off-by: Raed Salem <raeds@xxxxxxxxxxxx> > >Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> > >--- > > drivers/infiniband/core/uverbs.h | 1 + > > drivers/infiniband/core/uverbs_cmd.c | 81 > >+++++++++++++++++++++++++++++++----- > > include/uapi/rdma/ib_user_verbs.h | 13 ++++++ > > 3 files changed, 84 insertions(+), 11 deletions(-) > > > >diff --git a/drivers/infiniband/core/uverbs.h > >b/drivers/infiniband/core/uverbs.h > >index 5b2461fa634d..c0d40fc3a53a 100644 > >--- a/drivers/infiniband/core/uverbs.h > >+++ b/drivers/infiniband/core/uverbs.h > >@@ -263,6 +263,7 @@ struct ib_uverbs_flow_spec { > > struct ib_uverbs_flow_spec_action_tag flow_tag; > > struct ib_uverbs_flow_spec_action_drop drop; > > struct ib_uverbs_flow_spec_action_handle action; > >+ struct ib_uverbs_flow_spec_action_count flow_count; > > }; > > }; > > > >diff --git a/drivers/infiniband/core/uverbs_cmd.c > >b/drivers/infiniband/core/uverbs_cmd.c > >index ddb9d79691be..3179a95c6f5e 100644 > >--- a/drivers/infiniband/core/uverbs_cmd.c > >+++ b/drivers/infiniband/core/uverbs_cmd.c > >@@ -2748,43 +2748,82 @@ ssize_t ib_uverbs_detach_mcast(struct > >ib_uverbs_file *file, > > struct ib_uflow_resources { > > size_t max; > > size_t num; > >- struct ib_flow_action *collection[0]; > >+ size_t collection_num; > >+ size_t counters_num; > >+ struct ib_counters **counters; > >+ struct ib_flow_action **collection; > > }; > > > > static struct ib_uflow_resources *flow_resources_alloc(size_t num_specs) > > { > > struct ib_uflow_resources *resources; > > > >- resources = > >- kmalloc(sizeof(*resources) + > >- num_specs * sizeof(*resources->collection), > >GFP_KERNEL); > >+ resources = kzalloc(sizeof(*resources), GFP_KERNEL); > > > > if (!resources) > >- return NULL; > >+ goto err_res; > > Why the new goto? No real reason :) > > >+ > >+ resources->counters = > >+ kcalloc(num_specs, sizeof(*resources->counters), > >GFP_KERNEL); > >+ > >+ if (!resources->counters) > >+ goto err_cnt; > > kcalloc() zeros stuff. Could you just have a single common goto for the > cleanup? I have mixed feelings regarding such approach, technically you are right, but I think that it will hurt readability. I can send followup patch, will it work for you? Thanks for review. > > Mike > > >+ > >+ resources->collection = > >+ kcalloc(num_specs, sizeof(*resources->collection), > >GFP_KERNEL); > >+ > >+ if (!resources->collection) > >+ goto err_collection; > > > >- resources->num = 0; > > resources->max = num_specs; > > > > return resources; > >+ > >+err_collection: > >+ kfree(resources->counters); > >+err_cnt: > >+ kfree(resources); > >+err_res: > >+ return NULL; > > } > > > > void ib_uverbs_flow_resources_free(struct ib_uflow_resources *uflow_res) > > { > > unsigned int i; > > > >- for (i = 0; i < uflow_res->num; i++) > >+ for (i = 0; i < uflow_res->collection_num; i++) > > atomic_dec(&uflow_res->collection[i]->usecnt); > > > >+ for (i = 0; i < uflow_res->counters_num; i++) > >+ atomic_dec(&uflow_res->counters[i]->usecnt); > >+ > >+ kfree(uflow_res->collection); > >+ kfree(uflow_res->counters); > > kfree(uflow_res); > > } > > > > static void flow_resources_add(struct ib_uflow_resources *uflow_res, > >- struct ib_flow_action *action) > >+ enum ib_flow_spec_type type, > >+ void *ibobj) > > { > > WARN_ON(uflow_res->num >= uflow_res->max); > > > >- atomic_inc(&action->usecnt); > >- uflow_res->collection[uflow_res->num++] = action; > >+ switch (type) { > >+ case IB_FLOW_SPEC_ACTION_HANDLE: > >+ atomic_inc(&((struct ib_flow_action *)ibobj)->usecnt); > >+ uflow_res->collection[uflow_res->collection_num++] = > >+ (struct ib_flow_action *)ibobj; > >+ break; > >+ case IB_FLOW_SPEC_ACTION_COUNT: > >+ atomic_inc(&((struct ib_counters *)ibobj)->usecnt); > >+ uflow_res->counters[uflow_res->counters_num++] = > >+ (struct ib_counters *)ibobj; > >+ break; > >+ default: > >+ WARN_ON(1); > >+ } > >+ > >+ uflow_res->num++; > > } > > > > static int kern_spec_to_ib_spec_action(struct ib_ucontext *ucontext, > >@@ -2821,9 +2860,29 @@ static int kern_spec_to_ib_spec_action(struct > >ib_ucontext *ucontext, > > return -EINVAL; > > ib_spec->action.size = > > sizeof(struct ib_flow_spec_action_handle); > >- flow_resources_add(uflow_res, ib_spec->action.act); > >+ flow_resources_add(uflow_res, > >+ IB_FLOW_SPEC_ACTION_HANDLE, > >+ ib_spec->action.act); > > uobj_put_obj_read(ib_spec->action.act); > > break; > >+ case IB_FLOW_SPEC_ACTION_COUNT: > >+ if (kern_spec->flow_count.size != > >+ sizeof(struct ib_uverbs_flow_spec_action_count)) > >+ return -EINVAL; > >+ ib_spec->flow_count.counters = > >+ uobj_get_obj_read(counters, > >+ UVERBS_OBJECT_COUNTERS, > >+ kern_spec->flow_count.handle, > >+ ucontext); > >+ if (!ib_spec->flow_count.counters) > >+ return -EINVAL; > >+ ib_spec->flow_count.size = > >+ sizeof(struct ib_flow_spec_action_count); > >+ flow_resources_add(uflow_res, > >+ IB_FLOW_SPEC_ACTION_COUNT, > >+ ib_spec->flow_count.counters); > >+ uobj_put_obj_read(ib_spec->flow_count.counters); > >+ break; > > default: > > return -EINVAL; > > } > >diff --git a/include/uapi/rdma/ib_user_verbs.h > >b/include/uapi/rdma/ib_user_verbs.h > >index 409507f83b91..4f9991de8e3a 100644 > >--- a/include/uapi/rdma/ib_user_verbs.h > >+++ b/include/uapi/rdma/ib_user_verbs.h > >@@ -998,6 +998,19 @@ struct ib_uverbs_flow_spec_action_handle { > > __u32 reserved1; > > }; > > > >+struct ib_uverbs_flow_spec_action_count { > >+ union { > >+ struct ib_uverbs_flow_spec_hdr hdr; > >+ struct { > >+ __u32 type; > >+ __u16 size; > >+ __u16 reserved; > >+ }; > >+ }; > >+ __u32 handle; > >+ __u32 reserved1; > >+}; > >+ > > struct ib_uverbs_flow_tunnel_filter { > > __be32 tunnel_id; > > }; > >-- > >2.14.3 >
Attachment:
signature.asc
Description: PGP signature
