Sasha Khapyorsky wrote:
Hi Tim, Ira,
On 08:58 Fri 23 May , Timothy A. Meier wrote:
Following Hals advice, authorization is based on the umad permissions.
I will send some more comments about this method later today. But
basically still think that some things could be broken and that it is
not really trivial to separate in this way wrong usage from desired
behavior reliably (with some approximation it is possible of course).
The intent is simply to provide a consistent and
non-silent fail mechanism.
OTOH I fully agree with yours and Ira's arguments about this - 'Silent'
fails are bad. I thought about how to solve this and and started to run
diag perl scripts from unprivileged account in various conditions (cache
file exists or not, cache dir is readable or not, etc.).
First thing I saw was that even on bad usage most scripts return 0. Then
I found that on many failures return status is not checked or ignored
and program return 0. I did those two patches (below) and up to now it
works fine for me (but likely I didn't cover everything). What do you
say?
I think this patch is fine, and helps solve the improper "usage" issue.
(btw - should we prefer the "adapter" spelling over "adaptor"?)
My patch was addressing non-authorized use. Our philosophy was to not allow
"any" sort of functionality (even help) if not authorized. Fail, and provide
a reason/code.
So rather than go through each perl script to see if the proper thing is done
(return code is checked, error msg provided, terminate, etc.) each time a
privileged function is invoked, we just do it at the beginning of the script,
using a common (consistent) function call ( auth_check() ).
I don't know if this is the desired behavior, but it would have caught a few
problems we have encountered with "silent" failures that produce misleading
results. It would also catch any future (unauthorized) scripting issues.
On 5-23, I submitted a patch which adds an auth_check() function to the common
perl module. I agree, the implementation is non-ideal, but it is probably
sufficient for the vast majority of installations.
If you think the concept of an auth_check() function is desirable/acceptable,
then I will pursue fixing the implementation in a more universal way.
Sasha
From cbbc155996c9f6efe91b78f055a643809b997468 Mon Sep 17 00:00:00 2001
From: root <[EMAIL PROTECTED]>
Date: Sat, 24 May 2008 11:04:08 +0300
Subject: [PATCH] infiniband-diags/scripts/*.pl: exit 2 on usage errors
Add non-zero exit status (2) on usage errors for perl scripts.
Signed-off-by: root <[EMAIL PROTECTED]>
---
infiniband-diags/scripts/check_lft_balance.pl | 2 +-
infiniband-diags/scripts/ibfindnodesusing.pl | 2 +-
infiniband-diags/scripts/ibidsverify.pl | 2 +-
infiniband-diags/scripts/iblinkinfo.pl | 2 +-
infiniband-diags/scripts/ibprintca.pl | 2 +-
infiniband-diags/scripts/ibprintrt.pl | 2 +-
infiniband-diags/scripts/ibprintswitch.pl | 2 +-
infiniband-diags/scripts/ibqueryerrors.pl | 2 +-
infiniband-diags/scripts/ibswportwatch.pl | 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/infiniband-diags/scripts/check_lft_balance.pl
b/infiniband-diags/scripts/check_lft_balance.pl
index 66f5f0f..b0f0fef 100755
--- a/infiniband-diags/scripts/check_lft_balance.pl
+++ b/infiniband-diags/scripts/check_lft_balance.pl
@@ -70,7 +70,7 @@ sub usage
print "Usage: $prog [-R -v]\n";
print " -R recalculate all cached information\n";
print " -v verbose output\n";
- exit 0;
+ exit 2;
}
sub is_port_up
diff --git a/infiniband-diags/scripts/ibfindnodesusing.pl
b/infiniband-diags/scripts/ibfindnodesusing.pl
index 1bf0987..71656b3 100755
--- a/infiniband-diags/scripts/ibfindnodesusing.pl
+++ b/infiniband-diags/scripts/ibfindnodesusing.pl
@@ -80,7 +80,7 @@ sub usage_and_exit
print " -R Recalculate ibnetdiscover information\n";
print " -C <ca_name> use selected Channel Adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibidsverify.pl
b/infiniband-diags/scripts/ibidsverify.pl
index de78e6b..1a236c8 100755
--- a/infiniband-diags/scripts/ibidsverify.pl
+++ b/infiniband-diags/scripts/ibidsverify.pl
@@ -46,7 +46,7 @@ sub usage_and_exit
print " -h This help message\n";
print
" -R Recalculate ibnetdiscover information (Default is to reuse ibnetdiscover
output)\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/iblinkinfo.pl
b/infiniband-diags/scripts/iblinkinfo.pl
index a195474..a7a3df5 100755
--- a/infiniband-diags/scripts/iblinkinfo.pl
+++ b/infiniband-diags/scripts/iblinkinfo.pl
@@ -62,7 +62,7 @@ sub usage_and_exit
print " -C <ca_name> use selected Channel Adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
print " -g print port guids instead of node guids\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibprintca.pl
b/infiniband-diags/scripts/ibprintca.pl
index 38b4330..0baea0b 100755
--- a/infiniband-diags/scripts/ibprintca.pl
+++ b/infiniband-diags/scripts/ibprintca.pl
@@ -51,7 +51,7 @@ sub usage_and_exit
print " -l list cas\n";
print " -C <ca_name> use selected channel adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibprintrt.pl
b/infiniband-diags/scripts/ibprintrt.pl
index 86dcb64..0b3db19 100755
--- a/infiniband-diags/scripts/ibprintrt.pl
+++ b/infiniband-diags/scripts/ibprintrt.pl
@@ -51,7 +51,7 @@ sub usage_and_exit
print " -l list rts\n";
print " -C <ca_name> use selected channel adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibprintswitch.pl
b/infiniband-diags/scripts/ibprintswitch.pl
index 6712201..c7377a9 100755
--- a/infiniband-diags/scripts/ibprintswitch.pl
+++ b/infiniband-diags/scripts/ibprintswitch.pl
@@ -50,7 +50,7 @@ sub usage_and_exit
print " -l list switches\n";
print " -C <ca_name> use selected channel adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibqueryerrors.pl
b/infiniband-diags/scripts/ibqueryerrors.pl
index c807c02..5f2e167 100755
--- a/infiniband-diags/scripts/ibqueryerrors.pl
+++ b/infiniband-diags/scripts/ibqueryerrors.pl
@@ -149,7 +149,7 @@ sub usage_and_exit
print " -d include the data counters in the output\n";
print " -C <ca_name> use selected Channel Adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibswportwatch.pl
b/infiniband-diags/scripts/ibswportwatch.pl
index 6d6ba1c..d888f51 100755
--- a/infiniband-diags/scripts/ibswportwatch.pl
+++ b/infiniband-diags/scripts/ibswportwatch.pl
@@ -81,7 +81,7 @@ sub usage_and_exit
print " -n <cycles> run n cycles then exit (default -1 == forever)\n";
print " -G Address provided is a GUID\n";
print " -b report bytes/second packets/second\n";
- exit 0;
+ exit 2;
}
# =========================================================================
--
Timothy A. Meier
Computer Scientist
ICCD/High Performance Computing
925.422.3341
[EMAIL PROTECTED]
_______________________________________________
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
