On 2019/11/5 12:09, Parav Pandit wrote:
> Hi Yixian Liu,
>
>> -----Original Message-----
>> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
>> owner@xxxxxxxxxxxxxxx> On Behalf Of Yixian Liu
>> Sent: Monday, November 4, 2019 9:48 PM
>> To: dledford@xxxxxxxxxx; jgg@xxxxxxxx
>> Cc: Parav Pandit <parav@xxxxxxxxxxxx>; leon@xxxxxxxxxx; linux-
>> rdma@xxxxxxxxxxxxxxx; linuxarm@xxxxxxxxxx
>> Subject: [PATCH rdma-next] RDMA/core: Use pr_warn_ratelimited
>>
>> This warning can be triggered easily when adding a large number of VLANs
>> while the capacity of GID table is small.
>>
>> Fixes: 598ff6bae689 ("IB/core: Refactor GID modify code for RoCE")
>> Signed-off-by: Yixian Liu <liuyixian@xxxxxxxxxx>
>> ---
>> drivers/infiniband/core/cache.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
> Thanks for the patch. However, vlan netdevice addition is an administrative operation allowed to process which has CAP_NET_ADMIN privilege.
> So large number of VLAN addition by a user for a RoCE device whose GID capacity is small is constrained operation.
How can we constrain this operation from an administrator?
> Therefore, we shouldn't be rate limiting it.
> By rate limiting we miss the information about any bugs in GID cache management.
pr_warn_ratelimited only prevent from printing **the same messages** frequently, why information will be lost?
> At best we can convert it to dev_dbg() so that we still get the necessary debug information when needed.
> I wanted to convert them trace functions which will allow us to more debug level prints such as netdev name, vlan etc.
> I think I remember comment from Leon too while working on it, but it was long haul that time.
>
> Its base infrastructure is just got ready with Chuck Lever's patch.
> So around 5.5, I should convert to trace calls.
Okay, whatever, the situation described in commit message may be occurred, please consider it.
>
>> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
>> index 00fb3ea..2990075 100644
>> --- a/drivers/infiniband/core/cache.c
>> +++ b/drivers/infiniband/core/cache.c
>> @@ -579,8 +579,8 @@ static int __ib_cache_gid_add(struct ib_device
>> *ib_dev, u8 port,
>> out_unlock:
>> mutex_unlock(&table->lock);
>> if (ret)
>> - pr_warn("%s: unable to add gid %pI6 error=%d\n",
>> - __func__, gid->raw, ret);
>> + pr_warn_ratelimited("%s: unable to add gid %pI6
>> error=%d\n",
>> + __func__, gid->raw, ret);
>> return ret;
>> }
>>
>> --
>> 2.7.4
>
>
>
