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