On 10/28/2017 6:42 AM, Thomas Bogendoerfer wrote:
>> I must be missing something as to what is going on in this scenario.
I think I see what's going on now. The -EINVAL in kernel sysfs/rate_show
causes error to either open or read of file. I had checked that an empty
file is parsed correctly but didn't consider an error on open or read.
>> sysfs.c:rate_show is inconsistent as it paves over an invalid speed
>> setting that to SDR but does not pave over invalid width returning
>> -EINVAL but this comment is in another "direction".
> I thought about going in the other direction by making the speed/width unknown case
> somehow visible in rate_show(). When I checked all other drivers only mlx5 stand out
> so I decided to only change mlx5. But I make a change to let rate_show() print out,
> that is currently has no rate for the port.
I wasn't proposing to make that visible in rate_show() but rather
pointing out the inconsistency in changing unknown speeds to SDR but
unknown widths are error. There were comments on not having to set
"random" numbers for speed/width. If so, shouldn't these be handled
consistently here ? Would setting -EINVAL when not one of recognized
speeds cause an issue (rather than setting to SDR) ?
IMO a more future proof implementation is not to have error for
ib_width_enum_to_int but have unknown widths default to 1x similar to
how unknown speeds default to SDR:
include/rdma/ib_verbs.h:
static inline int ib_width_enum_to_int(enum ib_port_width width)
{
switch (width) {
case IB_WIDTH_1X: return 1;
case IB_WIDTH_4X: return 4;
case IB_WIDTH_8X: return 8;
case IB_WIDTH_12X: return 12;
default: return 1;
}
}
For example, 2x link widths have been added to IBA spec.
This is alternative approach to libibumad/ibstat patches for invalid
link width as it's not set when QSFP is not plugged into port.
-- Hal
--
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
