| Message ID | 20240830095758.20018-1-wegao@suse.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] mpls01.sh: Add --allow-unsupported for modprobe | expand |
Hi Wei, > In sle-micro we encounter following error when do modprobe: > root# modprobe mpls_router > modprobe: ERROR: module 'mpls_router' is unsupported > modprobe: ERROR: Use --allow-unsupported or set allow_unsupported_modules 1 in > modprobe: ERROR: /etc/modprobe.d/10-unsupported-modules.conf > modprobe: ERROR: could not insert 'mpls_router': Operation not permitted > Signed-off-by: Wei Gao <wegao@suse.com> > --- > testcases/network/mpls/mpls01.sh | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > diff --git a/testcases/network/mpls/mpls01.sh b/testcases/network/mpls/mpls01.sh > index 196b5b2f9..7f262d83e 100755 > --- a/testcases/network/mpls/mpls01.sh > +++ b/testcases/network/mpls/mpls01.sh > @@ -21,7 +21,9 @@ cleanup() > setup() > { > - ROD modprobe mpls_router > + if ! modprobe mpls_router > /dev/null 2>&1; then > + ROD modprobe mpls_router --allow-unsupported I'm not sure with ROD. --allow-unsupported is SUSE specific modprobe extension, which is used for SLES. It is also on Tumbleweed [1] [2] where does nothing. If you try to run it with ROD, which quits testing on failure, all distros except SUSE products/openSUSE (e.g. Debian, Fedora, ...) which don't have mpls_router will TBROK in setup: modprobe: unrecognised option '--allow-unsupported'. And I'm not talking about these small distros which use busybox kmod implementation, which would also fail. Also you remove /dev/null 2>&1 from the first command, thus other distros would not see error message about missing mpls_router module. Without looking into the source the tester will be pretty confused. I would do: if grep -q suse /etc/os-release; then ROD modprobe --allow-unsupported mpls_router else ROD modprobe mpls_router fi (nit: better to put the option before the module name.) With this, you can put in the next version: Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr > + fi > } > test1() [1] https://build.opensuse.org/projects/Base:System/packages/kmod/files/0002-modprobe-Recognize-allow-unsupported-modules-on-comm.patch?expand=1
Hi! > I'm not sure with ROD. --allow-unsupported is SUSE specific modprobe extension, > which is used for SLES. It is also on Tumbleweed [1] [2] where does nothing. > > If you try to run it with ROD, which quits testing on failure, all distros > except SUSE products/openSUSE (e.g. Debian, Fedora, ...) which don't have > mpls_router will TBROK in setup: > > modprobe: unrecognised option '--allow-unsupported'. > > And I'm not talking about these small distros which use busybox kmod > implementation, which would also fail. > > Also you remove /dev/null 2>&1 from the first command, thus other distros would > not see error message about missing mpls_router module. Without looking into the > source the tester will be pretty confused. > > I would do: > > if grep -q suse /etc/os-release; then There are a couple of english words that contain "suse" as a substring if one of these ends up in the os-release it would match, e.g. the os-release may contain VERSION="24 (Disused Miracle)". And yes both Fedora and Ubuntu seems to include the the code names in the file. So I would be stricter here and checked for ID=suse.
Hi Cyril, Wei, > Hi! > > if grep -q suse /etc/os-release; then > There are a couple of english words that contain "suse" as a substring > if one of these ends up in the os-release it would match, e.g. the > os-release may contain VERSION="24 (Disused Miracle)". And yes both > Fedora and Ubuntu seems to include the the code names in the file. > So I would be stricter here and checked for ID=suse. Very good point. FYI SLES (removed non-related lines) NAME="SLES" VERSION="15-SP6" PRETTY_NAME="SUSE Linux Enterprise Server 15 SP6" ID="sles" ID_LIKE="suse" SLE Micro (removed non-related lines): NAME="SL-Micro" PRETTY_NAME="SUSE Linux Micro 6.0" ID="sl-micro" ID_LIKE="suse" Debian: NAME="Debian GNU/Linux" ID=debian => SUSE quite all variables, but at least Debian not (ID=debian). Therefore for more complex scenarios I would source the file (. /etc/os-release). But in this case grep sl-micro could be enough (unique enough). Will you ack this change (RBT/ABT) in v2 [1] (newer version than this, I would change before merge)? - if grep -q suse /etc/os-release; then + if grep -q 'sl-micro' /etc/os-release; then Kind regards, Petr [1] https://patchwork.ozlabs.org/project/ltp/patch/20240902024017.6404-1-wegao@suse.com/
diff --git a/testcases/network/mpls/mpls01.sh b/testcases/network/mpls/mpls01.sh index 196b5b2f9..7f262d83e 100755 --- a/testcases/network/mpls/mpls01.sh +++ b/testcases/network/mpls/mpls01.sh @@ -21,7 +21,9 @@ cleanup() setup() { - ROD modprobe mpls_router + if ! modprobe mpls_router > /dev/null 2>&1; then + ROD modprobe mpls_router --allow-unsupported + fi } test1()
In sle-micro we encounter following error when do modprobe: root# modprobe mpls_router modprobe: ERROR: module 'mpls_router' is unsupported modprobe: ERROR: Use --allow-unsupported or set allow_unsupported_modules 1 in modprobe: ERROR: /etc/modprobe.d/10-unsupported-modules.conf modprobe: ERROR: could not insert 'mpls_router': Operation not permitted Signed-off-by: Wei Gao <wegao@suse.com> --- testcases/network/mpls/mpls01.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)