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.
Thanks
Attachment:
signature.asc
Description: PGP signature
