Re: [PATCH for-next v3 1/2] RDMA/core: Add helper function to retrieve driver gid context from gid attr

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Feb 17, 2020 at 10:20:09PM -0800, Selvin Xavier wrote:
> Adding a helper function to retrieve the driver gid context
> from the gid attr.
> 
> Suggested-by: Jason Gunthorpe <jgg@mellanox.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
>  drivers/infiniband/core/cache.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/rdma/ib_cache.h         |  1 +
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index 17bfedd..1b73a71 100644
> +++ b/drivers/infiniband/core/cache.c
> @@ -973,6 +973,47 @@ int rdma_query_gid(struct ib_device *device, u8 port_num,
>  EXPORT_SYMBOL(rdma_query_gid);
>  
>  /**
> + * rdma_read_gid_hw_context - Read the HW GID context from GID attribute
> + * @attr:		Potinter to the GID attribute
> + *
> + * rdma_read_gid_hw_context() reads the vendor drivers GID HW
> + * context corresponding to SGID attr. It takes reference to the GID
> + * attribute and this need to be released by the caller using
> + * rdma_put_gid_attr
> + *
> + * Returns HW context on success or NULL on error
> + *
> + */
> +void *rdma_read_gid_hw_context(const struct ib_gid_attr *attr)
> +{
> +	struct ib_gid_table_entry *entry =
> +		container_of(attr, struct ib_gid_table_entry, attr);
> +	struct ib_device *device = entry->attr.device;
> +	u8 port_num = entry->attr.port_num;
> +	struct ib_gid_table *table;
> +	unsigned long flags;
> +	void *context = NULL;
> +
> +	if (!rdma_is_port_valid(device, port_num))
> +		return NULL;
> +
> +	table = rdma_gid_table(device, port_num);
> +	read_lock_irqsave(&table->rwlock, flags);
> +
> +	if (attr->index < 0 || attr->index >= table->sz ||
> +	    !is_gid_entry_valid(table->data_vec[attr->index]))
> +		goto done;

Why all this validation and locking? ib_gid_attrs are only created by
the core code..

> +	get_gid_entry(entry);

And why a get? Surely it is invalid to call this function without a
get already held?

Jason