Skip to contentSkip to conversation

IB/hns: Fix some error handling in hns_roce_v1_query_qp()

PATCH In review Dan Carpenter 2 participants 4 messages Oct 14, 2016 – Oct 24, 2016 1 message not in the archive

Subsystems (3)
Kernel janitors InfiniBand HiSilicon RoCE driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 2/2 IB/hns: Fix some error handling in hns_roce_v1_query_qp()

to_ib_qp_state() returns IB_QPS_ERR (6) on error, it doesn't return -1.

Fixes 9a4435375cd1 ('IB/hns: Add driver files for hns RoCE driver')
Signed-off-by Dan Carpenter <[email protected]>
drivers/infiniband/hw/hns/hns_roce_hw_v1.c +1 -1
index 58b150e..280abac 100644
@@ -2544,7 +2544,7 @@ int hns_roce_v1_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
 			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S);
 	tmp_qp_state = (int)to_ib_qp_state((enum hns_roce_qp_state)state);
-	if (tmp_qp_state = -1) {
+	if (tmp_qp_state = IB_QPS_ERR) {
 		dev_err(dev, "to_ib_qp_state error\n");
 		ret = -EINVAL;
 		goto out;
#1 lore raw
Quoted 19 lines · Dan Carpenter

to_ib_qp_state() returns IB_QPS_ERR (6) on error, it doesn't return -1.

Fixes 9a4435375cd1 ('IB/hns: Add driver files for hns RoCE driver')
Signed-off-by Dan Carpenter <[email protected]>
drivers/infiniband/hw/hns/hns_roce_hw_v1.c +1 -1
index 58b150e..280abac 100644
@@ -2544,7 +2544,7 @@ int hns_roce_v1_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
 			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S);
 	tmp_qp_state = (int)to_ib_qp_state((enum hns_roce_qp_state)state);
-	if (tmp_qp_state = -1) {
+	if (tmp_qp_state = IB_QPS_ERR) {
 		dev_err(dev, "to_ib_qp_state error\n");
 		ret = -EINVAL;
 		goto out;
Quoted 2 lines

.

Hi, Dan Carpenter
This is good for me.

Thanks

#2 lore raw
In reply to Dan Carpenter
Quoted 19 lines · Dan Carpenter

to_ib_qp_state() returns IB_QPS_ERR (6) on error, it doesn't return -1.

Fixes 9a4435375cd1 ('IB/hns: Add driver files for hns RoCE driver')
Signed-off-by Dan Carpenter <[email protected]>
drivers/infiniband/hw/hns/hns_roce_hw_v1.c +1 -1
index 58b150e..280abac 100644
@@ -2544,7 +2544,7 @@ int hns_roce_v1_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
 			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S);
 	tmp_qp_state = (int)to_ib_qp_state((enum hns_roce_qp_state)state);
-	if (tmp_qp_state = -1) {
+	if (tmp_qp_state = IB_QPS_ERR) {
 		dev_err(dev, "to_ib_qp_state error\n");
 		ret = -EINVAL;
 		goto out;
Quoted 2 lines

.

Hi, Dan Carpenter
After checking your modification, I think that it maybe fix this:
static enum ib_qp_state to_ib_qp_state(enum hns_roce_qp_state state)

{
	switch (state) {
	case HNS_ROCE_QP_STATE_RST:
		return IB_QPS_RESET;
	case HNS_ROCE_QP_STATE_INIT:
		return IB_QPS_INIT;
	case HNS_ROCE_QP_STATE_RTR:
		return IB_QPS_RTR;
	case HNS_ROCE_QP_STATE_RTS:
		return IB_QPS_RTS;
	case HNS_ROCE_QP_STATE_SQD:
		return IB_QPS_SQD;
	case HNS_ROCE_QP_STATE_ERR:
		return IB_QPS_ERR;
	default:
		return -1;
	}
}

because the IB_QPS_ERR is legal state, but the tmp_qp_state should return a illegal state when the the to_ib_qp_state executed the default branch.

thanks
Lijun Ou

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#4 lore raw

Good catch. Thanks for the review. I will resend.

regards,
dan carpenter