***SPAM*** Re: [ofa-general] [PATCH v3 2/2] infiniband-diags support PortXmitWait get and set

[Eli Dorfman]

On Thu, Jan 15, 2009 at 3:58 PM, Sasha Khapyorsky <sashak at voltaire.com> wrote:
> On 08:53 Thu 15 Jan     , Eli Dorfman (Voltaire) wrote:
>> Sasha Khapyorsky wrote:
>> > Hi Eli,
>> >
>> > On 16:56 Tue 13 Jan     , Eli Dorfman (Voltaire) wrote:
>> >> support PortXmitWait get and set
>> >> fix syntax error
>> >>
>> >> Signed-off-by: Eli Dorfman <elid at voltaire.com>
>> >> ---
>> >>  infiniband-diags/src/perfquery.c |   14 +++++++++++++-
>> >>  libibmad/src/gs.c                |    2 ++
>> >>  2 files changed, 15 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/infiniband-diags/src/perfquery.c b/infiniband-diags/src/perfquery.c
>> >> index 41a8b74..5e5b3ed 100644
>> >> --- a/infiniband-diags/src/perfquery.c
>> >> +++ b/infiniband-diags/src/perfquery.c
>> >> @@ -67,6 +67,7 @@ struct perf_count {
>> >>    uint32_t rcvdata;
>> >>    uint32_t xmtpkts;
>> >>    uint32_t rcvpkts;
>> >> +  uint32_t xmtwait;
>> >>  };
>> >>
>> >>  struct perf_count_ext {
>> >> @@ -209,6 +210,8 @@ static void aggregate_perfcounters(void)
>> >>    aggregate_32bit(&perf_count.xmtpkts, val);
>> >>    mad_decode_field(pc, IB_PC_RCV_PKTS_F, &val);
>> >>    aggregate_32bit(&perf_count.rcvpkts, val);
>> >> +        mad_decode_field(pc, IB_PC_XMT_WAIT_F, &val);
>> >
>> > Please use tab character for indentation.
>> >
>> >> +  aggregate_32bit(&perf_count.xmtwait, val);
>> >>  }
>> >>
>> >>  static void output_aggregate_perfcounters(ib_portid_t *portid)
>> >> @@ -235,6 +238,7 @@ static void output_aggregate_perfcounters(ib_portid_t *portid)
>> >>    mad_encode_field(pc, IB_PC_RCV_BYTES_F, &perf_count.rcvdata);
>> >>    mad_encode_field(pc, IB_PC_XMT_PKTS_F, &perf_count.xmtpkts);
>> >>    mad_encode_field(pc, IB_PC_RCV_PKTS_F, &perf_count.rcvpkts);
>> >> +  mad_encode_field(pc, IB_PC_XMT_WAIT_F, &perf_count.xmtwait);
>> >>
>> >>    mad_dump_perfcounters(buf, sizeof buf, pc, sizeof pc);
>> >>
>> >> @@ -298,9 +302,14 @@ static void dump_perfcounters(int extended, int timeout, uint16_t cap_mask, ib_p
>> >>    if (extended != 1) {
>> >>            if (!port_performance_query(pc, portid, port, timeout))
>> >>                    IBERROR("perfquery");
>> >> +          if (!(cap_mask & 0x1000)) {
>> >> +                  /* if PortCounters:PortXmitWait not suppported clear this counter */
>> >> +                  perf_count.xmtwait = 0;
>> >> +                  mad_encode_field(pc, IB_PC_XMT_WAIT_F, &perf_count.xmtwait);
>> >> +          }
>> >
>> > Is this a good thing to hide the reported value? We could to not show
>> > XmitWait at all in case when it is not supported, or to show it as was
>> > reported by port and not "to lie" about zero value.
>>
>> This is a good idea, but it requires to pass the mask to the mad_dump_perfcounters
>> which is a generic function.
>> I preferred to leave it like this than making a special case for perfcounters dump.
>
> So I'm removing this if ()...?

Without changing the perfcounter dump a device that does not support
XmitWait might return some garbage value.
I preferred to show 0 instead.