RDMA/ocrdma: Add driver for Emulex OneConnect IBoE RDMA adapter

DISCUSSION Dan Carpenter 2 participants 2 messages Feb 7, 2014 1 message not in the archive InfiniBand

Share & tools

Dan Carpenter

author

2014-02-07 11:07 UTC #0 lore raw

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;

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

Devesh Sharma

2014-02-07 11:51 UTC #2 lore raw

Hi Dan,

Thanks for the note, we will supply this patch to fix this on linux-rdma asap.

-Regards
Devesh

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Dan Carpenter
Sent: Friday, February 07, 2014 4:37 PM
To: Parav Pandit
Cc: [email protected]
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

  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

  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 [email protected] 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html