FYI - in general patches to ibstat and other ib-diags should be submitted to the linux-rdma mail list and Sasha (both are copied on the reply).
> 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 (ir...@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.rosenst...@gmail.com] > Sent: Tuesday, July 20, 2010 11:04 PM > To: Irena Kruchkovsky > Cc: Alex Naslednikov; o...@lists.openfabrics.org; Uri Habusha > Subject: Re: [ofw] [Patch] [UMAD][Tools] > > Hi Irena, > > On Tue, Jul 20, 2010 at 8:08 AM, Irena Kruchkovsky <ir...@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.rosenst...@gmail.com] > > Sent: Tuesday, July 13, 2010 8:42 PM > > To: Alex Naslednikov > > Cc: Irena Kruchkovsky; o...@lists.openfabrics.org > > Subject: Re: [ofw] [Patch] [UMAD][Tools] > > > > On 7/13/10, Hal Rosenstock <hal.rosenst...@gmail.com> wrote: > >> On Tue, Jul 13, 2010 at 11:45 AM, Alex Naslednikov > <xa...@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-boun...@lists.openfabrics.org > >>> [mailto:ofw-boun...@lists.openfabrics.org] On Behalf Of Hal Rosenstock > >>> Sent: Tuesday, July 13, 2010 6:41 PM > >>> To: Irena Kruchkovsky > >>> Cc: o...@lists.openfabrics.org > >>> Subject: Re: [ofw] [Patch] [UMAD][Tools] > >>> > >>> On Tue, Jul 13, 2010 at 10:53 AM, Irena Kruchkovsky > <ir...@mellanox.co.il> > >>> wrote: > >>>> This patch fixes the wrong rate calculation within ibstat tool. > >>>> > >>>> Signed-off by: Irena Kruchkovsky (ir...@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 > >>>> o...@lists.openfabrics.org > >>>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw > >>>> > >>> _______________________________________________ > >>> ofw mailing list > >>> o...@lists.openfabrics.org > >>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw > >>> > >> > > N�����r��y����b�X��ǧv�^�){.n�+����{��ٚ�{ay�ʇڙ�,j��f���h���z��w��� ���j:+v���w�j�m��������zZ+�����ݢj"��!�i