On Thu, Mar 15, 2018 at 08:48:12AM -0600, Jason Gunthorpe wrote:
> On Thu, Mar 15, 2018 at 04:35:39PM +0200, Leon Romanovsky wrote:
> > On Wed, Mar 14, 2018 at 01:42:34PM -0600, Jason Gunthorpe wrote:
> > > On Tue, Mar 13, 2018 at 03:14:20PM +0200, Leon Romanovsky wrote:
> > > > +struct mlx5_ib_burst_info {
> > > > + __u32 max_burst_sz;
> > > > + __u16 typical_pkt_sz;
> > > > +};
> > > > +
> > > > +struct mlx5_ib_modify_qp {
> > > > + __u32 comp_mask;
> > > > + struct mlx5_ib_burst_info burst_info;
> > > > + __u8 reserved[6];
> > > > +};
> > >
> > > This reserved stuff isn't right, padding is needed in the burst_info
> > > struct, and then a u32 at the end here.
> > >
> > > Why do we need the mlx5_ib_burst_info struct anyhow? Just increases
> > > the chance someone will get the padding wrong..
> >
> > I think that separation to structs minimizes chances of errors, we just
> > need to pay attention that structs are aligned to 64b.
>
> Well, it didn't minimize the chance of padding error (this would have
> been correct without the struct), so what error do you think having
> the struct will minimize?
>
> So far I've seen lots of mistakes from these extra structs..
>
> If there is an actual use then fine, but I can't see wa use here..
If we ensure that all new structs are 64b, we won't see no errors at all.
It groups fields by functionality instead of throwing all in one pile,
like it was done in struct ib_device.
>
> Jason
Attachment:
signature.asc
Description: PGP signature
