Re: [PATCH] infiniband-diags/perfquery.c: Fix extended counter reset mask

Re: [PATCH] infiniband-diags/perfquery.c: Fix extended counter reset mask

[Hal Rosenstock]

On Thu, Oct 29, 2009 at 9:17 PM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
>
> On 17:17 Sat 26 Sep     , Hal Rosenstock wrote:
>>
>> to not have any bits on for reserved components
>
> Why to care?

Because a PMA implementation which follows the IBA recommendation will
reject this so it's safer.

-- Hal

>>
>> Signed-off-by: Hal Rosenstock <hal.rosenstock@gmail.com>
>> ---
>> diff --git a/infiniband-diags/src/perfquery.c
>> b/infiniband-diags/src/perfquery.c
>> index d70af9e..5d4046b 100644
>> --- a/infiniband-diags/src/perfquery.c
>> +++ b/infiniband-diags/src/perfquery.c
>> @@ -91,6 +91,8 @@ struct perf_count perf_count =
>>      { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>>  struct perf_count_ext perf_count_ext = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>>
>> +int not_def_mask = 0;
>> +
>>  #define ALL_PORTS 0xFF
>>
>>  /* Notes: IB semantics is to cap counters if count has exceeded limits.
>> @@ -337,8 +339,10 @@ static void reset_counters(int extended, int timeout,
>> int mask,
>>                                             IB_GSI_PORT_COUNTERS,
>> srcport))
>>                          IBERROR("perf reset");
>>          } else {
>> -                if (!performance_reset_via(pc, portid, port, mask,
>> timeout,
>> -                                           IB_GSI_PORT_COUNTERS_EXT,
>> srcport))
>> +                if (!performance_reset_via(pc, portid, port,
>> +                                           not_def_mask ? mask : mask &
>> 0xff,
>> +                                           timeout,
>> IB_GSI_PORT_COUNTERS_EXT,
>> +                                           srcport))
>
> And if it is really needed please do it somehow without introducing yet
> another global flow switcher.
>
> Sasha
>
>>                          IBERROR("perf ext reset");
>>          }
>>  }
>> @@ -476,8 +480,10 @@ int main(int argc, char **argv)
>>
>>          if (argc > 1)
>>                  port = strtoul(argv[1], 0, 0);
>> -        if (argc > 2)
>> +        if (argc > 2) {
>>                  mask = strtoul(argv[2], 0, 0);
>> +                not_def_mask = 1;
>> +        }
>>
>>          srcport = mad_rpc_open_port(ibd_ca, ibd_ca_port, mgmt_classes,
>> 4);
>>          if (!srcport)
>>
>

Re: [PATCH] infiniband-diags/perfquery.c: Fix extended counter reset mask

[Hal Rosenstock]

On Fri, Oct 30, 2009 at 6:16 PM, Hal Rosenstock
<hal.rosenstock@gmail.com> wrote:
> On Thu, Oct 29, 2009 at 9:17 PM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
>>
>> On 17:17 Sat 26 Sep     , Hal Rosenstock wrote:
>>>
>>> to not have any bits on for reserved components
>>
>> Why to care?
>
> Because a PMA implementation which follows the IBA recommendation will
> reject this so it's safer.

There's an errata removing this so it's a don't care.

-- Hal

>
> -- Hal
>
>>>
>>> Signed-off-by: Hal Rosenstock <hal.rosenstock@gmail.com>
>>> ---
>>> diff --git a/infiniband-diags/src/perfquery.c
>>> b/infiniband-diags/src/perfquery.c
>>> index d70af9e..5d4046b 100644
>>> --- a/infiniband-diags/src/perfquery.c
>>> +++ b/infiniband-diags/src/perfquery.c
>>> @@ -91,6 +91,8 @@ struct perf_count perf_count =
>>>      { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>>>  struct perf_count_ext perf_count_ext = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
>>>
>>> +int not_def_mask = 0;
>>> +
>>>  #define ALL_PORTS 0xFF
>>>
>>>  /* Notes: IB semantics is to cap counters if count has exceeded limits.
>>> @@ -337,8 +339,10 @@ static void reset_counters(int extended, int timeout,
>>> int mask,
>>>                                             IB_GSI_PORT_COUNTERS,
>>> srcport))
>>>                          IBERROR("perf reset");
>>>          } else {
>>> -                if (!performance_reset_via(pc, portid, port, mask,
>>> timeout,
>>> -                                           IB_GSI_PORT_COUNTERS_EXT,
>>> srcport))
>>> +                if (!performance_reset_via(pc, portid, port,
>>> +                                           not_def_mask ? mask : mask &
>>> 0xff,
>>> +                                           timeout,
>>> IB_GSI_PORT_COUNTERS_EXT,
>>> +                                           srcport))
>>
>> And if it is really needed please do it somehow without introducing yet
>> another global flow switcher.
>>
>> Sasha
>>
>>>                          IBERROR("perf ext reset");
>>>          }
>>>  }
>>> @@ -476,8 +480,10 @@ int main(int argc, char **argv)
>>>
>>>          if (argc > 1)
>>>                  port = strtoul(argv[1], 0, 0);
>>> -        if (argc > 2)
>>> +        if (argc > 2) {
>>>                  mask = strtoul(argv[2], 0, 0);
>>> +                not_def_mask = 1;
>>> +        }
>>>
>>>          srcport = mad_rpc_open_port(ibd_ca, ibd_ca_port, mgmt_classes,
>>> 4);
>>>          if (!srcport)
>>>
>>
>

Re: [PATCH] infiniband-diags/perfquery.c: Fix extended counter reset mask

[Sasha Khapyorsky]

On 18:16 Fri 30 Oct     , Hal Rosenstock wrote:
> 
> Because a PMA implementation which follows the IBA recommendation will
> reject this

Hmm, where did you find such recommendation where stated that if
reserved bits are not '0' the request should be rejected (I would rather
expect ignoring of those values at all)?

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] infiniband-diags/perfquery.c: Fix extended counter reset mask

[Hal Rosenstock]

On Mon, Nov 2, 2009 at 9:03 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 18:16 Fri 30 Oct     , Hal Rosenstock wrote:
>>
>> Because a PMA implementation which follows the IBA recommendation will
>> reject this
>
> Hmm, where did you find such recommendation where stated that if
> reserved bits are not '0' the request should be rejected (I would rather
> expect ignoring of those values at all)?

I sent a subsequent email indicating there was an errata on this and
that these changes are needed.

-- Hal

>
> Sasha
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] infiniband-diags/perfquery.c: Fix extended counter reset mask

[Sasha Khapyorsky]

On 11:23 Mon 02 Nov     , Hal Rosenstock wrote:
> On Mon, Nov 2, 2009 at 9:03 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> > On 18:16 Fri 30 Oct     , Hal Rosenstock wrote:
> >>
> >> Because a PMA implementation which follows the IBA recommendation will
> >> reject this
> >
> > Hmm, where did you find such recommendation where stated that if
> > reserved bits are not '0' the request should be rejected (I would rather
> > expect ignoring of those values at all)?
> 
> I sent a subsequent email indicating there was an errata on this and
> that these changes are needed.

Let me understand correctly. Do you mean that proposed patch is not
needed?

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] infiniband-diags/perfquery.c: Fix extended counter reset mask

[Hal Rosenstock]

On Mon, Nov 2, 2009 at 12:13 PM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 11:23 Mon 02 Nov     , Hal Rosenstock wrote:
>> On Mon, Nov 2, 2009 at 9:03 AM, Sasha Khapyorsky <sashak-smomgflXvObQFizaE/u3fw@public.gmane.orgm> wrote:
>> > On 18:16 Fri 30 Oct     , Hal Rosenstock wrote:
>> >>
>> >> Because a PMA implementation which follows the IBA recommendation will
>> >> reject this
>> >
>> > Hmm, where did you find such recommendation where stated that if
>> > reserved bits are not '0' the request should be rejected (I would rather
>> > expect ignoring of those values at all)?
>>
>> I sent a subsequent email indicating there was an errata on this and
>> that these changes are needed.
>
> Let me understand correctly. Do you mean that proposed patch is not
> needed?

Yes, it's not strictly required and you typically nix those sorts of things.

-- Hal

>
> Sasha
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html