re: [SCSI] cxgb4i: add support for T5 adapter

re: [SCSI] cxgb4i: add support for T5 adapter

[Dan Carpenter]

Hello Karen Xie,

The patch 3bd3e8bf6250: "[SCSI] cxgb4i: add support for T5 adapter"
from May 29, 2013, leads to the following Sparse warning:

drivers/scsi/cxgbi/cxgb4i/cxgb4i.c:284:29: warning: incorrect type in assignment (different base types)
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c:284:29:    expected restricted __be64 [usertype] params
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c:284:29:    got restricted __be32 [usertype] <noident>

drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
   271                  struct cpl_t5_act_open_req *req =
   272                                  (struct cpl_t5_act_open_req *)skb->head;
   273  
   274                  req = (struct cpl_t5_act_open_req *)skb->head;
   275  
   276                  INIT_TP_WR(req, 0);
   277                  OPCODE_TID(req) = cpu_to_be32(MK_OPCODE_TID(CPL_ACT_OPEN_REQ,
   278                                          qid_atid));
   279                  req->local_port = csk->saddr.sin_port;
   280                  req->peer_port = csk->daddr.sin_port;
   281                  req->local_ip = csk->saddr.sin_addr.s_addr;
   282                  req->peer_ip = csk->daddr.sin_addr.s_addr;
   283                  req->opt0 = cpu_to_be64(opt0);
   284                  req->params = cpu_to_be32(select_ntuple(csk->cdev, csk->l2t));
                        ^^^^^^^^^^^
For cpl_t5_act_open_req ->params is a __be64.  For other types it is a
__be32 so I suspect this is a copy and paste error.

   285                  opt2 |= 1 << 31;
   286                  req->opt2 = cpu_to_be32(opt2);

Here is an article on how to use Sparse for endian bugs:
http://lwn.net/Articles/205624/

Also I'm on the topic, I had another concern later in the file:

drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
  1629          rpl = (struct cpl_act_establish *)skb->data;
  1630          opc = rpl->ot.opcode;
  1631          log_debug(1 << CXGBI_DBG_TOE,
  1632                  "cdev %p, opcode 0x%x(0x%x,0x%x), skb %p.\n",
  1633                   cdev, opc, rpl->ot.opcode_tid, ntohl(rpl->ot.opcode_tid), skb);
  1634          if (cxgb4i_cplhandlers[opc])
  1635                  cxgb4i_cplhandlers[opc](cdev, skb);
                                           ^^^
Since "opc" comes from skb->data shouldn't we check that it is less than
ARRAY_SIZE(cxgb4i_cplhandlers) (239 elements)?  Another solution that's
used in drivers/net/ethernet/chelsio/cxgb/cpl5_cmd.h would be to define
the array as having 256 elements.

  1636          else {
  1637                  pr_err("No handler for opcode 0x%x.\n", opc);
  1638                  __kfree_skb(skb);
  1639          }

regards,
dan carpenter