diff mbox

[rdma-next,v3,7/7] IB/core: Change wc.slid from 16 to 32 bits

Message ID 1496943469-44017-8-git-send-email-don.hiatt@intel.com (mailing list archive)
State Accepted
Headers show

Commit Message

Hiatt, Don June 8, 2017, 5:37 p.m. UTC 
slid field in struct ib_wc is increased to 32 bits.
This enables core components to use larger LIDs if needed.
The user ABI is unchanged and return 16 bit values when queried.

Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli@intel.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Don Hiatt <don.hiatt@intel.com>
---
 drivers/infiniband/core/cm.c            |  4 ++--
 drivers/infiniband/core/user_mad.c      |  2 +-
 drivers/infiniband/core/uverbs_cmd.c    | 10 +++++++---
 drivers/infiniband/hw/hfi1/mad.c        |  2 +-
 drivers/infiniband/hw/mlx4/mad.c        |  6 +++---
 drivers/infiniband/hw/mlx5/mad.c        |  2 +-
 drivers/infiniband/hw/mthca/mthca_cmd.c |  4 ++--
 drivers/infiniband/hw/mthca/mthca_mad.c |  2 +-
 drivers/infiniband/sw/rdmavt/cq.c       |  2 +-
 include/rdma/ib_verbs.h                 | 14 +++++++++++++-
 10 files changed, 32 insertions(+), 16 deletions(-)

Comments

Doug Ledford Aug. 8, 2017, 7:05 p.m. UTC | #1 
On Thu, 2017-06-08 at 13:37 -0400, Don Hiatt wrote:
> slid field in struct ib_wc is increased to 32 bits.
> This enables core components to use larger LIDs if needed.
> The user ABI is unchanged and return 16 bit values when queried.

Hi Don,

My comments here are not really about this particular patch, I just
grabbed it from the pile as one to respond to.  But my question is
this:

Multiple of these patch say the user ABI is unchanged and return 16 bit
values when queried.  What's your plan for a long term solution to
this?  Eventually, the user ABI will need to be extended to support
32bit LIDs, yes?  Or do you plan to try and hide this forever?

As for the kernel to user space driver API, I waffled a bit about
whether we shouldn't go ahead and return the 32bit value to the driver
and let it do the conversion to 16 bit for the user ABI, then decided
this method provides a cleaner means of supporting newer kernels with
older verbs OPA drivers and so didn't object, but it still leaves the
question about the long term plans here.
Hiatt, Don Aug. 9, 2017, 4:46 p.m. UTC | #2 
On 8/8/2017 12:05 PM, Doug Ledford wrote:
> On Thu, 2017-06-08 at 13:37 -0400, Don Hiatt wrote:
>> slid field in struct ib_wc is increased to 32 bits.
>> This enables core components to use larger LIDs if needed.
>> The user ABI is unchanged and return 16 bit values when queried.
> Hi Don,
>
> My comments here are not really about this particular patch, I just
> grabbed it from the pile as one to respond to.  But my question is
> this:
>
> Multiple of these patch say the user ABI is unchanged and return 16 bit
> values when queried.  What's your plan for a long term solution to
> this?  Eventually, the user ABI will need to be extended to support
> 32bit LIDs, yes?  Or do you plan to try and hide this forever?
>
> As for the kernel to user space driver API, I waffled a bit about
> whether we shouldn't go ahead and return the 32bit value to the driver
> and let it do the conversion to 16 bit for the user ABI, then decided
> this method provides a cleaner means of supporting newer kernels with
> older verbs OPA drivers and so didn't object, but it still leaves the
> question about the long term plans here.
>
Hi Doug,

Our long term plan is to use the new verbs 2 API for extending
the LID size. We do not plan to keep this hidden forever and plan to
use the new API once it's available.

Thanks.

