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
>
> Thank you.
>
> don
>
Attachment:
signature.asc
Description: PGP signature
