Re: [rdma-next v1 13/22] RDMA/hns: Remove empty functions

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]

On Fri, Aug 18, 2017 at 03:16:12PM +0530, Selvin Xavier wrote:
>  Removing the "modify_port" hook is causing some issues with ib_cm module.
> While adding a mad agent for the new device, ib_cm invokes
> ib_modify_port  which fails
> with return value -ENOSYS. So the mad agent gets unregistered. This
> break the connection
> establishment. It is broken for bnxt_re also.
>
> The following patch helps. But it is like a workaround to solve the problem.
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index d5ca101..59911dd 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -4189,7 +4189,7 @@ static void cm_add_one(struct ib_device *ib_device)
>                         goto error2;
>
>                 ret = ib_modify_port(ib_device, i, 0, &port_modify);
> -               if (ret)
> +               if (ret && ret != -ENOSYS)
>                         goto error3;
>
>                 count++;
>
> Or should we have the modify_port hook with some basic checks?

I think that your proposal is right thing to do. The driver should
properly return the status of its callbacks (-ENOSYS) and it is
responsibility of the caller to decide what to do in such case.

The dummy function (return 0) hides the fact that modify_port do
nothing and it can be wrong for some callers.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]