On Tue, Feb 05, 2019 at 02:52:08PM -0700, Jason Gunthorpe wrote:
> On Sat, Feb 02, 2019 at 01:42:47PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > Implement doit callbacks and ensure that users won't provide port values
> > on resource entry allocated in per-device mode needed for .doit callback.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > drivers/infiniband/core/nldev.c | 186 +++++++++++++++++++++-----------
> > include/rdma/restrack.h | 2 +-
> > 2 files changed, 127 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> > index 7bbc98080488..e5910bbbcec7 100644
> > +++ b/drivers/infiniband/core/nldev.c
> > @@ -375,7 +375,6 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
> > struct ib_qp *qp = container_of(res, struct ib_qp, res);
> > struct ib_device *dev = qp->device;
> > struct ib_qp_init_attr qp_init_attr;
> > - struct nlattr *entry_attr;
> > struct ib_qp_attr qp_attr;
> > int ret;
> >
> > @@ -386,10 +385,6 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
> > if (port && port != qp_attr.port_num)
> > return 0;
>
> Now that the caller is checking if the port is needed or not these
> 'if (port)' and !port should be removed from the fill functions
I'll check
>
> > + if ((port && fe->flags & NLDEV_PER_DEV) ||
> > + (!port && ~fe->flags & NLDEV_PER_DEV)) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
>
> 0 is a valid port number, this should be something like
Why? We are not supporting IB switches for now, and port provided
by the user should pass rdma_is_port_valid() check, which requires
port to be >= 1.
>
> if (tb[RDMA_NLDEV_ATTR_PORT_INDEX]) {
> if (fe->flags & NLDEV_PER_DEV) {
> ret = -EINVAL;
> goto err;
> }
> port = nla_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]);
> if (!rdma_is_port_valid(device, port)) {
> ret = -EINVAL;
> goto err;
> }
> } else {
> if (!(fe->flags & NLDEV_PER_DEV)) {
> ret = -EINVAL;
> goto err;
> }
> }
>
> > diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> > index b1e80818d8ab..b8fa99392837 100644
> > +++ b/include/rdma/restrack.h
> > @@ -58,7 +58,7 @@ struct rdma_restrack_entry;
> > */
> > struct rdma_restrack_root {
> > /*
> > - * @rwsem: Read/write lock to protect lists
> > + * @rwsem: Read/write lock to protect lists and IDR
> */
>
> This hunk doesn't seem to belong
Thanks
>
> Jason
Attachment:
signature.asc
Description: PGP signature
