Hello Hal, The original purpose of returning a 0 was to handle cases in which we get invalid port width (i.e. a constant that is not supported). According to your example, is it OK if the returned value for this case will be “-1” instead of 0? The ibstat.c file will be changed back to be the same in Windows and Linux, but in this case the -1 (or any other error value) will be printed as is in the rate, if such a case occurs.
Thank you, Irena -----Original Message----- From: Hal Rosenstock [mailto:[email protected]] Sent: Tuesday, July 13, 2010 8:42 PM To: Alex Naslednikov Cc: Irena Kruchkovsky; [email protected] Subject: Re: [ofw] [Patch] [UMAD][Tools] On 7/13/10, Hal Rosenstock <[email protected]> wrote: > On Tue, Jul 13, 2010 at 11:45 AM, Alex Naslednikov <[email protected]> > wrote: >> It can happen when the subnet isn't configured yet, for example. > > Isn't rate based on link speed and width ? Is one of those values > invalid (reserved) ? What am I missing ? IMO ibstat.c should be the same in Windows and Linux. I'm not sure what the underlying issue is in generating a rate of 0. The rate might not be "real" in that the active (link speed/width) components are only valid in certain port states but the value should never be 0 AFAIT. If a change needs to be made, it would be best to isolate it to the Windows libibumad IMO. For comparison purposes, in Linux, libibumad parses the rate file which is written in the kernel by sysfs.c as follows: struct ib_port_attr attr; char *speed = ""; int rate; ssize_t ret; ret = ib_query_port(p->ibdev, p->port_num, &attr); if (ret) return ret; switch (attr.active_speed) { case 2: speed = " DDR"; break; case 4: speed = " QDR"; break; } rate = 25 * ib_width_enum_to_int(attr.active_width) * attr.active_speed; if (rate < 0) return -EINVAL; return sprintf(buf, "%d%s Gb/sec (%dX%s)\n", rate / 10, rate % 10 ? ".5" : "", ib_width_enum_to_int(attr.active_width), speed); where: 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; } } I didn't look to see how Windows libibumad handles this. -- Hal > > -- Hal > >> >> -----Original Message----- >> From: [email protected] >> [mailto:[email protected]] On Behalf Of Hal Rosenstock >> Sent: Tuesday, July 13, 2010 6:41 PM >> To: Irena Kruchkovsky >> Cc: [email protected] >> Subject: Re: [ofw] [Patch] [UMAD][Tools] >> >> On Tue, Jul 13, 2010 at 10:53 AM, Irena Kruchkovsky <[email protected]> >> wrote: >>> This patch fixes the wrong rate calculation within ibstat tool. >>> >>> Signed-off by: Irena Kruchkovsky ([email protected]) >>> >>> CR: Alexander Naslednikov (xalex at mellanox.co.il) >>> >>> >>> >>> Index: D:/Windows/MLNX_VPI/tools/infiniband-diags/src/ibstat.c >>> >>> =================================================================== >>> >>> --- D:/Windows/MLNX_VPI/tools/infiniband-diags/src/ibstat.c (revision >>> 6077) >>> >>> +++ D:/Windows/MLNX_VPI/tools/infiniband-diags/src/ibstat.c >>> (revision 6092) >>> >>> @@ -117,7 +117,15 @@ >>> >>> printf("%sPhysical state: %s\n", pre, >>> >>> (unsigned)port->state <= >>> >>> 7 ? port_phy_state_str[port->phys_state] : >>> "???"); >>> >>> - printf("%sRate: %d\n", pre, port->rate); >>> >>> + if (port->rate != 0) >> >> Just wondering: >> When is the rate 0 returned by umad ? Is that a valid case ? >> >> -- Hal >> >>> >>> + { >>> >>> + printf("%sRate: %d Gbps\n", pre, >>> +port->rate); >>> >>> + } >>> >>> + else >>> >>> + { >>> >>> + printf("%sRate: N\\A\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); >>> >>> Index: D:/Windows/MLNX_VPI/ulp/libibumad/src/umad.cpp >>> >>> =================================================================== >>> >>> --- D:/Windows/MLNX_VPI/ulp/libibumad/src/umad.cpp >>> (revision >>> 6077) >>> >>> +++ D:/Windows/MLNX_VPI/ulp/libibumad/src/umad.cpp (revision >>> +++ 6092) >>> >>> @@ -141,9 +141,25 @@ >>> >>> port->sm_sl = attr.sm_sl; >>> >>> port->state = attr.state; >>> >>> port->phys_state = attr.phys_state; >>> >>> - port->rate = attr.active_speed; >>> >>> port->capmask = attr.port_cap_flags; >>> >>> >>> >>> + switch (attr.active_width){ >>> >>> + case 1: >>> >>> + port->rate = (unsigned) >>> (2.5*attr.active_speed); >>> >>> + break; >>> >>> + case 2: >>> >>> + port->rate = >>> 10*attr.active_speed; //2.5*4 >>> >>> + break; >>> >>> + case 4: >>> >>> + port->rate = >>> 20*attr.active_speed; //2.5*8 >>> >>> + break; >>> >>> + case 8: >>> >>> + port->rate = >>> 30*attr.active_speed; //2.5*12 >>> >>> + break; >>> >>> + default: >>> >>> + port->rate = 0; >>> >>> + } >>> >>> + >>> >>> // Assume GID 0 contains port GUID and gid prefix >>> >>> ret = ibv_query_gid(context, (uint8_t) port->portnum, >>> 0, &gid); >>> >>> if (ret != 0) { >>> >>> _______________________________________________ >>> ofw mailing list >>> [email protected] >>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw >>> >> _______________________________________________ >> ofw mailing list >> [email protected] >> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw >> > _______________________________________________ ofw mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
