Verbs 2.0 questions

Verbs 2.0 questions

[Ruhl, Michael J]

Hi,

I have been playing with the Verbs 2.0 interface a bit (ioctl interface),
and have some comments and things to discuss.

Comments/code review:

1) Some of the DECLARE_xxx() macros (and their associated
UVERBS_TYPE_xxx(), etc) use 'sizeof(struct blah)', other use just a plain
'struct blah'.

Is it possible to do one or the other for these macros for consistency?

I.e.:

(sizeof)
DECLARE_UVERBS_OBJECT(uverbs_object_srq, UVERBS_OBJECT_SRQ,
		      &UVERBS_TYPE_ALLOC_IDR_SZ(sizeof(struct ib_usrq_object), 0,
						  uverbs_free_srq));
(vs struct)
&UVERBS_ATTR_PTR_OUT(DESTROY_CQ_RESP, struct ib_uverbs_destroy_cq_resp,
			     UA_FLAGS(UVERBS_ATTR_SPEC_F_MANDATORY)));

2) Matan has a patch in a private tree: 

Author: Matan Barak <matanb@mellanox.com>
Date:   Tue Jan 23 15:55:04 2018 -0500

    IB/uverbs: Fix method merging in uverbs_ioctl_merge
    
    Fix a bug in uverbs_ioctl_merge that looked at objects iterators
    number instead of methods iterators number when merging methods.
    While we're at it, make the uverbs_ioctl_merge code a bit more clear
    and faster.

That I need in my build environment, or I get a kernel panic when I add my new
objects and methods.  Should (or could) patches like this be up-streamed more
quickly?

3)  The Open Fabrics Alliance Verbs Kernel ABI slide deck (slides 16 and 17) imply
that the standard objects can be augmented with 'DEV Specific' methods and attributes.

Since the methods are defined in the DECLARE_UVERBS_OBJECT(), it is not
clear how to add methods to, say 'uverbs_object_qp' in my driver specific
code.

Are there any examples of how this should be done?

Thanks,

Mike

Re: Verbs 2.0 questions

[Jason Gunthorpe]

On Fri, Feb 02, 2018 at 09:35:17PM +0000, Ruhl, Michael J wrote:
> Hi,
> 
> I have been playing with the Verbs 2.0 interface a bit (ioctl interface),
> and have some comments and things to discuss.
> 
> Comments/code review:
> 
> 1) Some of the DECLARE_xxx() macros (and their associated
> UVERBS_TYPE_xxx(), etc) use 'sizeof(struct blah)', other use just a plain
> 'struct blah'.

Yeah, this should be consistent, also stuff like ordering:

 DECLARE_UVERBS_OBJECT(uverbs_object_srq, UVERBS_OBJECT_SRQ,
vs
 UVERBS_ATTR_PTR_OUT(DESTROY_CQ_RESP, struct ib_uverbs_destroy_cq_resp,

The constant should always be first.

> 2) Matan has a patch in a private tree: 
> 
> Author: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Date:   Tue Jan 23 15:55:04 2018 -0500
> 
>     IB/uverbs: Fix method merging in uverbs_ioctl_merge
>     
>     Fix a bug in uverbs_ioctl_merge that looked at objects iterators
>     number instead of methods iterators number when merging methods.
>     While we're at it, make the uverbs_ioctl_merge code a bit more clear
>     and faster.
> 
> That I need in my build environment, or I get a kernel panic when I add my new
> objects and methods.  Should (or could) patches like this be up-streamed more
> quickly?

The patch is in an 11 patch series coming that fixes various uverbs
ioctl things

  https://github.com/jgunthorpe/linux/commits/ioctl

Should be posted next week unless something wrong is found.

> 3)  The Open Fabrics Alliance Verbs Kernel ABI slide deck (slides 16 and 17) imply
> that the standard objects can be augmented with 'DEV Specific' methods and attributes.
> 
> Since the methods are defined in the DECLARE_UVERBS_OBJECT(), it is not
> clear how to add methods to, say 'uverbs_object_qp' in my driver specific
> code.
> 
> Are there any examples of how this should be done?

I don't think it has ever been done yet.

Honestly I think the entire driver hookup process needs a second set
of eyes to check if it is really the best it can be...

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html