linux-rdma / opensm

Other
66 stars 36 forks source link

Travis validation #13

Closed nmorey closed 5 years ago

nmorey commented 5 years ago

@hnrose I cannot enable -Werror for clang due to this error: osm_vendor_ibumad.c: In function ‘osm_vendor_open_port’: osm_vendor_ibumad.c:745:56: error: passing argument 2 of ‘umad_get_ca_portguids’ from incompatible pointer type [-Werror=incompatible-pointer-types] if ((r = umad_get_ca_portguids(p_vend->ca_names[ca], portguids,

You should be able to enable it once you fix it

nmorey commented 5 years ago

@hnrose Do you know if osmtest can be run over ibsim ? It feels a bit wrong to validate ibsim with opensm and opensm with ibsim but I don't see any other way to do this in travis. And the software is probably mature enough for it not to be an issue

hnrose commented 5 years ago

[nmorey wrote:] I cannot enable -Werror for clang due to this error: osm_vendor_ibumad.c: In function ‘osm_vendor_open_port’: osm_vendor_ibumad.c:745:56: error: passing argument 2 of ‘umad_get_ca_portguids’ from incompatible pointer type [-Werror=incompatible-pointer-types] if ((r = umad_get_ca_portguids(p_vend->ca_names[ca], portguids,

Is it correct to assume that you are using libibumad in rdma-core and it's compiles OK without -Werror=incompatible-pointer-types ?

hnrose commented 5 years ago

[nmorey wrote:] Do you know if osmtest can be run over ibsim ? It feels a bit wrong to validate ibsim with opensm and opensm with ibsim but I don't see any other way to do this in travis. And the software is probably mature enough for it not to be an issue

osmtest can't be run on ibsim because osmtest uses RMPP which is mostly not supported by the simulator except for single packet RMPP which is insufficient for osmtest. This has been a long time shortcoming of ibsim.

I agree it's a little weird to have the circular dependency to test ibsim. ibsim could also be exercised with some of the InfiniBand-diags but that is probably a similar dependency. More of InfiniBand-diags could run if opensm were running on the simulator. I need to think about this more.

nmorey commented 5 years ago

@hnrose Yes it compiles fine without -Werror. For some reason clang is being a real pain in the *** and cannot figure out that the fields are actually aligned.

If we can't use ibsim, we do not have to create a circular dependency anyway. Not sure how we can do useful testing in Travis though.

This PR still make sense to verify that new patches do not break compilation at least

hnrose commented 5 years ago

I'll run clang with -Werror and fix the issue but won't be able to get to this until mid next week.

hnrose commented 5 years ago

[nmorey wrote:] If we can't use ibsim, we do not have to create a circular dependency anyway. Not sure how we can do useful testing in Travis though.

This PR still make sense to verify that new patches do not break compilation at least

Yes checking that everything compiles is better than nothing ;-)

I will think more about testing even if it is primitive. It will at least put the structure in place to build on.

Thanks for your amazing efforts on this over the last few days!

hnrose commented 5 years ago

Manually merged.

Created PR #14 (under test) to attempt to fix clang complaint so -Werror can be used

hnrose commented 5 years ago

@nmorey Is libibumad from rdma-core being picked up ?

602osm_vendor_ibumad.c:746:56: error: incompatible pointer types passing '__be64 ' 603 (aka 'unsigned long long ') to parameter of type 'uint64_t ' 604 (aka 'unsigned long ') [-Werror,-Wincompatible-pointer-types] 605 ...if ((r = umad_get_ca_portguids(p_vend->ca_names[ca], &portguids[0], 606 ^~~~~ 607/usr/include/infiniband/umad.h:166:59: note: passing argument to parameter 608 'portguids' here 609int umad_get_ca_portguids(const char ca_name, uint64_t portguids, int max); 610 ^

This declaration of umad_get_ca_portguids looks to be from umad.h prior to rdma-core.

I also see clang complaints of packed member alignment issue with p_mad->trans_id