don



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford Aug. 9, 2017, 5:18 p.m. UTC | #3 
On 8/9/2017 12:46 PM, Don Hiatt wrote:
> On 8/8/2017 12:05 PM, Doug Ledford wrote:
>> On Thu, 2017-06-08 at 13:37 -0400, Don Hiatt wrote:
>>> slid field in struct ib_wc is increased to 32 bits.
>>> This enables core components to use larger LIDs if needed.
>>> The user ABI is unchanged and return 16 bit values when queried.
>> Hi Don,
>>
>> My comments here are not really about this particular patch, I just
>> grabbed it from the pile as one to respond to.  But my question is
>> this:
>>
>> Multiple of these patch say the user ABI is unchanged and return 16 bit
>> values when queried.  What's your plan for a long term solution to
>> this?  Eventually, the user ABI will need to be extended to support
>> 32bit LIDs, yes?  Or do you plan to try and hide this forever?
>>
>> As for the kernel to user space driver API, I waffled a bit about
>> whether we shouldn't go ahead and return the 32bit value to the driver
>> and let it do the conversion to 16 bit for the user ABI, then decided
>> this method provides a cleaner means of supporting newer kernels with
>> older verbs OPA drivers and so didn't object, but it still leaves the
>> question about the long term plans here.
>>
> Hi Doug,
> 
> Our long term plan is to use the new verbs 2 API for extending
> the LID size. We do not plan to keep this hidden forever and plan to
> use the new API once it's available.

Good.  That's more or less what I wanted to hear ;-).  Thanks.
diff mbox

