Re: [PATCH rdma-next v3-v6] Add OPA extended LID support

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3558 bytes --]

On Tue, Aug 15, 2017 at 03:13:35PM -0700, Don Hiatt wrote:
>
>
> On 8/15/2017 1:40 PM, Don Hiatt wrote:
> >
> >
> > On 8/14/2017 1:12 PM, Don Hiatt wrote:
> > > On 8/14/2017 11:36 AM, Don Hiatt wrote:
> > > >
> > > >
> > > > On 8/14/2017 11:17 AM, Don Hiatt wrote:
> > > > > This patch series primarily increases sizes of variables that hold
> > > > > lid values from 16 to 32 bits. Additionally, it adds a check in
> > > > > the IB mad stack to verify a properly formatted MAD when OPA
> > > > > extended LIDs are used.
> > > > >
> > > > > Signed-off-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > ---
> > > > >
> > > > > This is an incremental patch to move from v3 of the 'Add OPA
> > > > > extended LID support' to v6 of the series.
> > > > > Changes from v5:
> > > > > ---------------
> > > > > * Fixed typo in WARN_ON_ONCE usage in helper functions.
> > > > > * Actually return be16 in ib_lid_be16() helper function.
> > > >
> > > > Sorry, this was meant to go to my email as a test, not to the
> > > > list. My tests are still running so please
> > > > hold off on this until I confirm.
> > > >
> > > All test completed fine. I think we're good to go.
> > >
> > > Leon, if I missed anything else please let me know.
> > >
> > > Thanks,
> > >
> > > don
> > >
> > I did not get this email respond but saw it on the mailing list so
> > pasted it in to respond)
> >
> > >Yeah, you should fix the function below too.
> > >The whole extended LID series did enormous mess with all these
> > lid/slid/dlid.
> > >
> > > 88 static inline bool opa_is_extended_lid(u32 dlid, u32 slid)
> > > 89 {
> > > 90         if ((be32_to_cpu(dlid) >=
> > > 91              be16_to_cpu(IB_MULTICAST_LID_BASE)) ||
> > > 92             (be32_to_cpu(slid) >=
> > > 93              be16_to_cpu(IB_MULTICAST_LID_BASE)))
> > > 94                 return true;
> > > 95         else
> > > 96                 return false;
> > > 97 }
> > >
> > >It will help a lot, if you break this patch to small steps:
> > >1. Fix existing annotation errors.
> > >2. Change (rename) the ib_lid/ib_slid functions.
> > >3. Add WARN_ON.
> > >
> > >Right now, we have potential breakage of compatibility between
> > >big-endian vs. little-endian systems.
> > >
> > >Please run smatch and sparse checkers before LID patches and after to
> > >know what else you should fix.
> > >
> > > Thanks
> >
> > This patch series sat on the mailing list for over two months since the
> > your last
> > request. It then got merged in and an incremental patch was asked for.
> >
> > I've been trying to address your concerns but since this patch is going
> > in as an
> > incremental  patch I do not see how breaking it up as requested is
> > required.
> >
> > If Doug would like to pull the entire series then I'll break the
> > patchfor es up.
> >
> > As of now, with this patch the endian-ness issues have been resolved, or
> > are you
> > saying they are not?
> >
> >
> I ran sparse/smatch and the endian issues are fixed. The only smatch
> warnings in this
> patch are for OPA record types. Once this incremental patch is accepted I'll
> follow
> up to specifically address the OPA record type warnings.

Please don't wait and address it now.

Thanks.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]