Re: [PATCH rdma-next 03/16] IB/core: Introduce counter set describe verb

Yishai Hadas
2017-10-22T12:00:55Z

On 10/21/2017 3:29 AM, Guy Shattah wrote:
>
>
>> On Thu, 19 Oct 2017, Yishai Hadas wrote:
>>
>>> +#define IB_COUNTER_NAME_LEN 64
>>> +struct ib_counter_set_describe_attr {
>>> + /* Type that this set refers to, use enum ib_counter_set_type */
>>> + u8 counted_type;
>>
>> Gap here of 7 bytes. Maybe move the field to get a more compact structure?
>>
>>> + /* Number of instances of this counter-set available in the hardware
>> */
>>> + u64 num_of_cs;
>>
>> u16 or u32 should be enough right?
>
> Future API might be added to create user-customizable sets.
> How many potential subsets are for a set of 32 counters? More than 2^64.
> We might want to be able to support it.
>
>>
>>> + /* Attributes of the set, use enum ib_counter_set_attributes */
>>> + u32 attributes;
>>> + /* Number of counters in this set */
>>> + u8 entries_count;
>>
>> A counter set is limited to 255? Could there be counter sets that have a
>> thousand or so?
>
> A counter set is merely a set. I can't see a reason why would a user want to query a set
> which contains more than 256 counters. All at the same time.
> However, I agree that we could modify it to u16 or u32.

Thanks Christoper for looking at the series.

In V1 will change this field to be u16 and re-order the fields in struct
ib_counter_set_describe_attr to be more compact as you suggest.