| Message ID | 1604057975-23388-8-git-send-email-liweihang@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA/hns: Support UD for HIP09 | expand |
On Fri, Oct 30, 2020 at 07:39:34PM +0800, Weihang Li wrote: > HIP09 supports service type of Unreliable Datagram, add necessary process > to enable this feature. > > Signed-off-by: Weihang Li <liweihang@huawei.com> > drivers/infiniband/hw/hns/hns_roce_device.h | 2 ++ > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 8 +++++--- > drivers/infiniband/hw/hns/hns_roce_qp.c | 3 ++- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h > index 9a032d0..23f8fe7 100644 > +++ b/drivers/infiniband/hw/hns/hns_roce_device.h > @@ -222,7 +222,9 @@ enum { > HNS_ROCE_CAP_FLAG_FRMR = BIT(8), > HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL = BIT(9), > HNS_ROCE_CAP_FLAG_ATOMIC = BIT(10), > + HNS_ROCE_CAP_FLAG_UD = BIT(11), Why add this flag if nothing reads it? > HNS_ROCE_CAP_FLAG_SDI_MODE = BIT(14), > + > }; Extra space If I recall properly earlier chips did not have a GID table so could not support UD because they could not secure the AH, or something like that. So, I would expect to see that only the new devices support UD, but I can't quite see that in this patch?? Jason
On 2020/11/13 2:35, Jason Gunthorpe wrote: > On Fri, Oct 30, 2020 at 07:39:34PM +0800, Weihang Li wrote: >> HIP09 supports service type of Unreliable Datagram, add necessary process >> to enable this feature. >> >> Signed-off-by: Weihang Li <liweihang@huawei.com> >> drivers/infiniband/hw/hns/hns_roce_device.h | 2 ++ >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 8 +++++--- >> drivers/infiniband/hw/hns/hns_roce_qp.c | 3 ++- >> 3 files changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h >> index 9a032d0..23f8fe7 100644 >> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h >> @@ -222,7 +222,9 @@ enum { >> HNS_ROCE_CAP_FLAG_FRMR = BIT(8), >> HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL = BIT(9), >> HNS_ROCE_CAP_FLAG_ATOMIC = BIT(10), >> + HNS_ROCE_CAP_FLAG_UD = BIT(11), > > Why add this flag if nothing reads it? Hi Jason, I checked it to set IB_USER_VERBS_CMD_CREATE_AH and IB_USER_VERBS_CMD_DESTROY_AH in uverbs_cmd_mask which is not needed recently, I forgot to remove it in this patch :) Will drop it. > >> HNS_ROCE_CAP_FLAG_SDI_MODE = BIT(14), >> + >> }; > > Extra space OK, will remove it. > > > If I recall properly earlier chips did not have a GID table so could > not support UD because they could not secure the AH, or something like > that. > > So, I would expect to see that only the new devices support UD, but > I can't quite see that in this patch?? You are right. I made a mistake, I thought it's enough to add judgment of hardware version in rdma-core to prevent the HIP08's user from using UD in userspace and I realize it's meaningless just now... I think I can add a check about the hardware version when creating a user's UD QP. Thanks again for the reminder. Weihang > > Jason >
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h index 9a032d0..23f8fe7 100644 --- a/drivers/infiniband/hw/hns/hns_roce_device.h +++ b/drivers/infiniband/hw/hns/hns_roce_device.h @@ -222,7 +222,9 @@ enum { HNS_ROCE_CAP_FLAG_FRMR = BIT(8), HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL = BIT(9), HNS_ROCE_CAP_FLAG_ATOMIC = BIT(10), + HNS_ROCE_CAP_FLAG_UD = BIT(11), HNS_ROCE_CAP_FLAG_SDI_MODE = BIT(14), + }; #define HNS_ROCE_DB_TYPE_COUNT 2 diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c index ab68e6b..29cbf9f 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c @@ -701,7 +701,7 @@ static int hns_roce_v2_post_send(struct ib_qp *ibqp, ~(((qp->sq.head + nreq) >> ilog2(qp->sq.wqe_cnt)) & 0x1); /* Corresponding to the QP type, wqe process separately */ - if (ibqp->qp_type == IB_QPT_GSI) + if (ibqp->qp_type == IB_QPT_GSI || ibqp->qp_type == IB_QPT_UD) ret = set_ud_wqe(qp, wr, wqe, &sge_idx, owner_bit); else if (ibqp->qp_type == IB_QPT_RC) ret = set_rc_wqe(qp, wr, wqe, &sge_idx, owner_bit); @@ -1892,7 +1892,7 @@ static void set_default_caps(struct hns_roce_dev *hr_dev) caps->flags |= HNS_ROCE_CAP_FLAG_ATOMIC | HNS_ROCE_CAP_FLAG_MW | HNS_ROCE_CAP_FLAG_SRQ | HNS_ROCE_CAP_FLAG_FRMR | - HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL; + HNS_ROCE_CAP_FLAG_QP_FLOW_CTRL | HNS_ROCE_CAP_FLAG_UD; caps->num_qpc_timer = HNS_ROCE_V2_MAX_QPC_TIMER_NUM; caps->qpc_timer_entry_sz = HNS_ROCE_V2_QPC_TIMER_ENTRY_SZ; @@ -5157,7 +5157,9 @@ static int hns_roce_v2_destroy_qp_common(struct hns_roce_dev *hr_dev, unsigned long flags; int ret = 0; - if (hr_qp->ibqp.qp_type == IB_QPT_RC && hr_qp->state != IB_QPS_RESET) { + if ((hr_qp->ibqp.qp_type == IB_QPT_RC || + hr_qp->ibqp.qp_type == IB_QPT_UD) && + hr_qp->state != IB_QPS_RESET) { /* Modify qp to reset before destroying qp */ ret = hns_roce_v2_modify_qp(&hr_qp->ibqp, NULL, 0, hr_qp->state, IB_QPS_RESET); diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c index e288946..9ffd92a 100644 --- a/drivers/infiniband/hw/hns/hns_roce_qp.c +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c @@ -1010,6 +1010,7 @@ struct ib_qp *hns_roce_create_qp(struct ib_pd *pd, switch (init_attr->qp_type) { case IB_QPT_RC: case IB_QPT_GSI: + case IB_QPT_UD: break; default: ibdev_err(ibdev, "not support QP type %d\n", @@ -1030,7 +1031,7 @@ struct ib_qp *hns_roce_create_qp(struct ib_pd *pd, if (ret) { ibdev_err(ibdev, "Create QP type 0x%x failed(%d)\n", init_attr->qp_type, ret); - ibdev_err(ibdev, "Create GSI QP failed!\n"); + kfree(hr_qp); return ERR_PTR(ret); }
HIP09 supports service type of Unreliable Datagram, add necessary process to enable this feature. Signed-off-by: Weihang Li <liweihang@huawei.com> --- drivers/infiniband/hw/hns/hns_roce_device.h | 2 ++ drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 8 +++++--- drivers/infiniband/hw/hns/hns_roce_qp.c | 3 ++- 3 files changed, 9 insertions(+), 4 deletions(-)