On Thu, Jan 03, 2019 at 06:07:18PM +0200, Leon Romanovsky wrote:
> On Thu, Jan 03, 2019 at 03:48:05PM +0000, Jason Gunthorpe wrote:
> > On Thu, Jan 03, 2019 at 11:35:43AM +0530, Devesh Sharma wrote:
> >
> > > > > struct bnxt_re_uctx_resp {
> > > > > - __u32 dev_id;
> > > > > - __u32 max_qp;
> > > > > + __u32 chip_id0;
> > > > > + __u32 chip_id1;
> > > >
> > > > You changed something called max_qp into chip_id1 ? How is that
> > > > backwards compatible???
> > > Yes, this is breaking indeed as Leon also commented. replacing with
> > > union and ABI range check
> > > will be implemented in V2.
> >
> > I'm not sure how a union will save you - add the new stuff to the end
> > is the usual approach.
>
> Why won't union work?
>
> struct bnxt_re_uctx_resp {
> union {
> __u32 dev_id;
> __u32 chip_id0;
> }
> union {
> __u32 max_qp;
> __u32 chip_id1;
> }
> ....
>
> AND ABI_VERSION is define, so backward compilation will work too
>
> #ifdef ABI_VERSION < 2
We've never approached ABI_VERSION like this - if you crank ABI
version you throw away source compilation compatability too.
The issue here is ABI compatability, today's rdma-core provider
expectx the bytes for max_qp to be max_qp, you can't just change it to
be dev_id without breaking everything.
ABI_VERSION should only be used in distress - add the new data to the
end of the struct and preserve the existing fields.
Jason
