[ofa-general] [infiniband-diags] add --loop_ports option to perfquery

[Al Chu]

On Thu, 2008-10-09 at 13:13 -0400, Hal Rosenstock wrote:
> Hi Al,
> 
> On Thu, Oct 9, 2008 at 12:25 PM, Al Chu <chu11 at llnl.gov> wrote:
> > Hey Hal,
> >
> > On Thu, 2008-10-09 at 08:37 -0400, Hal Rosenstock wrote:
> >> Hi again Al,
> >>
> >> On Wed, Oct 8, 2008 at 6:45 PM, Al Chu <chu11 at llnl.gov> wrote:
> >> > Hey Hal,
> >> >
> >> > On Wed, 2008-10-08 at 09:44 -0400, Hal Rosenstock wrote:
> >> >> Hi Al,
> >> >>
> >> >> On Wed, Oct 8, 2008 at 6:26 AM, Al Chu <chu11 at llnl.gov> wrote:
> >> >> > Hey Hal,
> >> >> >
> >> >> > On Wed, 2008-10-08 at 07:03 -0400, Hal Rosenstock wrote:
> >> >> >> Al,
> >> >> >>
> >> >> >> On Tue, Oct 7, 2008 at 6:54 PM, Al Chu <chu11 at llnl.gov> wrote:
> >> >> >> > Hey Sasha,
> >> >> >> >
> >> >> >> > We have a switch here that does not report the AllPortSelect flag as a
> >> >> >> > capability.  It's pretty annoying typing each port on the switch or
> >> >> >> > always having to script around this one oddball switch we have.  So I
> >> >> >> > added an option --loop_ports for perfquery.  If you want to do something
> >> >> >> > to all the ports on the CA/Switch, but AllPortSelect isn't available, it
> >> >> >> > loops through all the available ports instead.
> >> >> >>
> >> >> >> Why not add simulated AllPortSelect for multiple ports rather than add
> >> >> >> another perquery option for this ?
> >> >> >
> >> >> > I did try that, and it did seem to work for the switches we had.  But
> >> >> > when I read the IB spec, it said something to the affect that if a
> >> >> > system doesn't support AllPortSelect, setting the PortSelect field to
> >> >> > 0xFF was undefined behavior.
> >> >>
> >> >> I was suggesting that the emulation support (when AllPortSelect is not
> >> >> supported) be enhanced for multiple ports and work on both CAs and all
> >> >> switches. The one difference is one response for AllPortSelect
> >> >> (whether emulated or not) v. many responses for port loop.
> >> >
> >> > Oh.  I thought you were referring the the workaround "simulation" that
> >> > was in the original code.  But you're referring to aggregating the
> >> > data/output make it look like AllPortSelect was supported.  I'll put
> >> > this on the TODO.
> >>
> >> So it seems that the reason for adding an additional option for this
> >> is that the lack of this support ? Are there any other uses ?
> >
> > I made the --loop_ports option b/c I just didn't want to change the
> > default behavior perfquery.  But we could easily make it an automatic if
> > the AllPortsSelect flag isn't supported.
> >
> > Thinking about it, there is a bit of a subtlety in the command line
> > options and expectations.
> >
> > If a user inputs a port of '255', to me this means that the user wants
> > to do an AllPortSelect and we should error out if AllPortSelect isn't
> > supported.
> >
> > If a user inputs the '-a' option, it suggests that they want perfquery
> > to operate on every port, suggesting that we could automatically loop if
> > they don't input --loop_ports.
> 
> Yes, there is some redundancy now with port 255 and -a option (both do
> the same thing) and they could be made subtly different as you
> indicate.
> 
> In the case of query rather than reset, we are still left with the
> question of whether to return 1 aggregated response or 1
> response/port. Do we need to support both ? AllPortsSelect has this as
> aggregated.

I agree that we should aggregate it.  One of the reasons I made --
loop_ports an option was b/c I didn't want to change default behavior.
If we're going to change the default behavior and remove the option,
then I need to aggregate it.

> I'm also not sure that the loop ports option is needed.
> 
> IMO all that was needed was to loop on the ports when all ports is not
> supported by the PMA and aggregate the counters and then nothing else
> needs to change.

Should we remove the previous workaround in the code?

                if (allports == 1)
                        pc[1] = ALL_PORTS;      /* fake PortSelect */

Presumably we wouldn't need this anymore.  But I'm not sure if this was
a specific need for some specific hardware or something.

Al

> -- Hal
> 
> > Al
> >
> >> -- Hal
> >>
> >> >>
> >> >> >> > There was already a workaround in the tool for a CA that did not support
> >> >> >> > the AllPortSelect flag.  I get the feeling the workaround may have been
> >> >> >> > for a specific hardware, so I kept the workaround in there.
> >> >> >>
> >> >> >> > Al
> >> >> >> >
> >> >> >> > --
> >> >> >> > Albert Chu
> >> >> >> > chu11 at llnl.gov
> >> >> >> > Computer Scientist
> >> >> >> > High Performance Systems Division
> >> >> >> > Lawrence Livermore National Laboratory
> >> >> >> >
> >> >> >> > _______________________________________________
> >> >> >> > 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
> >> >> >> >
> >> >> >>
> >> >> >> There are also 2 for loops which are not correct for some switches:
> >> >> >> for (i = 1; i <= num_ports; i++)
> >> >> >
> >> >> > I guess I've never seen a switch that doesn't go from 1 to num_ports.
> >> >> > Is there something else I need to handle?
> >> >>
> >> >> Yes, per the spec, enhanced SP0 supports PortCounters. All your
> >> >> switches likely support AllPortSelect so it's not an issue there.
> >> >
> >> > Ok I see now.  Wasn't aware of it.  I'll get a patch together.
> >> >
> >> > Thanks,
> >> > Al
> >> >
> >> >> -- Hal
> >> >>
> >> >> > Al
> >> >> >
> >> >> >> -- Hal
> >> >> >>
> >> >> > --
> >> >> > Albert Chu
> >> >> > chu11 at llnl.gov
> >> >> > Computer Scientist
> >> >> > High Performance Systems Division
> >> >> > Lawrence Livermore National Laboratory
> >> >> >
> >> >> >
> >> >>
> >> > --
> >> > Albert Chu
> >> > chu11 at llnl.gov
> >> > Computer Scientist
> >> > High Performance Systems Division
> >> > Lawrence Livermore National Laboratory
> >> >
> >> >
> >>
> > --
> > Albert Chu
> > chu11 at llnl.gov
> > Computer Scientist
> > High Performance Systems Division
> > Lawrence Livermore National Laboratory
> >
> >
> 
-- 
Albert Chu
chu11 at llnl.gov
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory