[ofa-general] Re: [PATCH] Fix credit loop checking
Oren Kladnitsky
orenk at dev.mellanox.co.il
Sun Mar 15 11:26:50 PDT 2009
Dale Purdy wrote:
> ibdiagnet/ibdiagui and ibdmchk assume that up/down routing is being
> used if it is able to find roots, whether the root are correct or not.
> If it finds roots it does its credit loop validation based on whether
> the up/down rules are followed instead of doing a rigorous credit loop
> check. If the roots aren't correct, this can lead to determination of
> credit loop problems on topologies that don't have problems. ibdmchk
> has an option to supply one's own root_guids file to override this if
> you actually are using up/down and have your own roots that were used
> to set up the routing, but ibdiagnet/ibdiagui doesn't. In any case
> there shouldn't be assumptions about what the routing algorithm is, or
> what the roots are when checking for credit loops.
>
> Add a -u option to ibdiagnet/ibdiagui. Change ibdiagnet/ibdiagui and
> ibdmchk to only do the simple up/down rule checking against roots when
> the -u option is used. Otherwise the full credit loop check is done.
>
> Signed-off-by: Dale Purdy <purdy at sgi.com>
> ---
> ibdiag/doc/ibdiagnet.pod | 11 ++++++++++-
> ibdiag/doc/ibdiagui.pod | 2 +-
> ibdiag/src/ibdebug.tcl | 11 +++++++----
> ibdiag/src/ibdebug_if.tcl | 10 +++++++---
> ibdm/src/osm_check.cpp | 22 +++++++++-------------
> 5 files changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/ibdiag/doc/ibdiagnet.pod b/ibdiag/doc/ibdiagnet.pod
> index d2cf460..cdc78ed 100644
> --- a/ibdiag/doc/ibdiagnet.pod
> +++ b/ibdiag/doc/ibdiagnet.pod
> @@ -4,7 +4,7 @@ B<ibdiagnet - IB diagnostic net>
>
> =head1 SYNOPSYS
>
> -ibdiagnet [-c <count>] [-v] [-r] [-o <out-dir>]
> +ibdiagnet [-c <count>] [-v] [-r] [-u] [-o <out-dir>]
> [-t <topo-file>] [-s <sys-name>] [-i <dev-index>] [-p <port-num>] [-wt]
> [-pm] [-pc] [-P <<PM>=<Value>>]
> [-lw <1x|4x|12x>] [-ls <2.5|5|10>]
> @@ -135,6 +135,15 @@ Provides a report of the fabric qualities
>
> =back
>
> +=item B<-u> :
> +
> +=over
> +
> +=item
> +Credit loop check based on UpDown rules
> +
> +=back
> +
> =item B<-t <topo-file>> :
>
> =over
> diff --git a/ibdiag/doc/ibdiagui.pod b/ibdiag/doc/ibdiagui.pod
> index 4e0250f..86a2df9 100644
> --- a/ibdiag/doc/ibdiagui.pod
> +++ b/ibdiag/doc/ibdiagui.pod
> @@ -4,7 +4,7 @@ B<ibdiagui - IB Diagnostic GUI>
>
> =head1 SYNOPSYS
>
> -ibdiagui [-c <count>] [-v] [-r] [-o <out-dir>]
> +ibdiagui [-c <count>] [-v] [-r] [-u] [-o <out-dir>]
> [-t <topo-file>] [-s <sys-name>] [-i <dev-index>] [-p <port-num>]
> [-pm] [-pc] [-P <PM counter>=<Trash Limit>]
> [-lw <1x|4x|12x>] [-ls <2.5|5|10>]
> diff --git a/ibdiag/src/ibdebug.tcl b/ibdiag/src/ibdebug.tcl
> index 3a464f2..04a8566 100644
> --- a/ibdiag/src/ibdebug.tcl
> +++ b/ibdiag/src/ibdebug.tcl
> @@ -4391,10 +4391,13 @@ proc DumpFabQualities {} {
> inform "-I-ibdiagnet:check.credit.loops.header"
>
> # report credit loops
> - ibdmCalcMinHopTables $fabric
> - set roots [ibdmFindRootNodesByMinHop $fabric]
> - # just flush out any logs
> - set report [ibdmGetAndClearInternalLog]
> + set roots ""
> + if { [info exists G(argv:updown)] } {
> + ibdmCalcMinHopTables $fabric
> + set roots [ibdmFindRootNodesByMinHop $fabric]
> + # just flush out any logs
> + set report [ibdmGetAndClearInternalLog]
> + }
> if {[llength $roots]} {
> inform "-I-reporting:found.roots" $roots
> ibdmReportNonUpDownCa2CaPaths $fabric $roots
> diff --git a/ibdiag/src/ibdebug_if.tcl b/ibdiag/src/ibdebug_if.tcl
> index 21afc45..cf1b571 100644
> --- a/ibdiag/src/ibdebug_if.tcl
> +++ b/ibdiag/src/ibdebug_if.tcl
> @@ -163,6 +163,10 @@ proc SetInfoArgv {} {
> -t,param "topo-file"
> -t,desc "Specifies the topology file name"
>
> + -u,name "updown"
> + -u,desc "Indicates that UpDown rule checking should be done against automaticly determined roots"
> + -u,arglen 0
> +
> -v,name "verbose"
> -v,desc "Instructs the tool to run in verbose mode"
> -v,arglen 0
> @@ -322,8 +326,8 @@ proc SetToolsFlags {} {
> array set TOOLS_FLAGS {
> ibping "(n|l|d) . c w v o . t s i p "
> ibdiagpath "(n|l|d) . c v o smp . t s i p . pm pc P . lw ls sl ."
> - ibdiagui " c v r o . t s i p . pm pc P . lw ls ."
> - ibdiagnet " c v r o . t s i p wt . pm pc P . lw ls . skip load_db csv"
> + ibdiagui " c v r u o . t s i p . pm pc P . lw ls ."
> + ibdiagnet " c v r u o . t s i p wt . pm pc P . lw ls . skip load_db csv"
> ibcfg "(n|l|d) (c|q) . t s i p o"
> ibmad "(m) (a) (n|l|d) . t s i p o ; (q) a"
> ibsac "(m) (a) k . t s i p o ; (q) a"
> @@ -2535,7 +2539,7 @@ proc showHelpPage { args } {
> Hop-count information:
> maximal hop-count, an example path, and a hop-count histogram
> All CA-to-CA paths traced
> - Credit loop report
> + Credit loop report (based on UpDown if -u option is provided)
> mgid-mlid-HCAs matching table
> Note: In case the IB fabric includes only one CA, then CA-to-CA paths are not
> reported.
> diff --git a/ibdm/src/osm_check.cpp b/ibdm/src/osm_check.cpp
> index 1c18c1c..09a3637 100644
> --- a/ibdm/src/osm_check.cpp
> +++ b/ibdm/src/osm_check.cpp
> @@ -552,21 +552,17 @@ int main (int argc, char **argv) {
> list <IBNode *> rootNodes;
> int anyErr = 0;
>
> - if (RootsFileName.size())
> - {
> - if (TopoFile.size())
> - {
> - rootNodes = ParseRootNodeNamesFile(&fabric, RootsFileName);
> - }
> - else
> - {
> - rootNodes = ParseRootNodeGuidsFile(&fabric, RootsFileName);
> - }
> - }
> - else
> - {
> + if (UseUpDown) {
> + if (RootsFileName.size()) {
> + if (TopoFile.size()) {
> + rootNodes = ParseRootNodeNamesFile(&fabric, RootsFileName);
> + } else {
> + rootNodes = ParseRootNodeGuidsFile(&fabric, RootsFileName);
> + }
> + } else {
> rootNodes = SubnMgtFindRootNodesByMinHop(&fabric);
> }
> + }
>
> if (!rootNodes.empty()) {
> cout << "-I- Recognized " << rootNodes.size() << " root nodes:" << endl;
>
Hi Dale,
The above patch broke osm_check.cpp .
Attached is the patch I used for this purpose.
Thanks,
Oren.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Provide-a-new-flag-u-updn-to-ibdiagnet-and-ibdmch.patch
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20090315/653dda70/attachment.ksh>
More information about the general
mailing list