Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Adopt robust udata

[Konstantin Taranov]

> > +struct mana_ib_uctx_req {
> > +       __aligned_u64 client_caps1;
> > +       __aligned_u64 client_caps2;
> > +       __aligned_u64 client_caps3;
> > +       __aligned_u64 client_caps4;
> > +       __aligned_u64 comp_mask;
> > +};
> > +
> 
> I am curious about the addition of these unused "client_caps" fields.
> 
> I guess the idea is to be able to reject older providers that lack support for
> some mandatory feature in the future - like if a new HW variant breaks the
> descriptor ABI or something and therefore requires a provider update?

Not really. The capability bits will be used for simpler integration of new features,
and not to reject older providers. To reject an older provider, I could use the abi version.

> 
> My main question is: how come they need to be added now as opposed to
> extending the structure later?
> 

these client caps allow mana_ib to integrate easier client wide optimizations or feature enablement.
Once we have some important code in the rdma-core that the kernel should know about, we can declare it
as a capability of a client (meaning a client can do additionally X). Important aspect that the capability is
an addition, and it is not a new behavior (meaning a client is backwards compatible). So it is handy for bug
fixes and new features, rather than overwhelming other udata requests via chained changes.

For example, the upcoming patch after this one is that WQEs in the rdma-core will be of fixed size.
The HW supports all sizes, but the knowledge that all WQEs are the same will allow the HW to apply optimizations.
So, this fixed size can be defined as a capability. Then in the kernel code for QP creation, we can add HW flag from the client
capability (so a simple line in the kernel. 1) if the cap present then add a certain flag to HWC).
Otherwise, the change would be more complex: new response for alloc_ucontext, and then new request for various qp create.

A similar idea was in bnxt_re (see BNXT_RE_COMP_MASK_REQ_UCNTX_POW2_SUPPORT and uctx->cmask),
but I think there was misunderstanding as the cap field was named as comp_mask and now bnxt_re is locked to 2 capability bits.
As it is the first rdma-core ioctl, there is no way to know which comp_mask is allowed. With the wave of robust udata,
providers will be locked to one udata request format for alloc_ucontext() without a chance of extending.
That is why, I try to introduce the idea now.

All in all, I believe it would be beneficial for kernel to get some initial feedback from rdma-core,
and I think it was an initial goal of having udata request in alloc_ucontext().

- Konstantin

> I'm not proposing any changes, just trying to understand the intent.
> 
> Thanks,
> Jake