[PATCH 1/2] RDMA/uverbs: Don't set rcq if qp_type is IB_QPT_XRC_INI

[PATCH 1/2] RDMA/uverbs: Don't set rcq if qp_type is IB_QPT_XRC_INI

[Xiao Yang]

INI QP doesn't require receive CQ.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 drivers/infiniband/core/uverbs_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 418d133a8fb0..d8bc8ea3ad1e 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1345,7 +1345,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 		if (has_sq)
 			scq = uobj_get_obj_read(cq, UVERBS_OBJECT_CQ,
 						cmd->send_cq_handle, attrs);
-		if (!ind_tbl)
+		if (!ind_tbl && cmd->qp_type != IB_QPT_XRC_INI)
 			rcq = rcq ?: scq;
 		pd = uobj_get_obj_read(pd, UVERBS_OBJECT_PD, cmd->pd_handle,
 				       attrs);
-- 
2.25.1

[PATCH 2/2] RDMA/rxe: Add check for supported QP types

[Xiao Yang]

1) Current rdma_rxe only supports five QP types which always sets recv_cq.
2) INI QP doesn't set recv_cq(NULL) so creating INI QP over softroce
   triggers 'missing cq' warning.

Avoid the warning by checking supported QP type.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 drivers/infiniband/sw/rxe/rxe_qp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index 656a5b4be847..65c8df812aeb 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -62,6 +62,17 @@ int rxe_qp_chk_init(struct rxe_dev *rxe, struct ib_qp_init_attr *init)
 	struct rxe_port *port;
 	int port_num = init->port_num;
 
+	switch(init->qp_type) {
+	case IB_QPT_SMI:
+	case IB_QPT_GSI:
+	case IB_QPT_RC:
+	case IB_QPT_UC:
+	case IB_QPT_UD:
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
 	if (!init->recv_cq || !init->send_cq) {
 		pr_warn("missing cq\n");
 		goto err1;
-- 
2.25.1

Re: [PATCH 2/2] RDMA/rxe: Add check for supported QP types

[Jason Gunthorpe]

On Wed, Dec 16, 2020 at 03:17:55PM +0800, Xiao Yang wrote:
> 1) Current rdma_rxe only supports five QP types which always sets recv_cq.
> 2) INI QP doesn't set recv_cq(NULL) so creating INI QP over softroce
>    triggers 'missing cq' warning.
> 
> Avoid the warning by checking supported QP type.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
>  drivers/infiniband/sw/rxe/rxe_qp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index 656a5b4be847..65c8df812aeb 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -62,6 +62,17 @@ int rxe_qp_chk_init(struct rxe_dev *rxe, struct ib_qp_init_attr *init)
>  	struct rxe_port *port;
>  	int port_num = init->port_num;
>  
> +	switch(init->qp_type) {
> +	case IB_QPT_SMI:
> +	case IB_QPT_GSI:
> +	case IB_QPT_RC:
> +	case IB_QPT_UC:
> +	case IB_QPT_UD:
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}

This does make sense, but not really because of recv_cq - rxe doesn't
support other QP types at all and should return an error when making
an XRC anything

So applied to for-next

Thanks,
Jason

Re: [PATCH 1/2] RDMA/uverbs: Don't set rcq if qp_type is IB_QPT_XRC_INI

[Jason Gunthorpe]

On Wed, Dec 16, 2020 at 03:17:54PM +0800, Xiao Yang wrote:
> INI QP doesn't require receive CQ.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
>  drivers/infiniband/core/uverbs_cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 418d133a8fb0..d8bc8ea3ad1e 100644
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -1345,7 +1345,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
>  		if (has_sq)
>  			scq = uobj_get_obj_read(cq, UVERBS_OBJECT_CQ,
>  						cmd->send_cq_handle, attrs);
> -		if (!ind_tbl)
> +		if (!ind_tbl && cmd->qp_type != IB_QPT_XRC_INI)
>  			rcq = rcq ?: scq;

Hmm, this does make it consistent with the UVERBS_METHOD_QP_CREATE
flow which does set attr.recv_cq to NULL if the user didn't specify one.

However this has been like this since the beginning - what are you
doing that this detail matters?

Jason

Re: [PATCH 1/2] RDMA/uverbs: Don't set rcq if qp_type is IB_QPT_XRC_INI

[Xiao Yang]

On 2021/1/13 4:09, Jason Gunthorpe wrote:
> On Wed, Dec 16, 2020 at 03:17:54PM +0800, Xiao Yang wrote:
>> INI QP doesn't require receive CQ.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>   drivers/infiniband/core/uverbs_cmd.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
>> index 418d133a8fb0..d8bc8ea3ad1e 100644
>> +++ b/drivers/infiniband/core/uverbs_cmd.c
>> @@ -1345,7 +1345,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
>>   		if (has_sq)
>>   			scq = uobj_get_obj_read(cq, UVERBS_OBJECT_CQ,
>>   						cmd->send_cq_handle, attrs);
>> -		if (!ind_tbl)
>> +		if (!ind_tbl&&  cmd->qp_type != IB_QPT_XRC_INI)
>>   			rcq = rcq ?: scq;
> Hmm, this does make it consistent with the UVERBS_METHOD_QP_CREATE
> flow which does set attr.recv_cq to NULL if the user didn't specify one.
>
> However this has been like this since the beginning - what are you
> doing that this detail matters?
Hi Jason,

Thanks for your comment.
1) I didn't get any issue for now.
2) I think it is not meaningful to set rcq for XRC INITIATOR QP, current 
code has ignores rcq as below:
-------------------------------------------------
static int create_qp(struct uverbs_attr_bundle *attrs,
                     struct ib_uverbs_ex_create_qp *cmd)
...
                 if (cmd->qp_type == IB_QPT_XRC_INI) {
                         cmd->max_recv_wr = 0;
                         cmd->max_recv_sge = 0;
...
-------------------------------------------------

By the way, I have a question:
Why do we need two kinds of uverbs API?(a: read & write, b: ioctl)

Best Regards,
Xiao Yang
> Jason
>
>
> .
>

Re: [PATCH 1/2] RDMA/uverbs: Don't set rcq if qp_type is IB_QPT_XRC_INI

[Jason Gunthorpe]

On Thu, Jan 14, 2021 at 04:58:30PM +0800, Xiao Yang wrote:
> On 2021/1/13 4:09, Jason Gunthorpe wrote:
> > On Wed, Dec 16, 2020 at 03:17:54PM +0800, Xiao Yang wrote:
> > > INI QP doesn't require receive CQ.
> > > 
> > > Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> > >   drivers/infiniband/core/uverbs_cmd.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > > index 418d133a8fb0..d8bc8ea3ad1e 100644
> > > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > > @@ -1345,7 +1345,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
> > >   		if (has_sq)
> > >   			scq = uobj_get_obj_read(cq, UVERBS_OBJECT_CQ,
> > >   						cmd->send_cq_handle, attrs);
> > > -		if (!ind_tbl)
> > > +		if (!ind_tbl&&  cmd->qp_type != IB_QPT_XRC_INI)
> > >   			rcq = rcq ?: scq;
> > Hmm, this does make it consistent with the UVERBS_METHOD_QP_CREATE
> > flow which does set attr.recv_cq to NULL if the user didn't specify one.
> > 
> > However this has been like this since the beginning - what are you
> > doing that this detail matters?
> Hi Jason,
> 
> Thanks for your comment.
> 1) I didn't get any issue for now.
> 2) I think it is not meaningful to set rcq for XRC INITIATOR QP, current
> code has ignores rcq as below:
> static int create_qp(struct uverbs_attr_bundle *attrs,
>                     struct ib_uverbs_ex_create_qp *cmd)
> ...
>                 if (cmd->qp_type == IB_QPT_XRC_INI) {
>                         cmd->max_recv_wr = 0;
>                         cmd->max_recv_sge = 0;
> ...
> 
> By the way, I have a question:
> Why do we need two kinds of uverbs API?(a: read & write, b: ioctl)

The write APIs can't be modified due to how they were
designed. Whenever someone needs to change something they have to move
things to ioctl

Jason

Re: [PATCH 1/2] RDMA/uverbs: Don't set rcq if qp_type is IB_QPT_XRC_INI

[Xiao Yang]

On 2021/1/16 3:23, Jason Gunthorpe wrote:
> On Thu, Jan 14, 2021 at 04:58:30PM +0800, Xiao Yang wrote:
>> On 2021/1/13 4:09, Jason Gunthorpe wrote:
>>> On Wed, Dec 16, 2020 at 03:17:54PM +0800, Xiao Yang wrote:
>>>> INI QP doesn't require receive CQ.
>>>>
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>>    drivers/infiniband/core/uverbs_cmd.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
>>>> index 418d133a8fb0..d8bc8ea3ad1e 100644
>>>> +++ b/drivers/infiniband/core/uverbs_cmd.c
>>>> @@ -1345,7 +1345,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
>>>>    		if (has_sq)
>>>>    			scq = uobj_get_obj_read(cq, UVERBS_OBJECT_CQ,
>>>>    						cmd->send_cq_handle, attrs);
>>>> -		if (!ind_tbl)
>>>> +		if (!ind_tbl&&   cmd->qp_type != IB_QPT_XRC_INI)
>>>>    			rcq = rcq ?: scq;
>>> Hmm, this does make it consistent with the UVERBS_METHOD_QP_CREATE
>>> flow which does set attr.recv_cq to NULL if the user didn't specify one.
>>>
>>> However this has been like this since the beginning - what are you
>>> doing that this detail matters?
>> Hi Jason,
>>
>> Thanks for your comment.
>> 1) I didn't get any issue for now.
>> 2) I think it is not meaningful to set rcq for XRC INITIATOR QP, current
>> code has ignores rcq as below:
>> static int create_qp(struct uverbs_attr_bundle *attrs,
>>                      struct ib_uverbs_ex_create_qp *cmd)
>> ...
>>                  if (cmd->qp_type == IB_QPT_XRC_INI) {
>>                          cmd->max_recv_wr = 0;
>>                          cmd->max_recv_sge = 0;
>> ...
>>
>> By the way, I have a question:
>> Why do we need two kinds of uverbs API?(a: read&  write, b: ioctl)
> The write APIs can't be modified due to how they were
> designed. Whenever someone needs to change something they have to move
> things to ioctl
Hi Jason,

Could you explain that the write APIs can't be modified? :-)

Best Regards,
Xiao Yang
> Jason
>
>
> .
>

Re: [PATCH 1/2] RDMA/uverbs: Don't set rcq if qp_type is IB_QPT_XRC_INI

[Jason Gunthorpe]

On Wed, Dec 16, 2020 at 03:17:54PM +0800, Xiao Yang wrote:
> INI QP doesn't require receive CQ.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  drivers/infiniband/core/uverbs_cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to for-next, thanks

Jason