On 02-Apr-19 21:35, Leon Romanovsky wrote:
> On Mon, Apr 01, 2019 at 04:36:42PM +0300, Gal Pressman wrote:
>> On 01-Apr-19 16:26, Leon Romanovsky wrote:
>>> On Mon, Apr 01, 2019 at 03:04:55PM +0300, Gal Pressman wrote:
>>>> On 01-Apr-19 11:47, Leon Romanovsky wrote:
>>>>> From: Mark Zhang <markz@xxxxxxxxxxxx>
>>>>>
>>>>> Add an API to support set/clear per-port auto mode.
>>>>>
>>>>> Signed-off-by: Mark Zhang <markz@xxxxxxxxxxxx>
>>>>> Reviewed-by: Majd Dibbiny <majd@xxxxxxxxxxxx>
>>>>> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>>>>> ---
>>>>> +void rdma_counter_init(struct ib_device *dev)
>>>>> +{
>>>>> + struct rdma_port_counter *port_counter;
>>>>> + u32 port;
>>>>> +
>>>>> + if (!dev->ops.alloc_hw_stats)
>>>>
>>>> Why is alloc_hw_stats needed?
>>>
>>> We are using mlx5_ib Q-counters to bind/unbind and they are allocated through
>>> alloc_hw_stat interface. This interface is not available in all flavours of mlx4.
>>
>> What about other drivers which are not mlx4/5?
>> From an API standpoint, I don't see a reason to require alloc_hw_stats.
>>
>> This kinda relates to my question of whether these new counters are related to
>> the existing hw_counters. This check suggests that they are, but I don't
>> understand why.
>
> I see "rdma statistics ..." as an ultimate tool to present everything
> related to device statistics. It includes already exposed HW counters.
> This is an extra reason why we want ops.alloc_hw_stats check.
The rdma statistics is the userspace entry point to two (or more) kernel APIs,
we shouldn't add a dependency between the kernel APIs themselves.
