On Fri, Mar 23, 2018 at 04:22:05PM -0600, Jason Gunthorpe wrote:
> On Tue, Mar 20, 2018 at 03:01:37PM +0200, Leon Romanovsky wrote:
> > From: Matan Barak <matanb@xxxxxxxxxxxx>
> >
> > Binding a flow_action to flow steering rule requires using a new
> > specification. Therefore, adding such an IB_FLOW_SPEC_ACTION_HANDLE flow
> > specification.
> >
> > Flow steering rules could use flow_action(s) and as of that we need to
> > avoid deleting flow_action(s) as long as they're being used.
> > Moreover, when the attached rules are deleted, action_handle reference
> > count should be decremented. Introducing a new mechanism of flow
> > resources to keep track on the attached action_handle(s). Later on, this
> > mechanism should be extended to other attached flow steering resources
> > like flow counters.
> >
> > Reviewed-by: Yishai Hadas <yishaih@xxxxxxxxxxxx>
> > Signed-off-by: Matan Barak <matanb@xxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > drivers/infiniband/core/uverbs.h | 8 +++
> > drivers/infiniband/core/uverbs_cmd.c | 86 +++++++++++++++++++++++++++---
> > drivers/infiniband/core/uverbs_std_types.c | 14 ++++-
> > include/rdma/ib_verbs.h | 8 +++
> > include/uapi/rdma/ib_user_verbs.h | 13 +++++
> > 5 files changed, 121 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
> > index a94b5e7ee02a..1bac0b51686a 100644
> > +++ b/drivers/infiniband/core/uverbs.h
> > @@ -203,11 +203,18 @@ struct ib_ucq_object {
> > u32 async_events_reported;
> > };
> >
> > +struct ib_uflow_resources;
> > +struct ib_uflow_object {
> > + struct ib_uobject uobject;
> > + struct ib_uflow_resources *resources;
> > +};
> > +
> > extern const struct file_operations uverbs_event_fops;
> > void ib_uverbs_init_event_queue(struct ib_uverbs_event_queue *ev_queue);
> > struct file *ib_uverbs_alloc_async_event_file(struct ib_uverbs_file *uverbs_file,
> > struct ib_device *ib_dev);
> > void ib_uverbs_free_async_event_file(struct ib_uverbs_file *uverbs_file);
> > +void ib_uverbs_flow_resources_free(struct ib_uflow_resources *uflow_res);
> >
> > void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
> > struct ib_uverbs_completion_event_file *ev_file,
> > @@ -254,6 +261,7 @@ struct ib_uverbs_flow_spec {
> > struct ib_uverbs_flow_spec_ipv6 ipv6;
> > 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;
> > };
> > };
> >
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > index 1a42b0219935..9c424f178b76 100644
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -2739,8 +2739,52 @@ ssize_t ib_uverbs_detach_mcast(struct ib_uverbs_file *file,
> > return ret ? ret : in_len;
> > }
> >
> > -static int kern_spec_to_ib_spec_action(struct ib_uverbs_flow_spec *kern_spec,
> > - union ib_flow_spec *ib_spec)
> > +struct ib_uflow_resources {
> > + size_t max;
> > + size_t num;
> > + struct ib_flow_action *collection[0];
> > +};
> > +
> > +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);
>
> num_specs is controlled by userspace, in this case it is limited to
> u16, so no bug today, however it would be easy to see the limit
> quietly go away when someone does the ioctl conversion someday.
>
> So lets have this checked please?
Hi Jason,
Matan is OOO in those days, can such change be sent as a patch followup
after he returns to the office?
>
> if (num_specs >= (SIZE_MAX - sizeof(*resources))/sizeof(*resources->collection))
> return ERR_PTR(-ENOMEM);
>
>
> Jason
Attachment:
signature.asc
Description: PGP signature
