Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 6, 2015 at 4:30 PM, Haggai Eran <haggaie@xxxxxxxxxxxx> wrote:
> On Wednesday, August 5, 2015 8:16 PM, Jason Gunthorpe jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote:
>> On Wed, Aug 05, 2015 at 06:34:26PM +0300, Amir Vadai wrote:
>>>  struct ib_uverbs_ex_query_device {
>>>       __u32 comp_mask;
>>> +     __u32 csum_caps;
>>>       __u32 reserved;
>>>  };
>>
>> Uh no.
> This is the struct of the command, not the response. There is no need to extend it. The command is designed to always return as much information as possible, so the user space code doesn't need to pass anything for it to work.
>
> Even if you did want to extend it, you would need to replace the reserved word. The structs in this header file must be made in such way that they have the same size on 32-bit systems and on 64-bit systems (see the comment at the beginning of the header file). This is why the reserved word is there.
>
>>
>>> @@ -221,6 +222,7 @@ struct ib_uverbs_odp_caps {
>>>  struct ib_uverbs_ex_query_device_resp {
>>>       struct ib_uverbs_query_device_resp base;
>>>       __u32 comp_mask;
>>> +     __u32 csum_caps;
>>>       __u32 response_length;
>>>       struct ib_uverbs_odp_caps odp_caps;
>>>       __u64 timestamp_mask;
>>
>> Also totally wrong.
>
> The response struct must maintain backward compatibility. You cannot change the order of the existing fields. The only valid way of extending it is at the end. Here too, you must make sure that the struct has the same size on 32-bit systems, so you would need to add a 32-bit reserved word at the end.
>

As struct ib_uverbs_ex_query_device_resp captures extended
capabilities, does it make sense to have few more reserved words
defined as part of this patch?
So that later on those reserved can be defined in future for
additional features.
This way for every new feature we dont need to bump structure size of
ABI, not we need to define new set of ABI calls.
Its hard to say how much more is sufficient, but was thinking of 8 32-bit words.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux