On Wed, 20 Jun 2018 12:00:37 -0600
Jason Gunthorpe <jgg@xxxxxxxxxxxx> wrote:
>
> diff --git a/drivers/infiniband/core/uverbs_cmd.c
> b/drivers/infiniband/core/uverbs_cmd.c index
> 908ee8ab32972f..d34179d2aa6d4b 100644 ---
> a/drivers/infiniband/core/uverbs_cmd.c +++
> b/drivers/infiniband/core/uverbs_cmd.c @@ -2027,13 +2027,35 @@ static
> int modify_qp(struct ib_uverbs_file *file, attr->alt_timeout
> = cmd->base.alt_timeout; attr->rate_limit = cmd->rate_limit;
>
> - if (cmd->base.attr_mask & IB_QP_AV)
> + if (cmd->base.attr_mask & IB_QP_AV) {
> + unsigned int primary_port = (cmd->base.attr_mask & IB_QP_PORT) ?
> + cmd->base.port_num :
> + qp->port;
Hi Jason,
You have a problem in the above line when qp != qp->real_qp (as is the case with XRC
target QPs).
The problem is that for XRC target QPs, only qp->real_qp->port is updated when
the IB_QP_PORT is specified. Thus, for XRC TGT qps, qp->port is left at 0.
(At this point in your code, you are still dealing with the "outer" qp.
This changes under function ib_modify_qp_with_udata(), where all modifications
are done (and recorded) on qp->real_qp. This includes updating qp->port:
In file drivers/infiniband/core/verbs.c, seee function ib_modify_qp_with_udata():
int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr,
int attr_mask, struct ib_udata *udata)
{
return _ib_modify_qp(ib_qp->real_qp, attr, attr_mask, udata);
}
and under function _ib_modify_qp() in the same file:
if (attr_mask & IB_QP_PORT)
qp->port = attr->port_num;
Thus, notice that it qp->real_qp->port which gets updated.
For all QP types except XRC_TGT, this is OK since qp->real_qp = qp.
However, for XRC_TGT, qp->real_qp != qp.
Thus:
+ unsigned int primary_port = (cmd->base.attr_mask & IB_QP_PORT) ?
+ cmd->base.port_num :
+ qp->port;
should be:
+ unsigned int primary_port = (cmd->base.attr_mask & IB_QP_PORT) ?
+ cmd->base.port_num :
+ qp->real_qp->port;
-Jack
> +
> + /*
> + * The Linux ABI specifies the primary port number
> both
> + * loosely in the struct (as IBA suggests) and also
> in the
> + * AV. The primary AV port number must always agree
> with the
> + * primary port number set by IB_QP_PORT.
> + */
> + if (cmd->base.dest.port_num != primary_port) {
> + ret = -EINVAL;
> + goto release_qp;
> + }
> copy_ah_attr_from_uverbs(qp->device, &attr->ah_attr,
> &cmd->base.dest);
> + }
> +
> + if (cmd->base.attr_mask & IB_QP_ALT_PATH) {
> + if (!rdma_is_port_valid(qp->device,
> + cmd->base.alt_dest.port_num)) {
> + ret = -EINVAL;
> + goto release_qp;
> + }
>
> - if (cmd->base.attr_mask & IB_QP_ALT_PATH)
> copy_ah_attr_from_uverbs(qp->device,
> &attr->alt_ah_attr, &cmd->base.alt_dest);
> + }
>
> ret = ib_modify_qp_with_udata(qp, attr,
> modify_qp_mask(qp->qp_type,
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
