On 08/06/2015 02:18 PM, Parav Pandit wrote: > On Thu, Aug 6, 2015 at 4:30 PM, Haggai Eran <[email protected] > <mailto:[email protected]>> wrote: > > On Wednesday, August 5, 2015 8:16 PM, Jason Gunthorpe > [email protected] > <mailto:[email protected]>> 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. > > Haggai > > 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. >
I don't see how increasing the size now would get you anything that changing the returned response_length field wouldn't. I'm not sure what you consider an ABI change. Doesn't adding new meaning to reserved fields count as a change? In any case, increasing the response length doesn't require adding new calls. The kernel code will agree to fill only the fields that fit in the buffer provided by the user-space caller. Haggai -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
