On Thu, Feb 07, 2019 at 10:24:42AM -0700, Jason Gunthorpe wrote:
> On Thu, Feb 07, 2019 at 05:43:51PM +0200, Leon Romanovsky wrote:
> > 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
>
> In any event, the check for 0 port is wrong, so it need some kind of
> revision.
See, my other email reply on that.
>
> > >
> > > > + 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
>
> The kernel code supports switches for some reason
>
> > by the user should pass rdma_is_port_valid() check, which requires
> > port to be >= 1.
>
> Not if rdma_cap_ib_switch is true.
Long time ago, I posted patch to remove this switch, because it is
forwarded directly from HW to sysfs without any kernel need of it.
Someone complained (I don't remember who) that it hurts their out-of-tree
kernel and he asked politely do not remove this bit together with check,
so we did this, but decided that rdmatool won't support switches for now.
This is why port == 0 is not valid. You probably can find that discussion
in web archives.
Thanks
>
> Jason
Attachment:
signature.asc
Description: PGP signature
