[ofw] [Patch] [UMAD][Tools]
Irena Kruchkovsky
irena at mellanox.co.il
Wed Jul 21 23:23:19 PDT 2010
Hello,
In fact this case should not happen. It was defined, as you said, to handle it if it were to occur.
This patch fixes the wrong rate calculation within ibstat tool.
Signed-off by: Irena Kruchkovsky (irena at mellanox.co.il)
CR: XaleX
Index: D:/Windows/MLNX_VPI/tools/infiniband-diags/src/ibstat.c
===================================================================
--- D:/Windows/MLNX_VPI/tools/infiniband-diags/src/ibstat.c (revision 5005)
+++ D:/Windows/MLNX_VPI/tools/infiniband-diags/src/ibstat.c (revision 6168)
@@ -117,7 +117,7 @@
printf("%sPhysical state: %s\n", pre,
(unsigned)port->state <=
7 ? port_phy_state_str[port->phys_state] : "???");
- printf("%sRate: %d\n", pre, port->rate);
+ printf("%sRate: %d Gbps\n", pre, port->rate);
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 5765)
+++ 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) {
-----Original Message-----
From: Hal Rosenstock [mailto:hal.rosenstock at gmail.com]
Sent: Tuesday, July 20, 2010 11:04 PM
To: Irena Kruchkovsky
Cc: Alex Naslednikov; ofw at lists.openfabrics.org; Uri Habusha
Subject: Re: [ofw] [Patch] [UMAD][Tools]
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
>>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ibstat.patch
Type: application/octet-stream
Size: 1775 bytes
Desc: ibstat.patch
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20100722/bc210e53/attachment.obj>
More information about the ofw
mailing list