[ofa-general] Re: [PATCHv2] infiniband-diags/perfquery.c: Fix issues when checking PerfMgt:ClassPortInfo.CapabilityMask

[Hal Rosenstock]

On Sat, 2007-10-13 at 19:32 +0200, Sasha Khapyorsky wrote:
> On 06:30 Fri 12 Oct     , Hal Rosenstock wrote:
> > infiniband-diags/perfquery.c: Fix issues when checking
> > PerfMgt:ClassPortInfo.CapabilityMask
> > 
> > 1. bit 9, if we're counting from 0, will have mask of 0x200,
> >    not 0x100. mask of 0x100 will be for counter aggregation according
> >    to IBA 1.2.
> > 
> > 2. If capmask is 16 bit big-endian word, then we're looking
> >    at the wrong byte on x86, we must ntohs(*pc2) first.
> > 
> > 3. Also, change pointer dereference with memcpy,
> > e.g.:
> > 
> >            memcpy (&capmask, pc+2, sizeof(capmask));
> >            capmask = ntohs(capmask);
> > 
> > Those pointer dereferenes are royal pain on ia64 unless you can
> > guarantee what pc is always aligned properly.
> > 
> > Found-by: Max Matveev <makc at sgi.com>
> > 
> > Compile tested only
> > 
> > Signed-off-by: Hal Rosenstock <hal at xsigo.com>
> 
> Applied. Thanks.
> 
> I have the question below (not related directly to specific patch).
> 
> > 
> > diff --git a/infiniband-diags/src/perfquery.c b/infiniband-diags/src/perfquery.c
> > index 2ae3281..148e452 100644
> > --- a/infiniband-diags/src/perfquery.c
> > +++ b/infiniband-diags/src/perfquery.c
> > @@ -40,8 +40,9 @@
> >  #include <unistd.h>
> >  #include <stdarg.h>
> >  #include <getopt.h>
> > +#include <netinet/in.h>
> >  
> > -#define __BUILD_VERSION_TAG__ 1.2.1
> > +#define __BUILD_VERSION_TAG__ 1.2.2
> 
> What is the motivation of this change and in general what
> __BUILD_VERSION_TAG__ is supposed to show?

It predates me but I had been using it as an indicator of when changes
(major or minor) were made to the tool.

>  If it is just unique build
> version then I guess t would be better to use infiniband-diags version +
> git-describe sequence.

If a new release is generated for each change of this sort (which it
wasn't), then this is fine.

>  If it is per-tool "compat" string, then likely we
> don't need to change it each time when tools behavior is not changed.

It was a per tool thing but could be different depending on the
processes being used.

-- Hal

> 
> Sasha
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general