RE: [RDMA RFC v6 14/16] RDMA/irdma: Add ABI definitions

["Saleem, Shiraz" <shiraz.saleem@intel.com>]

> Subject: Re: [RDMA RFC v6 14/16] RDMA/irdma: Add ABI definitions
> 
> On Wed, May 20, 2020 at 12:02:35PM +0300, Gal Pressman wrote:
> > On 20/05/2020 11:52, Greg KH wrote:
> > > On Wed, May 20, 2020 at 10:54:25AM +0300, Gal Pressman wrote:
> > >> On 20/05/2020 10:04, Jeff Kirsher wrote:
> > >>> +struct i40iw_create_qp_resp {
> > >>> +   __u32 qp_id;
> > >>> +   __u32 actual_sq_size;
> > >>> +   __u32 actual_rq_size;
> > >>> +   __u32 i40iw_drv_opt;
> > >>> +   __u16 push_idx;
> > >>> +   __u8 lsmm;
> > >>> +   __u8 rsvd;
> > >>> +};
> > >>
> > >> This struct size should be 8 bytes aligned.
> > >
> > > Aligned in what way?  Seems sane to me, what would you want it to
> > > look like instead?
> >
> > The uverbs ABI structs sizes are assumed to be padded to 8 bytes
> > alignment, I would expect the reserved field to be an array of 5 bytes
> > as done in other structs in this file (irdma_modify_qp_req for example).
> > Jason could correct me if I'm wrong?
> 
> "it is complicated"
> 
> The udata structs must have alignment that is compatible with the core struct that
> prefixes them. Of course we have a mess here, and nothing is uniform..
> 
> In this case struct ib_uverbs_create_qp_resp has a '__u32 driver_data[0]' aligned
> to 8 bytes thus the alignment of this struct can be 4 or 8.
> 
> I generally don't recommend relying on this weird side effect, and encourage
> explicit padding when possible, but since the intent of this new driver is to be ABI
> compatible with the old driver, it should be kept the same.
> 
> The userspace has a number of static_asserts which are designed to automatically
> check these various cases. I assume Intel has revised the userspace to use the
> new struct names and tested it..
> 

Thanks Jason for the explanation! Yes these abi structs are kept the same for old user-space compatibility. And yes its been tested with old user-space.