On Wed, Feb 08, 2017 at 03:47:47PM -0500, Hal Rosenstock wrote:
> 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@xxxxxxxxxxxx>
> >>
> >> Signed-off-by: Oded Nissan <odedni@xxxxxxxxxxxx>
> >> Signed-off-by: Hal Rosenstock <hal@xxxxxxxxxxxx>
> >
> > 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 ?
Fair enough... And who uses SDR anymore anyway... ;-)
But if we ever do get a fractional rate again I'd like to see this handled
better.
So I've taken the patch as we are unlikely to get a fractional rate at this
point.
Ira
>
> -- 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);
> >
--
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
