Re: [PATCH for-next v2] IB/core: Only update PKEY and GID caches on respective events

[Leon Romanovsky <leon@kernel.org>]

On Thu, May 06, 2021 at 10:31:44AM +0200, Håkon Bugge wrote:
> Both the PKEY and GID tables in an HCA can hold in the order of
> hundreds entries. Reading them are expensive. Partly because the API
> for retrieving them only returns a single entry at a time. Further, on
> certain implementations, e.g., CX-3, the VFs are paravirtualized in
> this respect and have to rely on the PF driver to perform the
> read. This again demands VF to PF communication.
> 
> IB Core's cache is refreshed on all events. Hence, filter the refresh
> of the PKEY and GID caches based on the event received being
> IB_EVENT_PKEY_CHANGE and IB_EVENT_GID_CHANGE respectively.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> 
> ---
> 
> v1 -> v2:
>    * Changed signature of ib_cache_update() as per Leon's suggestion
>    * Added Fixes tag as per Zhu Yanjun' suggestion
> ---
>  drivers/infiniband/core/cache.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index 5c9fac7..1493a60 100644
> --- a/drivers/infiniband/core/cache.c
> +++ b/drivers/infiniband/core/cache.c
> @@ -1472,10 +1472,12 @@ static int config_non_roce_gid_cache(struct ib_device *device,
>  }
>  
>  static int
> -ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
> +ib_cache_update(struct ib_device *device, u8 port, bool update_gids,
> +		bool update_pkeys, bool enforce_security)
>  {
>  	struct ib_port_attr       *tprops = NULL;
> -	struct ib_pkey_cache      *pkey_cache = NULL, *old_pkey_cache;
> +	struct ib_pkey_cache      *pkey_cache = NULL;
> +	struct ib_pkey_cache      *old_pkey_cache = NULL;
>  	int                        i;
>  	int                        ret;
>  
> @@ -1492,14 +1494,16 @@ static int config_non_roce_gid_cache(struct ib_device *device,
>  		goto err;
>  	}
>  
> -	if (!rdma_protocol_roce(device, port)) {
> +	if (!rdma_protocol_roce(device, port) && update_gids) {

Can you please elaborate why it is safe to do for IB_EVENT_GID_CHANGE only?
What about IB_EVENT_CLIENT_REREGISTER?

Thanks