Patch

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 1844770..8525d65 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1703,7 +1703,7 @@  static void cm_process_routed_req(struct cm_req_msg *req_msg, struct ib_wc *wc)
 {
 	if (!cm_req_get_primary_subnet_local(req_msg)) {
 		if (req_msg->primary_local_lid == IB_LID_PERMISSIVE) {
-			req_msg->primary_local_lid = cpu_to_be16(wc->slid);
+			req_msg->primary_local_lid = ib_slid_be16(wc->slid);
 			cm_req_set_primary_sl(req_msg, wc->sl);
 		}
 
@@ -1713,7 +1713,7 @@  static void cm_process_routed_req(struct cm_req_msg *req_msg, struct ib_wc *wc)
 
 	if (!cm_req_get_alt_subnet_local(req_msg)) {
 		if (req_msg->alt_local_lid == IB_LID_PERMISSIVE) {
-			req_msg->alt_local_lid = cpu_to_be16(wc->slid);
+			req_msg->alt_local_lid = ib_slid_be16(wc->slid);
 			cm_req_set_alt_sl(req_msg, wc->sl);
 		}
 
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 200422d..0f8fc01 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -229,7 +229,7 @@  static void recv_handler(struct ib_mad_agent *agent,
 	packet->mad.hdr.status	   = 0;
 	packet->mad.hdr.length	   = hdr_size(file) + mad_recv_wc->mad_len;
 	packet->mad.hdr.qpn	   = cpu_to_be32(mad_recv_wc->wc->src_qp);
-	packet->mad.hdr.lid	   = cpu_to_be16(mad_recv_wc->wc->slid);
+	packet->mad.hdr.lid	   = ib_slid_be16(mad_recv_wc->wc->slid);
 	packet->mad.hdr.sl	   = mad_recv_wc->wc->sl;
 	packet->mad.hdr.path_bits  = mad_recv_wc->wc->dlid_path_bits;
 	packet->mad.hdr.pkey_index = mad_recv_wc->wc->pkey_index;
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 6964c05..45791ce 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1190,7 +1190,8 @@  ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	return ret ? ret : in_len;
 }
 
-static int copy_wc_to_user(void __user *dest, struct ib_wc *wc)
+static int copy_wc_to_user(struct ib_device *ib_dev, void __user *dest,
+			   struct ib_wc *wc)
 {
 	struct ib_uverbs_wc tmp;
 
@@ -1204,7 +1205,10 @@  static int copy_wc_to_user(void __user *dest, struct ib_wc *wc)
 	tmp.src_qp		= wc->src_qp;
 	tmp.wc_flags		= wc->wc_flags;
 	tmp.pkey_index		= wc->pkey_index;
-	tmp.slid		= wc->slid;
+	if (rdma_cap_opa_ah(ib_dev, wc->port_num))
+		tmp.slid  = OPA_TO_IB_UCAST_LID(wc->slid);
+	else
+		tmp.slid  = ib_slid_cpu16(wc->slid);
 	tmp.sl			= wc->sl;
 	tmp.dlid_path_bits	= wc->dlid_path_bits;
 	tmp.port_num		= wc->port_num;
@@ -1248,7 +1252,7 @@  ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 		if (!ret)
 			break;
 
-		ret = copy_wc_to_user(data_ptr, &wc);
+		ret = copy_wc_to_user(ib_dev, data_ptr, &wc);
 		if (ret)
 			goto out_put;
 
diff --git a/drivers/infiniband/hw/hfi1/mad.c b/drivers/infiniband/hw/hfi1/mad.c
index a081a98..6270bf3 100644
--- a/drivers/infiniband/hw/hfi1/mad.c
+++ b/drivers/infiniband/hw/hfi1/mad.c
@@ -4022,7 +4022,7 @@  static int opa_local_smp_check(struct hfi1_ibport *ibp,
 			       const struct ib_wc *in_wc)
 {
 	struct hfi1_pportdata *ppd = ppd_from_ibp(ibp);
-	u16 slid = in_wc->slid;
+	u16 slid = ib_slid_cpu16(in_wc->slid);
 	u16 pkey;
 
 	if (in_wc->pkey_index >= ARRAY_SIZE(ppd->pkeys))
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index cd5bfe1..c29ec65 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -169,7 +169,7 @@  int mlx4_MAD_IFC(struct mlx4_ib_dev *dev, int mad_ifc_flags,
 
 		op_modifier |= 0x4;
 
-		in_modifier |= in_wc->slid << 16;
+		in_modifier |= ib_slid_cpu16(in_wc->slid) << 16;
 	}
 
 	err = mlx4_cmd_box(dev->dev, inmailbox->dma, outmailbox->dma, in_modifier,
@@ -625,7 +625,7 @@  int mlx4_ib_send_to_slave(struct mlx4_ib_dev *dev, int slave, u8 port,
 		memcpy((char *)&tun_mad->hdr.slid_mac_47_32, &(wc->smac[4]), 2);
 	} else {
 		tun_mad->hdr.sl_vid = cpu_to_be16(((u16)(wc->sl)) << 12);
-		tun_mad->hdr.slid_mac_47_32 = cpu_to_be16(wc->slid);
+		tun_mad->hdr.slid_mac_47_32 = ib_slid_be16(wc->slid);
 	}
 
 	ib_dma_sync_single_for_device(&dev->ib_dev,
@@ -826,7 +826,7 @@  static int ib_process_mad(struct ib_device *ibdev, int mad_flags, u8 port_num,
 		}
 	}
 
-	slid = in_wc ? in_wc->slid : be16_to_cpu(IB_LID_PERMISSIVE);
+	slid = in_wc ? ib_slid_cpu16(in_wc->slid) : be16_to_cpu(IB_LID_PERMISSIVE);
 
 	if (in_mad->mad_hdr.method == IB_MGMT_METHOD_TRAP && slid == 0) {
 		forward_trap(to_mdev(ibdev), port_num, in_mad);
diff --git a/drivers/infiniband/hw/mlx5/mad.c b/drivers/infiniband/hw/mlx5/mad.c
index f1b56de6..f1a406c 100644
--- a/drivers/infiniband/hw/mlx5/mad.c
+++ b/drivers/infiniband/hw/mlx5/mad.c
@@ -78,7 +78,7 @@  static int process_mad(struct ib_device *ibdev, int mad_flags, u8 port_num,
 	u16 slid;
 	int err;
 
-	slid = in_wc ? in_wc->slid : be16_to_cpu(IB_LID_PERMISSIVE);
+	slid = in_wc ? ib_slid_cpu16(in_wc->slid) : be16_to_cpu(IB_LID_PERMISSIVE);
 
 	if (in_mad->mad_hdr.method == IB_MGMT_METHOD_TRAP && slid == 0)
 		return IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_CONSUMED;
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index 9d83a53..e19ae0b 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -1921,7 +1921,7 @@  int mthca_MAD_IFC(struct mthca_dev *dev, int ignore_mkey, int ignore_bkey,
 			(in_wc->wc_flags & IB_WC_GRH ? 0x80 : 0);
 		MTHCA_PUT(inbox, val,               MAD_IFC_G_PATH_OFFSET);
 
-		MTHCA_PUT(inbox, in_wc->slid,       MAD_IFC_RLID_OFFSET);
+		MTHCA_PUT(inbox, ib_slid_cpu16(in_wc->slid), MAD_IFC_RLID_OFFSET);
 		MTHCA_PUT(inbox, in_wc->pkey_index, MAD_IFC_PKEY_OFFSET);
 
 		if (in_grh)
@@ -1929,7 +1929,7 @@  int mthca_MAD_IFC(struct mthca_dev *dev, int ignore_mkey, int ignore_bkey,
 
 		op_modifier |= 0x4;
 
-		in_modifier |= in_wc->slid << 16;
+		in_modifier |= ib_slid_cpu16(in_wc->slid) << 16;
 	}
 
 	err = mthca_cmd_box(dev, inmailbox->dma, outmailbox->dma,
diff --git a/drivers/infiniband/hw/mthca/mthca_mad.c b/drivers/infiniband/hw/mthca/mthca_mad.c
index 617531f..a9caada 100644
--- a/drivers/infiniband/hw/mthca/mthca_mad.c
+++ b/drivers/infiniband/hw/mthca/mthca_mad.c
@@ -205,7 +205,7 @@  int mthca_process_mad(struct ib_device *ibdev,
 		      u16 *out_mad_pkey_index)
 {
 	int err;
-	u16 slid = in_wc ? in_wc->slid : be16_to_cpu(IB_LID_PERMISSIVE);
+	u16 slid = in_wc ? ib_slid_cpu16(in_wc->slid) : be16_to_cpu(IB_LID_PERMISSIVE);
 	u16 prev_lid = 0;
 	struct ib_port_attr pattr;
 	const struct ib_mad *in_mad = (const struct ib_mad *)in;
diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c
index 0ae2ff8..0335a3d 100644
--- a/drivers/infiniband/sw/rdmavt/cq.c
+++ b/drivers/infiniband/sw/rdmavt/cq.c
@@ -107,7 +107,7 @@  void rvt_cq_enter(struct rvt_cq *cq, struct ib_wc *entry, bool solicited)
 		wc->uqueue[head].src_qp = entry->src_qp;
 		wc->uqueue[head].wc_flags = entry->wc_flags;
 		wc->uqueue[head].pkey_index = entry->pkey_index;
-		wc->uqueue[head].slid = entry->slid;
+		wc->uqueue[head].slid = ib_slid_cpu16(entry->slid);
 		wc->uqueue[head].sl = entry->sl;
 		wc->uqueue[head].dlid_path_bits = entry->dlid_path_bits;
 		wc->uqueue[head].port_num = entry->port_num;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index f88aeb6..cf57b00 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -950,7 +950,7 @@  struct ib_wc {
 	u32			src_qp;
 	int			wc_flags;
 	u16			pkey_index;
-	u16			slid;
+	u32			slid;
 	u8			sl;
 	u8			dlid_path_bits;
 	u8			port_num;	/* valid only for DR SMPs on switches */
@@ -3643,4 +3643,16 @@  static inline enum rdma_ah_attr_type rdma_ah_find_type(struct ib_device *dev,
 	else
 		return RDMA_AH_ATTR_TYPE_IB;
 }
+
+/* Return slid in 16bit CPU encoding */
+static inline u16 ib_slid_cpu16(u32 slid)
+{
+	return (u16)slid;
+}
+
+/* Return slid in 16bit BE encoding */
+static inline u16 ib_slid_be16(u32 slid)
+{
+	return cpu_to_be16((u16)slid);
+}
 #endif /* IB_VERBS_H */