Re: [PATCH infiniband-diags] ibstat.c: If port is 1x SDR, rate is 2.5 rather than 2 Gbps

Hal Rosenstock
2017-02-08T20:47:47Z

On 2/8/2017 3:25 PM, ira.weiny wrote:
> On Mon, Feb 06, 2017 at 08:03:12AM -0500, Hal Rosenstock wrote:
>> From: Oded Nissan <odedni-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> Signed-off-by: Oded Nissan <odedni-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Shouldn't this be fixed at a lower level?

That requires changing the libibumad ABI as rate is an int rather than
float in umad_port and I didn't think it was worth it for this trivial
issue. sysfs uses a * 10 approach and then rate / 10, rate % 10 ? ".5" :
"" but that can't be used here without changing many more things.

> I think the problem is that libibumad is masking the information. AFAICT the
> kernel reports "2.5" in sysfs.
>
> I'm concerned that there are probably lots of other places in the software
> stack which have this bug because libibumad is masking the rate.

I don't think there are any places in open sources.

> If the value of "2" was at least documented to be special and meant "2.5" I
> think this could be an ok patch...
>
> Something like
>
> else if (port->rate == UMAD_2_5_RATE)
> printf("%sRate: 2.5\n", pre);
> else
> printf("%sRate: %d\n", pre, port->rate);

Is it worth it to tie infiniband-diags to some version of libibumad
where UMAD_2_5_RATE is defined like this ?

-- Hal

> Ira
>
>> ---
>> diff --git a/src/ibstat.c b/src/ibstat.c
>> index 37f2361..2d980c4 100644
>> --- a/src/ibstat.c
>> +++ b/src/ibstat.c
>> @@ -182,8 +182,10 @@ static int port_dump(umad_port_t * port, int alone)
>> 7 ? port_phy_state_str[port->phys_state] : "???");
>> if (is_fdr10(port))
>> printf("%sRate: %d (FDR10)\n", pre, port->rate);
>> - else
>> + else if (port->rate != 2) /* 1x SDR */
>> printf("%sRate: %d\n", pre, port->rate);
>> + else
>> + printf("%sRate: 2.5\n", pre);
>> printf("%sBase lid: %d\n", pre, port->base_lid);
>> printf("%sLMC: %d\n", pre, port->lmc);
>> printf("%sSM lid: %d\n", pre, port->sm_lid);