[ofa-general] [PATCH 1/3] libibverbs : add new verbs: create_qp_expanded

[Jack Morgenstein]

On Sunday 20 July 2008 22:14, Ron Livne wrote:
> index f8138ef..47ec759 100644
> --- a/include/infiniband/driver.h
> +++ b/include/infiniband/driver.h
> @@ -155,6 +155,11 @@ int ibv_cmd_reg_xrc_rcv_qp(struct ibv_xrc_domain *xrc_domain,
>  			   uint32_t xrc_qp_num);
>  int ibv_cmd_unreg_xrc_rcv_qp(struct ibv_xrc_domain *xrc_domain,
>  			     uint32_t xrc_qp_num);
> +int ibv_cmd_create_qp_expanded(struct ibv_pd *pd,
> +			struct ibv_qp *qp, struct ibv_qp_init_attr *attr,
> +			uint32_t create_flags,
> +			struct ibv_create_qp *cmd, size_t cmd_size,
> +			struct ibv_create_qp_resp *resp, size_t resp_size);
> 
You should be defining new structures: 
   struct ibv_create_qp_expanded
and
   struct ibv_create_qp_expanded_resp
(Possibly only the first is needed).


> 
>  /*
> @@ -455,7 +456,7 @@ struct ibv_create_qp {
>  	__u8  sq_sig_all;
>  	__u8  qp_type;
>  	__u8  is_srq;
> -	__u8  reserved;
> +	__u8  create_flags;
>  	__u64 driver_data[0];
>  };

You should not need to touch struct ibv_create_qp
You should define
	struct ibv_create_qp_expanded {
		...
		__u8  sq_sig_all;
		__u8  qp_type;
		__u8  is_srq;
		__u8  reserved;
		__u32 create_flags;
		__u32 reserved2;
		__u64 driver_data[0];
	};
and use that all along the way.

> diff --git a/src/cmd.c b/src/cmd.c
> index cedf55e..802f3ef 100644
> --- a/src/cmd.c
> +++ b/src/cmd.c
> @@ -642,7 +642,68 @@ int ibv_cmd_create_qp(struct ibv_pd *pd,
>  	cmd->srq_handle      = attr->qp_type == IBV_QPT_XRC ?
>  		(attr->xrc_domain ? attr->xrc_domain->handle : 0) :
>  		(attr->srq ? attr->srq->handle : 0);
> -	cmd->reserved	     = 0;
> +	cmd->create_flags     = 0;
> +
> +	if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
> +		return errno;
> +
> +	VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
> +
> +	qp->handle 		  = resp->qp_handle;
> +	qp->qp_num 		  = resp->qpn;
> +	qp->context		  = pd->context;
> +
> +	if (abi_ver > 3) {
> +		attr->cap.max_recv_sge    = resp->max_recv_sge;
> +		attr->cap.max_send_sge    = resp->max_send_sge;
> +		attr->cap.max_recv_wr     = resp->max_recv_wr;
> +		attr->cap.max_send_wr     = resp->max_send_wr;
> +		attr->cap.max_inline_data = resp->max_inline_data;
> +	}
> +
> +	if (abi_ver == 4) {
> +		struct ibv_create_qp_resp_v4 *resp_v4 =
> +			(struct ibv_create_qp_resp_v4 *) resp;
> +
> +		memmove((void *) resp + sizeof *resp,
> +			(void *) resp_v4 + sizeof *resp_v4,
> +			resp_size - sizeof *resp);
> +	} else if (abi_ver <= 3) {
> +		struct ibv_create_qp_resp_v3 *resp_v3 =
> +			(struct ibv_create_qp_resp_v3 *) resp;
> +
> +		memmove((void *) resp + sizeof *resp,
> +			(void *) resp_v3 + sizeof *resp_v3,
> +			resp_size - sizeof *resp);
> +	}
> +
> +	return 0;
> +}
> +
> +int ibv_cmd_create_qp_expanded(struct ibv_pd *pd,
> +		      struct ibv_qp *qp, struct ibv_qp_init_attr *attr,
> +			uint32_t create_flags,
> +		      struct ibv_create_qp *cmd, size_t cmd_size,
> +		      struct ibv_create_qp_resp *resp, size_t resp_size)
> +{
> +	IBV_INIT_CMD_RESP(cmd, cmd_size, CREATE_QP, resp, resp_size);
> +
> +	cmd->create_flags    = create_flags;
> +	cmd->user_handle     = (uintptr_t) qp;
> +	cmd->pd_handle 	     = pd->handle;
> +	cmd->send_cq_handle  = attr->send_cq->handle;
> +	cmd->recv_cq_handle  = attr->recv_cq->handle;
> +	cmd->max_send_wr     = attr->cap.max_send_wr;
> +	cmd->max_recv_wr     = attr->cap.max_recv_wr;
> +	cmd->max_send_sge    = attr->cap.max_send_sge;
> +	cmd->max_recv_sge    = attr->cap.max_recv_sge;
> +	cmd->max_inline_data = attr->cap.max_inline_data;
> +	cmd->sq_sig_all	     = attr->sq_sig_all;
> +	cmd->qp_type 	     = attr->qp_type;
> +	cmd->is_srq 	     = !!attr->srq;
> +	cmd->srq_handle      = attr->qp_type == IBV_QPT_XRC ?
> +		(attr->xrc_domain ? attr->xrc_domain->handle : 0) :
> +		(attr->srq ? attr->srq->handle : 0);
> 
>  	if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
>  		return errno;

You should be modifying ibv_cmd_create_qp so that it calls
ibv_cmd_create_qp_expanded with the "create_flags" parameter = 0;
This will allow you to shrink the code.

> @@ -449,6 +449,32 @@ struct ibv_qp *__ibv_create_qp(struct ibv_pd *pd,
>  }
>  default_symver(__ibv_create_qp, ibv_create_qp);
> 
> +struct ibv_qp *__ibv_create_qp_expanded(struct ibv_pd *pd,
> +				struct ibv_qp_init_attr *qp_init_attr,
> +				uint32_t create_flags)
> +{
> +	struct ibv_qp *qp = pd->context->more_ops->create_qp_expanded(pd, qp_init_attr, create_flags);
> +
> +	if (qp) {
> +		qp->context          = pd->context;
> +		qp->qp_context       = qp_init_attr->qp_context;
> +		qp->pd               = pd;
> +		qp->send_cq          = qp_init_attr->send_cq;
> +		qp->recv_cq          = qp_init_attr->recv_cq;
> +		qp->srq              = qp_init_attr->srq;
> +		qp->qp_type          = qp_init_attr->qp_type;
> +		qp->state            = IBV_QPS_RESET;
> +		qp->events_completed = 0;
> +		qp->xrc_domain       = qp_init_attr->qp_type == IBV_QPT_XRC ?
> +			qp_init_attr->xrc_domain : NULL;
> +		pthread_mutex_init(&qp->mutex, NULL);
> +		pthread_cond_init(&qp->cond, NULL);
> +	}
> +
> +	return qp;
> +}
> +default_symver(__ibv_create_qp_expanded, ibv_create_qp_expanded);
You don't need default_symver here, because this is a completely new verb.
just use 
	struct ibv_qp *ibv_create_qp_expanded(struct ibv_pd *pd,
				struct ibv_qp_init_attr *qp_init_attr,
				uint32_t create_flags)