[ofa-general] Re: [PATCH] for-2.6.23 ib/umad: add partition support

[Michael S. Tsirkin]

> Quoting Sean Hefty <mshefty at ichips.intel.com>:
> Subject: Re: [PATCH] for-2.6.23 ib/umad: add partition support
> 
> Roland Dreier wrote:
> > > -#define IB_USER_MAD_ABI_VERSION	5
> > > +#define IB_USER_MAD_ABI_VERSION	6
> >
> >Bummer -- we've been able to keep the ABI stable for almost 2 years
> >now.  I wonder if there's something clever we can do to avoid breaking
> >existing apps?
> 
> Did you have something in mind?  (new ioctl?  re-using existing fields?)
> 
> Not all fields are used for both reads and writes.  E.g. status is 
> unused on a write, and retries is unused on a read.

We made a mistake of not validating the offset field otherwise we could
have used it, too: as it is I think apps just use "write" so
there's a useless byte counter in that field.

But if we do one of these things, the app does not get any indication that pkey's
ignored, isn't that right?

> Storing the 
> pkey_index on a read seems doable.  I think if we do anything on a 
> write, we need to make an assumption that the data is currently set to 0 
> by the app.

Suggestion:
We currently have:
        if (count < sizeof (struct ib_user_mad) + IB_MGMT_RMPP_HDR)
                return -EINVAL;


So we can have short writes set per-open-file properties such as pkey:
just be sure to validate the offset too for these so we can reuse
offsets other than 0 in the future.

This assumes an open file desriptor per-pkey, so the proposed API
extension umad_set_pkey would have to be changed to be per-port rather
than per-mad. But I think this is a better API, too: most apps
likely work within a single partition.



-- 
MST