[ofw] [Patch] [UMAD][Tools]

Irena Kruchkovsky irena at mellanox.co.il
Tue Jul 20 05:08:13 PDT 2010


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: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