On Wed, May 20, 2015 at 04:03:33PM -0600, Jason Gunthorpe wrote:
> On Wed, May 20, 2015 at 05:23:45PM -0400, ira.weiny wrote:
> > On Wed, May 20, 2015 at 12:27:35PM -0600, Jason Gunthorpe wrote:
> > > On Wed, May 20, 2015 at 04:13:23AM -0400, [email protected] wrote:
> > > > -struct ib_rmpp_mad {
> > > > +struct ib_rmpp_base {
> > > >         struct ib_mad_hdr       mad_hdr;
> > > >         struct ib_rmpp_hdr      rmpp_hdr;
> > > > +} __packed;
> > > > +
> > > > +struct ib_rmpp_mad {
> > > > +       struct ib_rmpp_base     base;
> > > >         u8                      data[IB_MGMT_RMPP_DATA];
> > > >  };
> > > 
> > > Why can't we just use:
> > > 
> > >  u8 data[];
> > > 
> > > And replace various sizeof(ib_rmpp_mad) with a rmpp_payload_size()
> > > call?
> > 
> > I don't think it makes much difference.  I think there is just 1 place we 
> > use
> > that.
> 
> There are many lines that are just churning rmpp_mad to
> rmpp_base, using a flex array avoids that entirely.

Ok.

> 
> Similarly patch 12 doesn't have to introduce a opa specific structure anymore.

Ok.

> 
> The same sort of comment probably applies to the mad_hdr/etc as well,
> but I didn't look very closely.

This would mean changing all the existing drivers which I'm not really keen on
doing.  Existing drivers using IBTA MADs expect the MADs to be exactly 256
bytes and that is exactly how struct ib_mad is defined.  So I think we need to
keep the definitions for drivers to use.

However, I think we can clean up the use of sizeof(ib_mad) in the MAD code such
that it is agnostic to the device/port type once the MAD size patch is in
place.  I'll look at doing that early in the series.

Ira

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

Reply via email to