Hi Irena,
On Tue, Jul 20, 2010 at 8:08 AM, Irena Kruchkovsky <[email protected]> wrote:
> 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.
I'm hoping we can get ibstat.c to be the same in Windows and Linux and
constrain the changes to umad.cpp/libibumad. In Linux, the rate file
is not written when the link width is not valid but I've never seen
that happen. Does that occur in Windows or is this to handle it if it
were to occur ?
Anyhow, here's what I would propose:
1. Just add Gbps to Rate display (without the N/A for 0 case) in ibstat.c:
<change:>
printf("%sRate: %d\n", pre, port->rate);
<to>
printf("%sRate: %d Gbps\n", pre, port->rate);
2. changing umad.cpp as you originally proposed setting the port->rate
to 0 for unknown link widths (yes, I flip flopped on this after
looking at it again...)
Thanks.
-- Hal
>
> 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
