[ofw] [Patch] [UMAD][Tools]
Hal Rosenstock
hal.rosenstock at gmail.com
Tue Jul 20 13:03:48 PDT 2010
Hi Irena,
On Tue, Jul 20, 2010 at 8:08 AM, Irena Kruchkovsky <irena at mellanox.co.il> 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:hal.rosenstock at gmail.com]
> Sent: Tuesday, July 13, 2010 8:42 PM
> To: Alex Naslednikov
> Cc: Irena Kruchkovsky; ofw at lists.openfabrics.org
> Subject: Re: [ofw] [Patch] [UMAD][Tools]
>
> On 7/13/10, Hal Rosenstock <hal.rosenstock at gmail.com> wrote:
>> On Tue, Jul 13, 2010 at 11:45 AM, Alex Naslednikov <xalex at mellanox.co.il>
>> 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: ofw-bounces at lists.openfabrics.org
>>> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Hal Rosenstock
>>> Sent: Tuesday, July 13, 2010 6:41 PM
>>> To: Irena Kruchkovsky
>>> Cc: ofw at lists.openfabrics.org
>>> Subject: Re: [ofw] [Patch] [UMAD][Tools]
>>>
>>> On Tue, Jul 13, 2010 at 10:53 AM, Irena Kruchkovsky <irena at mellanox.co.il>
>>> wrote:
>>>> This patch fixes the wrong rate calculation within ibstat tool.
>>>>
>>>> Signed-off by: Irena Kruchkovsky (irena at mellanox.co.il)
>>>>
>>>> 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
>>>> ofw at lists.openfabrics.org
>>>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>>>>
>>> _______________________________________________
>>> ofw mailing list
>>> ofw at lists.openfabrics.org
>>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>>>
>>
>
More information about the ofw
mailing list