On Thu, Oct 9, 2008 at 1:20 PM, Al Chu <[EMAIL PROTECTED]> wrote: > 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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.
That was just an easy way to say all ports for those who didn't know 255 was a magic number. That's not for any specific hardware. The original assumption was that all PMAs supported AllPortSelect option but that turned out not to be the case so I started adding the emulation for a single port (which was the one case I was aware of). Clearly, there are other cases. -- Hal > > 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 >> >> >> >> > [EMAIL PROTECTED] >> >> >> >> > Computer Scientist >> >> >> >> > High Performance Systems Division >> >> >> >> > Lawrence Livermore National Laboratory >> >> >> >> > >> >> >> >> > _______________________________________________ >> >> >> >> > 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 >> >> >> >> > >> >> >> >> >> >> >> >> 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 >> >> >> > [EMAIL PROTECTED] >> >> >> > Computer Scientist >> >> >> > High Performance Systems Division >> >> >> > Lawrence Livermore National Laboratory >> >> >> > >> >> >> > >> >> >> >> >> > -- >> >> > Albert Chu >> >> > [EMAIL PROTECTED] >> >> > Computer Scientist >> >> > High Performance Systems Division >> >> > Lawrence Livermore National Laboratory >> >> > >> >> > >> >> >> > -- >> > Albert Chu >> > [EMAIL PROTECTED] >> > Computer Scientist >> > High Performance Systems Division >> > Lawrence Livermore National Laboratory >> > >> > >> > -- > Albert Chu > [EMAIL PROTECTED] > Computer Scientist > High Performance Systems Division > Lawrence Livermore National Laboratory > > _______________________________________________ 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
