Skip to contentSkip to conversation

IB/mlx5: Support IB_WR_REG_SIG_MR

bug report Dan Carpenter 3 participants 5 messages Dec 9, 2016 – Dec 12, 2016 3 messages not in the archive InfiniBand

lore
Share & tools
lore
#0 lore raw

Hello Sagi Grimberg,

The patch e6631814fb3a: "IB/mlx5: Support IB_WR_REG_SIG_MR" from Feb 23, 2014, leads to the following static checker warning:

	net/rds/ib_recv.c:672 rds_ib_send_ack()
	warn: 'ret' can be either negative or positive

drivers/infiniband/hw/mlx5/qp.c
3614 static int set_psv_wr(struct ib_sig_domain *domain,
3615 u32 psv_idx, void **seg, int *size)
3616 {
3617 struct mlx5_seg_set_psv *psv_seg = *seg;
3618

  3619          memset(psv_seg, 0, sizeof(*psv_seg));
  3620          psv_seg->psv_num = cpu_to_be32(psv_idx);
  3621          switch (domain->sig_type) {
  3622          case IB_SIG_TYPE_NONE:

3623 break;
3624 case IB_SIG_TYPE_T10_DIF:
3625 psv_seg->transient_sig = cpu_to_be32(domain->sig.dif.bg << 16 |
3626 domain->sig.dif.app_tag);
3627 psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag);
3628 break;
3629 default:
3630 pr_err("Bad signature type given.\n");
3631 return 1;

Why do we return 1 here instead of a negative error code?

  3632          }
  3633  
  3634          *seg += sizeof(*psv_seg);
  3635          *size += sizeof(*psv_seg) / 16;
  3636  
  3637          return 0;
  3638  }

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

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

#2 lore raw
Quoted 28 lines · Dan Carpenter

Hello Sagi Grimberg,

The patch e6631814fb3a: "IB/mlx5: Support IB_WR_REG_SIG_MR" from Feb 23, 2014, leads to the following static checker warning:

	net/rds/ib_recv.c:672 rds_ib_send_ack()
	warn: 'ret' can be either negative or positive

drivers/infiniband/hw/mlx5/qp.c
3614 static int set_psv_wr(struct ib_sig_domain *domain,
3615 u32 psv_idx, void **seg, int *size)
3616 {
3617 struct mlx5_seg_set_psv *psv_seg = *seg;
3618

  3619          memset(psv_seg, 0, sizeof(*psv_seg));
  3620          psv_seg->psv_num = cpu_to_be32(psv_idx);
  3621          switch (domain->sig_type) {
  3622          case IB_SIG_TYPE_NONE:

3623 break;
3624 case IB_SIG_TYPE_T10_DIF:
3625 psv_seg->transient_sig = cpu_to_be32(domain->sig.dif.bg << 16 |
3626 domain->sig.dif.app_tag);
3627 psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag);
3628 break;
3629 default:
3630 pr_err("Bad signature type given.\n");
3631 return 1;

Quoted 1 line

Why do we return 1 here instead of a negative error code?

Yes, probably better to return something like EINVAL, instead of 1. do you want us to handle it?

Thanks

Quoted 15 lines
  3632          }
  3633
  3634          *seg += sizeof(*psv_seg);
  3635          *size += sizeof(*psv_seg) / 16;
  3636
  3637          return 0;
  3638  }

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

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

#4 lore raw
Quoted 9 lines · Leon Romanovsky
Quoted 9 lines
  3624          case IB_SIG_TYPE_T10_DIF:
  3625                  psv_seg->transient_sig = cpu_to_be32(domain->sig.dif.bg << 16 |
  3626                                                       domain->sig.dif.app_tag);
  3627                  psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag);
  3628                  break;
  3629          default:
  3630                  pr_err("Bad signature type given.\n");
  3631                  return 1;
Quoted 5 lines
Quoted 1 line

Why do we return 1 here instead of a negative error code?

Yes, probably better to return something like EINVAL, instead of 1. do you want us to handle it?

Yes, please.

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

#5 lore raw
Quoted 10 lines · Dan Carpenter
Quoted 9 lines · Leon Romanovsky
Quoted 9 lines
  3624          case IB_SIG_TYPE_T10_DIF:
  3625                  psv_seg->transient_sig = cpu_to_be32(domain->sig.dif.bg << 16 |
  3626                                                       domain->sig.dif.app_tag);
  3627                  psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag);
  3628                  break;
  3629          default:
  3630                  pr_err("Bad signature type given.\n");
  3631                  return 1;
Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Why do we return 1 here instead of a negative error code?

Yes, probably better to return something like EINVAL, instead of 1. do you want us to handle it?

Yes, please.

Thanks,
We will send it as part of our fixes series for 4.10.

Quoted 4 lines

regards,
dan carpenter

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

#7 lore raw
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
  3624          case IB_SIG_TYPE_T10_DIF:
  3625                  psv_seg->transient_sig = cpu_to_be32(domain->sig.dif.bg << 16 |
  3626                                                       domain->sig.dif.app_tag);
  3627                  psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag);
  3628                  break;
  3629          default:
  3630                  pr_err("Bad signature type given.\n");
  3631                  return 1;

Why do we return 1 here instead of a negative error code?

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Yes, probably better to return something like EINVAL, instead of 1. do you want us to handle it?

Yes, please.

Thanks,
We will send it as part of our fixes series for 4.10.

Thanks Leon. Thanks Dan for reporting.
--
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