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

[Jason Gunthorpe <jgg@mellanox.com>]

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..

Jason