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 <[email protected]>
> >> ---
> >>  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 ()...?

Sasha

> 
> > 
> >>            if (aggregate)
> >>                    aggregate_perfcounters();
> >> -          else
> >> +          else 
> >>                    mad_dump_perfcounters(buf, sizeof buf, pc, sizeof pc);
> >>    } else {
> >>            if (!(cap_mask & 0x200)) /* 1.2 errata: bit 9 is extended 
> >> counter support */
> >> @@ -500,6 +509,9 @@ main(int argc, char **argv)
> >>  
> >>  do_reset:
> >>  
> >> +  if (!extended && (cap_mask & 0x1000))
> >> +          mask |= (1<<16); /* reset portxmitwait */
> > 
> > The counter mask can be specified by user in command line to reset only
> > certain counters. The code above will add XmitWait unconditionally
> > regardless to user wishes. So shouldn't it be something like:
> > 
> >     if (argc <= 2 && !extended && (cap_mask & 0x1000))
> >             mask |= (1<<16); /* reset portxmitwait */
> > 
> 
> i agree. 
> 
> 
> > 
> > Sasha
> > 
> >> +          
> >>    if (all_ports_loop || (loop_ports && (all_ports || port == ALL_PORTS))) 
> >> {
> >>            for (i = start_port; i <= num_ports; i++)
> >>                    reset_counters(extended, timeout, mask, &portid, i);
> >> diff --git a/libibmad/src/gs.c b/libibmad/src/gs.c
> >> index d350c0d..30f00fb 100644
> >> --- a/libibmad/src/gs.c
> >> +++ b/libibmad/src/gs.c
> >> @@ -142,6 +142,8 @@ performance_reset_via(void *rcvbuf, ib_portid_t *dest, 
> >> int port, unsigned mask,
> >>    /* Same for attribute IDs */
> >>    mad_set_field(rcvbuf, 0, IB_PC_PORT_SELECT_F, port);
> >>    mad_set_field(rcvbuf, 0, IB_PC_COUNTER_SELECT_F, mask);
> >> +  mask = mask >> 16;
> >> +  mad_set_field(rcvbuf, 0, IB_PC_COUNTER_SELECT2_F, mask);
> >>    rpc.attr.mod = 0;
> >>    rpc.timeout = timeout;
> >>    rpc.datasz = IB_PC_DATA_SZ;
> >> -- 
> >> 1.5.5
> >>
> 
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to