On Fri, 2019-05-17 at 15:01 -0700, Ira Weiny wrote:
> On Thu, May 16, 2019 at 12:19:44PM -0300, Jason Gunthorpe wrote:
> > On Thu, May 16, 2019 at 02:52:48PM +0300, Kamal Heib wrote:
> > > On Thu, 2019-05-16 at 08:37 -0300, Jason Gunthorpe wrote:
> > > > On Thu, May 16, 2019 at 02:28:40PM +0300, Kamal Heib wrote:
> > > > > On Thu, 2019-05-16 at 08:16 -0300, Jason Gunthorpe wrote:
> > > > > > On Thu, May 16, 2019 at 01:53:08PM +0300, Kamal Heib wrote:
> > > > > > > For RoCE ports the call for ib_modify_port is not
> > > > > > > meaningful,
> > > > > > > so
> > > > > > > simplify the providers of RoCE by return OK in ib_core.
> > > > > > >
> > > > > > > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > > > > > > drivers/infiniband/core/device.c | 23
> > > > > > > ++++++-----
> > > > > > > drivers/infiniband/hw/hns/hns_roce_main.c | 7 ----
> > > > > > > drivers/infiniband/hw/mlx4/main.c | 8 ----
> > > > > > > drivers/infiniband/hw/mlx5/main.c | 6 ---
> > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_main.c | 1 -
> > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 6 ---
> > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.h | 2 -
> > > > > > > drivers/infiniband/hw/qedr/main.c | 1 -
> > > > > > > drivers/infiniband/hw/qedr/verbs.c | 6 ---
> > > > > > > drivers/infiniband/hw/qedr/verbs.h | 2 -
> > > > > > > .../infiniband/hw/vmw_pvrdma/pvrdma_main.c | 1 -
> > > > > > > .../infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 38 ---
> > > > > > > ------
> > > > > > > .../infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 -
> > > > > > > drivers/infiniband/sw/rxe/rxe_verbs.c | 18 ---
> > > > > > > ------
> > > > > > > 14 files changed, 14 insertions(+), 107 deletions(-)
> > > > > >
> > > > > > We have more roce only drivers than this, why isn't
> > > > > > everything
> > > > > > changed?
> > > > > >
> > > > > > Jason
> > > > >
> > > > > Not all of them implements modify_port().
> > > >
> > > > Then why didn't we just delete modify port from the other
> > > > drivers?
> > > >
> > > > Jason
> > >
> > > This patch is doing that for all roce drivers that implement
> > > modify
> > > port, unless you mean none-roce drivers?
> >
> > I mean just delete it without any change to the core code.. Here we
> > are now changing some roce drivers to have a working modify_port
> >
> > It is confusing what the intention is
>
> I see what Jason is saying here. If ib_modify_port() is meaningless
> then lets
> remove the call and let it return -EOPNOTSUPP.
>
Please see below the original implementation of ib_modify_port(), if
modify_port() callback is implemented then it will be called, otherwise
for RoCE driver the return value will be "ok" and for none RoCE driver
it will be "-ENOSYS".
So, what I tried to do in this patch is to return "ok" (like how it is
done for hns, mlx4, mlx5, ocrdma, and qedr) in the ib_core instead in
the drivers, and changed rxe and vmw_pvrdma to be aligned with this
change.
int ib_modify_port(struct ib_device *device,
u8 port_num, int port_modify_mask,
struct ib_port_modify *port_modify)
{
int rc;
if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
if (device->ops.modify_port)
rc = device->ops.modify_port(device, port_num,
port_modify_mask,
port_modify);
else
rc = rdma_protocol_roce(device, port_num) ? 0 :
-ENOSYS;
return rc;
}
> Returning "ok" implies that something worked. I guess that is what
> happens
> now...
>
> Also FWIW you are changing the return from ENOSYS to EOPNOTSUPP. Did
> you mean
> to do that?
>
Yes, I changed the return value to "-EOPNOTSUPP" which is the proper
return value for an unsupported option.
> Ira
>
> > Jason
