Re: [PATCH for-next 12/27] IB/core: Change port_attr.sm_lid from 16 to 32 bits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/8/2017 11:57 PM, Leon Romanovsky wrote:
On Tue, Aug 08, 2017 at 09:49:17AM -0700, Don Hiatt wrote:

On 8/8/2017 9:35 AM, Weiny, Ira wrote:
On Sun, Aug 06, 2017 at 11:22:17AM +0300, Leon Romanovsky wrote:
On Sun, Aug 06, 2017 at 11:18:57AM +0300, Leon Romanovsky wrote:
On Fri, Aug 04, 2017 at 01:53:21PM -0700, Dennis Dalessandro wrote:
From: Dasaratharaman Chandramouli
<dasaratharaman.chandramouli@xxxxxxxxx>

sm_lid field in struct ib_port_attr 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@xxxxxxxxx>
Reviewed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
Signed-off-by: Don Hiatt <don.hiatt@xxxxxxxxx>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxx>
---
   drivers/infiniband/core/uverbs_cmd.c |    8 +++++---
   include/rdma/ib_verbs.h              |    2 +-
   2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c
b/drivers/infiniband/core/uverbs_cmd.c
index 7ef74b0..38dce45 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -275,11 +275,13 @@ ssize_t ib_uverbs_query_port(struct
ib_uverbs_file *file,
   	resp.bad_pkey_cntr   = attr.bad_pkey_cntr;
   	resp.qkey_viol_cntr  = attr.qkey_viol_cntr;
   	resp.pkey_tbl_len    = attr.pkey_tbl_len;
-	resp.sm_lid 	     = attr.sm_lid;
-	if (rdma_cap_opa_ah(ib_dev, cmd.port_num))
+	if (rdma_cap_opa_ah(ib_dev, cmd.port_num)) {
   		resp.lid  = OPA_TO_IB_UCAST_LID(attr.lid);
-	else
+		resp.sm_lid  = OPA_TO_IB_UCAST_LID(attr.sm_lid);
+	} else {
   		resp.lid     = (u16)attr.lid;
+		resp.sm_lid  = (u16)attr.sm_lid;
I see that lid is already has casting from u32 to u16 and now it is sm_lid.
Do we have more elegant way to achieve that? And comment for future
developers can be good too.
I see now, you changed lid in patch 11. The same comment there.
To be clear on that point: NAK on *current* implementation.

At least, it should be done in separate function, with comment explains why
are you doing and why it is safe to cast from higher value to lower value and
proper checks that you are not loosing information when you are casting.

A comment is reasonable.  However, I'm not sure a separate function is needed.  With the current interface there is no way to pass all the information through.

Software which requires the use of these values will need to know to get them from other sources (OPA MAD queries for the SM LID for example.)

Ira

Hi Leon,

I had already created a function to do the cast in the 'IB/core: Change
wc.slid from 16 to 32 bits' patch
but I did not apply it here. Sorry about that.  My plan is to rename the
functions to ib_lid_{spu16,be16}
and introduce them in this patch with a comment in the git log that these
functions exist. I will also add
a comment stating why we aren't losing any information.
These functions should be introduced before "casting patches".

Thanks

Hi Leon,

I re-submitted these patches as v4.

Thank you.

don

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux