[ofa-general] Re: [PATCHv4 RFC] Scalable Reliable Connection: API and documentation

[Jack Morgenstein]

On Thursday 09 August 2007 20:29, Roland Dreier wrote:
>  > +++ b/SRC.txt
>  > @@ -0,0 +1,138 @@
>  > +Here's some documentation on Scalable Reliable Connections.
> 
>  > +		ibv_create_src_srq;
>  >  		ibv_modify_srq;
>  >  		ibv_query_srq;
>  >  		ibv_destroy_srq;
>  > @@ -35,6 +36,8 @@ IBVERBS_1.0 {
>  >  		ibv_destroy_ah;
>  >  		ibv_attach_mcast;
>  >  		ibv_detach_mcast;
>  > +		ibv_open_src_domain;
>  > +		ibv_close_src_domain;
> 
> Also, if we're going to change the signature of the create_srq method:
> 
>  >  	struct ibv_srq *	(*create_srq)(struct ibv_pd *pd,
>  > +					      struct ibv_src_domain *src_domain,
>  > +					      struct ibv_cq *src_cq,
>  >  					      struct ibv_srq_init_attr *srq_init_attr);
> 
> then I think we need something like
> 
> #define IBV_CREATE_SRQ_METHOD_HAS_SRC_PARAMS
> 
> so that driver libraries can still work with both old and new
> libibverbs.  I'm not sure the gain of avoiding a new entry point is
> worth forcing every driver library (even the ones that don't do SRC)
> to change -- it might be better to mimic the consumer API and just
> have a new create_srq_srq method.  That way driver libraries that
> don't do SRC would stay source compatible (I think).
> 
>  - R.

Having looked at implementation issues, it will be much simpler to just, as you say,
mimic the consumer API -- in kernel space as well.

I don't see the need to increment the libibverbs ABI for this -- if we can get away with adding open/close
src domain without doing so, I don't see that adding <driver-ops>->(*create_src_srq) would present a problem.

However, we may have a problem with ibv_create_qp (which will need to add a field to qp_init_attr).

IMHO, we will need to increment the kernel IB_USER_VERBS_ABI_VERSION to 7 (see below).

For example, the addition of src (or xrc?) qp's demands adding a domain field to ibv_init_qp_attr.
This will require adding an xrc_domain field to the verbs-layer command structure ibv_create_qp.
Since mlx4_create_qp (for example) has ibv_create_qp embedded in it, this will throw off all the field
calculations for the driver data if an older version of libibverbs is used:

struct ibv_create_qp {
	__u32 command;
	__u16 in_words;
	__u16 out_words;
	__u64 response;
	__u64 user_handle;
	__u32 pd_handle;
	__u32 send_cq_handle;
	__u32 recv_cq_handle;
	__u32 srq_handle;
	__u32 max_send_wr;
	__u32 max_recv_wr;
	__u32 max_send_sge;
	__u32 max_recv_sge;
	__u32 max_inline_data;
	__u8  sq_sig_all;
	__u8  qp_type;
	__u8  is_srq;
	__u8  reserved;
	__u32 xrc_domain_handle;  <-- need to add this here
	__u64 driver_data[0];
};


struct mlx4_create_qp {
	struct ibv_create_qp		ibv_cmd;  <-- We are including the verbs layer struct here
	__u64				buf_addr;
	__u64				db_addr;
	__u8				log_sq_bb_count;
	__u8				log_sq_stride;
	__u8				sq_no_prefetch;	/* was reserved in ABI 2 */
	__u8				reserved[5];
};

Why not add xrc_domain to the driver-data area?  Because the xrc_domain is a ib verbs-layer construct.

Will this mean that the libibverbs version must be increased to 1.2 ?  Will we have a problem with apps
which were compiled with an earlier version of the libibverbs header files?

- Jack