Re: [PATCH infiniband-diags] ibdiag_common.c: Disambiguate link speed when QDR and could be FDR10

Hal Rosenstock
2016-12-08T16:19:00Z

On 12/8/2016 11:10 AM, ira.weiny wrote:
> On Wed, Nov 23, 2016 at 11:47:59AM -0500, Hal Rosenstock wrote:
>> Rather than printing "Could be FDR10", print
>> "Found link at QDR but expected speed is FDR10" for this case.
>>
>> Confusion is with words "Could be" since both
>> QDR and FDR10 are both 10.0 Gbps.
>
> I'm not sure that I like this change. I understand that the "speed" may be the
> same but generally FDR10 was supposed to be "better".

FDR10 is better than QDR.

> For all the other speed
> checks the string is "Could be" and I know I have used that to grep for any
> links which are not at "maximum" (Or "best supported") speed.

I was trying to minimize the change rather than changing every
occurrence of "Could be"

> For this reason, I don't think it is going to generally be good for customers.
>
> Was there a particular reason you needed to change this?

Customer complaint that wording was confusing to them and proposed
wording was acceptable to eliminate their confusion with this.

Where do we go from here ?

-- Hal

> Ira
>
>
>>
>> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> ---
>> src/ibdiag_common.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/ibdiag_common.c b/src/ibdiag_common.c
>> index 2a1e971..fa9a294 100644
>> --- a/src/ibdiag_common.c
>> +++ b/src/ibdiag_common.c
>> @@ -814,8 +814,14 @@ check_ext_speed:
>>
>> check_fdr10_active:
>> if ((mad_get_field(port->ext_info, 0,
>> - IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0)
>> - snprintf(speed_msg, msg_size, "Could be FDR10");
>> + IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0) {
>> + /* Special case QDR to try to avoid confusion with FDR10 */
>> + if (mad_get_field(port->info, 0, IB_PORT_LINK_SPEED_ACTIVE_F) == 4)
>> + snprintf(speed_msg, msg_size,
>> + "Found link at QDR but expected speed is FDR10");
>> + else
>> + snprintf(speed_msg, msg_size, "Could be FDR10");
>> + }
>> }
>>
>> int vsnprint_field(char *buf, size_t n, enum MAD_FIELDS f, int spacing,
>