* re: RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter
@ 2014-02-07 11:07 Dan Carpenter
[not found] ` <20140207110728.GA6470-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2014-02-07 11:07 UTC (permalink / raw)
To: parav.pandit-laKkSmNT4hbQT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Parav Pandit,
The patch fe2caefcdf58: "RDMA/ocrdma: Add driver for Emulex
OneConnect IBoE RDMA adapter" from Mar 21, 2012, leads to the
following static checker warning:
drivers/infiniband/hw/ocrdma/ocrdma_verbs.c:1419 ocrdma_query_qp()
warn: right shifting to zero
drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
1414 qp_attr->ah_attr.grh.sgid_index = qp->sgid_idx;
1415 qp_attr->ah_attr.grh.hop_limit = (params.hop_lmt_rq_psn &
1416 OCRDMA_QP_PARAMS_HOP_LMT_MASK) >>
1417 OCRDMA_QP_PARAMS_HOP_LMT_SHIFT;
1418 qp_attr->ah_attr.grh.traffic_class = (params.tclass_sq_psn &
1419 OCRDMA_QP_PARAMS_SQ_PSN_MASK) >>
1420 OCRDMA_QP_PARAMS_TCLASS_SHIFT;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We always set traffic_class to zero. Maybe OCRDMA_QP_PARAMS_TCLASS_MASK
was intended instead of OCRDMA_QP_PARAMS_SQ_PSN_MASK?
Btw, the reason we are seing this bug is because the names are horrible.
Try reading the variable names out loud. OCRDMA_QP_PARAMS_SQ_PSN_MASK
is 6 words. It's 16 syllables long. It takes me about 4 seconds to
just pronounce it. The next one is almost identical except for one or
two characters in the middle. http://www.spotthedifference.com
Even though the variable names are longer than a Tolstoy novel, I still
have no idea what they mean. I know that the "T" in
OCRDMA_QP_PARAMS_TCLASS_SHIFT stands for "traffic" but when it comes to
the "SQ_PSN" then I have no idea about that.
1421
1422 qp_attr->ah_attr.ah_flags = IB_AH_GRH;
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter
[not found] ` <20140207110728.GA6470-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
@ 2014-02-07 11:51 ` Devesh Sharma
0 siblings, 0 replies; 4+ messages in thread
From: Devesh Sharma @ 2014-02-07 11:51 UTC (permalink / raw)
To: Dan Carpenter, Parav Pandit
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Hi Dan,
Thanks for the note, we will supply this patch to fix this on linux-rdma asap.
-Regards
Devesh
-----Original Message-----
From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Dan Carpenter
Sent: Friday, February 07, 2014 4:37 PM
To: Parav Pandit
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: re: RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter
Hello Parav Pandit,
The patch fe2caefcdf58: "RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter" from Mar 21, 2012, leads to the following static checker warning:
drivers/infiniband/hw/ocrdma/ocrdma_verbs.c:1419 ocrdma_query_qp()
warn: right shifting to zero
drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
1414 qp_attr->ah_attr.grh.sgid_index = qp->sgid_idx;
1415 qp_attr->ah_attr.grh.hop_limit = (params.hop_lmt_rq_psn &
1416 OCRDMA_QP_PARAMS_HOP_LMT_MASK) >>
1417 OCRDMA_QP_PARAMS_HOP_LMT_SHIFT;
1418 qp_attr->ah_attr.grh.traffic_class = (params.tclass_sq_psn &
1419 OCRDMA_QP_PARAMS_SQ_PSN_MASK) >>
1420 OCRDMA_QP_PARAMS_TCLASS_SHIFT;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ We always set traffic_class to zero. Maybe OCRDMA_QP_PARAMS_TCLASS_MASK was intended instead of OCRDMA_QP_PARAMS_SQ_PSN_MASK?
Btw, the reason we are seing this bug is because the names are horrible.
Try reading the variable names out loud. OCRDMA_QP_PARAMS_SQ_PSN_MASK is 6 words. It's 16 syllables long. It takes me about 4 seconds to just pronounce it. The next one is almost identical except for one or two characters in the middle. http://www.spotthedifference.com
Even though the variable names are longer than a Tolstoy novel, I still have no idea what they mean. I know that the "T" in OCRDMA_QP_PARAMS_TCLASS_SHIFT stands for "traffic" but when it comes to the "SQ_PSN" then I have no idea about that.
1421
1422 qp_attr->ah_attr.ah_flags = IB_AH_GRH;
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* re: RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter
@ 2015-02-26 12:30 Dan Carpenter
2015-02-26 17:54 ` Devesh Sharma
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2015-02-26 12:30 UTC (permalink / raw)
To: parav.pandit-laKkSmNT4hbQT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Parav Pandit,
The patch fe2caefcdf58: "RDMA/ocrdma: Add driver for Emulex
OneConnect IBoE RDMA adapter" from Mar 21, 2012, leads to the
following static checker warning:
drivers/infiniband/hw/ocrdma/ocrdma_verbs.c:1426 _ocrdma_modify_qp()
warn: bool is not less than zero.
drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
1411 int _ocrdma_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
1412 int attr_mask)
1413 {
1414 int status = 0;
1415 struct ocrdma_qp *qp;
1416 struct ocrdma_dev *dev;
1417 enum ib_qp_state old_qps;
1418
1419 qp = get_ocrdma_qp(ibqp);
1420 dev = get_ocrdma_dev(ibqp->device);
1421 if (attr_mask & IB_QP_STATE)
1422 status = ocrdma_qp_state_change(qp, attr->qp_state, &old_qps);
1423 /* if new and previous states are same hw doesn't need to
1424 * know about it.
1425 */
1426 if (status < 0)
This check is never true. Based on the comment then the check should
be:
if (status == 1)
return SOMETHING;
1427 return status;
1428 status = ocrdma_mbx_modify_qp(dev, qp, attr, attr_mask);
1429
1430 return status;
1431 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter
2015-02-26 12:30 Dan Carpenter
@ 2015-02-26 17:54 ` Devesh Sharma
0 siblings, 0 replies; 4+ messages in thread
From: Devesh Sharma @ 2015-02-26 17:54 UTC (permalink / raw)
To: Dan Carpenter, Parav Pandit
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Thanks Dan for pointing out, we will address this issue and send out a patch to fix this.
-Regards
Devesh
> -----Original Message-----
> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Dan Carpenter
> Sent: Thursday, February 26, 2015 6:00 PM
> To: Parav Pandit
> Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: re: RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA
> adapter
>
> Hello Parav Pandit,
>
> The patch fe2caefcdf58: "RDMA/ocrdma: Add driver for Emulex OneConnect
> IBoE RDMA adapter" from Mar 21, 2012, leads to the following static checker
> warning:
>
> drivers/infiniband/hw/ocrdma/ocrdma_verbs.c:1426
> _ocrdma_modify_qp()
> warn: bool is not less than zero.
>
> drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> 1411 int _ocrdma_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
> 1412 int attr_mask)
> 1413 {
> 1414 int status = 0;
> 1415 struct ocrdma_qp *qp;
> 1416 struct ocrdma_dev *dev;
> 1417 enum ib_qp_state old_qps;
> 1418
> 1419 qp = get_ocrdma_qp(ibqp);
> 1420 dev = get_ocrdma_dev(ibqp->device);
> 1421 if (attr_mask & IB_QP_STATE)
> 1422 status = ocrdma_qp_state_change(qp, attr->qp_state,
> &old_qps);
> 1423 /* if new and previous states are same hw doesn't need to
> 1424 * know about it.
> 1425 */
> 1426 if (status < 0)
>
> This check is never true. Based on the comment then the check should
> be:
> if (status == 1)
> return SOMETHING;
>
> 1427 return status;
> 1428 status = ocrdma_mbx_modify_qp(dev, qp, attr, attr_mask);
> 1429
> 1430 return status;
> 1431 }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body
> of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-02-26 17:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-07 11:07 RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter Dan Carpenter
[not found] ` <20140207110728.GA6470-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
2014-02-07 11:51 ` Devesh Sharma
-- strict thread matches above, loose matches on Subject: below --
2015-02-26 12:30 Dan Carpenter
2015-02-26 17:54 ` Devesh Sharma
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox