| Message ID | 20210405055000.215792-2-leon@kernel.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Generalize if ULP supported check | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 05/04/2021 8:49, Leon Romanovsky wrote: > From: Parav Pandit <parav@nvidia.com> > > RDMA devices are of different transport(iWarp, IB, RoCE) and have > different attributes. > Not all clients are interested in all type of devices. > > Implement a generic callback that each IB client can implement to decide > if client add() or remove() should be done by the IB core or not for a > given IB device, client combination. > > Signed-off-by: Parav Pandit <parav@nvidia.com> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > --- > drivers/infiniband/core/device.c | 3 +++ > include/rdma/ib_verbs.h | 9 +++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > index c660cef66ac6..c9af2deba8c1 100644 > --- a/drivers/infiniband/core/device.c > +++ b/drivers/infiniband/core/device.c > @@ -691,6 +691,9 @@ static int add_client_context(struct ib_device *device, > if (!device->kverbs_provider && !client->no_kverbs_req) > return 0; > > + if (client->is_supported && !client->is_supported(device)) > + return 0; Isn't it better to remove the kverbs_provider flag (from previous if statement) and unify it with this generic support check?
On Mon, Apr 05, 2021 at 09:20:32AM +0300, Gal Pressman wrote: > On 05/04/2021 8:49, Leon Romanovsky wrote: > > From: Parav Pandit <parav@nvidia.com> > > > > RDMA devices are of different transport(iWarp, IB, RoCE) and have > > different attributes. > > Not all clients are interested in all type of devices. > > > > Implement a generic callback that each IB client can implement to decide > > if client add() or remove() should be done by the IB core or not for a > > given IB device, client combination. > > > > Signed-off-by: Parav Pandit <parav@nvidia.com> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > --- > > drivers/infiniband/core/device.c | 3 +++ > > include/rdma/ib_verbs.h | 9 +++++++++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > > index c660cef66ac6..c9af2deba8c1 100644 > > --- a/drivers/infiniband/core/device.c > > +++ b/drivers/infiniband/core/device.c > > @@ -691,6 +691,9 @@ static int add_client_context(struct ib_device *device, > > if (!device->kverbs_provider && !client->no_kverbs_req) > > return 0; > > > > + if (client->is_supported && !client->is_supported(device)) > > + return 0; > > Isn't it better to remove the kverbs_provider flag (from previous if statement) > and unify it with this generic support check? I thought about it, but didn't find it worth. The kverbs_provider needs to be provided by device and all ULPs except uverbs will have the same check. Thanks
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index c660cef66ac6..c9af2deba8c1 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -691,6 +691,9 @@ static int add_client_context(struct ib_device *device, if (!device->kverbs_provider && !client->no_kverbs_req) return 0; + if (client->is_supported && !client->is_supported(device)) + return 0; + down_write(&device->client_data_rwsem); /* * So long as the client is registered hold both the client and device diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 59138174affa..777fbcbd4858 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -2756,6 +2756,15 @@ struct ib_client { const union ib_gid *gid, const struct sockaddr *addr, void *client_data); + /* + * Returns if the client is supported for a given device or not. + * @dev: An RDMA device to check if client can support this RDMA or not. + * + * A client that is interested in specific device attributes, should + * implement it to check if client can be supported for this device or + * not. + */ + bool (*is_supported)(struct ib_device *dev); refcount_t uses; struct completion uses_zero;