| Message ID | 1612772078-29651-1-git-send-email-zhufy.jy@cn.fujitsu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | netns_helper: Make iproute version check work correctly | expand |
Hi! > diff --git a/testcases/kernel/containers/netns/netns_helper.h b/testcases/kernel/containers/netns/netns_helper.h > index 8b87645..8337051 100644 > --- a/testcases/kernel/containers/netns/netns_helper.h > +++ b/testcases/kernel/containers/netns/netns_helper.h > @@ -37,6 +37,7 @@ static void check_iproute(unsigned int spe_ipver) > FILE *ipf; > int n; > unsigned int ipver = 0; > + char ver; > > ipf = popen("ip -V", "r"); > if (ipf == NULL) > @@ -44,7 +45,14 @@ static void check_iproute(unsigned int spe_ipver) > "Failed while opening pipe for iproute check"); > > n = fscanf(ipf, "ip utility, iproute2-ss%u", &ipver); > + pclose(ipf); > if (n < 1) { > + ipf = popen("ip -V", "r"); > + n = fscanf(ipf, "ip utility, iproute2-%s", &ver); > + if (n >= 1) { > + pclose(ipf); > + return; > + } Can we please instead read the whole string after the dash (-), skip the 'ss' prefix when present and then convert the string into an itneger? There is absolutely no reason to run the command twice. > static int dummy(void *arg) > diff --git a/testcases/kernel/containers/netns/netns_helper.sh b/testcases/kernel/containers/netns/netns_helper.sh > index a5b77a0..bec43ac 100755 > --- a/testcases/kernel/containers/netns/netns_helper.sh > +++ b/testcases/kernel/containers/netns/netns_helper.sh > @@ -50,6 +50,15 @@ tst_check_iproute() > local cur_ipver="$(ip -V)" > local spe_ipver="$1" > > + echo $cur_ipver | grep "ip utility, iproute2-ss" > /dev/null > + ret1=$? > + echo $cur_ipver | grep "ip utility, iproute2-" > /dev/null > + ret2=$? > + > + if [ $ret1 -ne 0 -a $ret2 -eq 0 ]; then > + return > + fi How is this supposed to fix the problem? This just skips the test if we haven't found one of the strings in the version, right? Which is wrong anyway. > cur_ipver=${cur_ipver##*s} But the version here is still wrong if the original string haven't contained the ss. We would end up with cur_ipver with iproute2-123456, which will be used in the numerical comparsion. > if [ -z "$cur_ipver" -o -z "$spe_ipver" ]; then > -- > 1.8.3.1 > > > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi! > > static int dummy(void *arg) > > diff --git a/testcases/kernel/containers/netns/netns_helper.sh b/testcases/kernel/containers/netns/netns_helper.sh > > index a5b77a0..bec43ac 100755 > > --- a/testcases/kernel/containers/netns/netns_helper.sh > > +++ b/testcases/kernel/containers/netns/netns_helper.sh > > @@ -50,6 +50,15 @@ tst_check_iproute() > > local cur_ipver="$(ip -V)" > > local spe_ipver="$1" > > > > + echo $cur_ipver | grep "ip utility, iproute2-ss" > /dev/null > > + ret1=$? > > + echo $cur_ipver | grep "ip utility, iproute2-" > /dev/null > > + ret2=$? > > + > > + if [ $ret1 -ne 0 -a $ret2 -eq 0 ]; then > > + return > > + fi > > How is this supposed to fix the problem? > > This just skips the test if we haven't found one of the strings in the ^ I mean version check, the test iself continues If nothing else this should TBROK.
Hi Zhu, thank you for working on this. I also started to work on this before LTP release, trying to fix it in the API, because there are more tests affected by this (there is also mc_cmds.sh). My WIP is to create tst_iproute_version.c, which would be used by shell and C tests. I actually wanted to go little bit further to have general C helper for getting version (but that'd have to wait after this is fixed). But as you were faster I'll let you to finish it. ... > diff --git a/testcases/kernel/containers/netns/netns_helper.sh b/testcases/kernel/containers/netns/netns_helper.sh > index a5b77a0..bec43ac 100755 > --- a/testcases/kernel/containers/netns/netns_helper.sh > +++ b/testcases/kernel/containers/netns/netns_helper.sh > @@ -50,6 +50,15 @@ tst_check_iproute() > local cur_ipver="$(ip -V)" > local spe_ipver="$1" > + echo $cur_ipver | grep "ip utility, iproute2-ss" > /dev/null > + ret1=$? > + echo $cur_ipver | grep "ip utility, iproute2-" > /dev/null > + ret2=$? > + > + if [ $ret1 -ne 0 -a $ret2 -eq 0 ]; then > + return > + fi > + FYI my shell version. Kind regards, Petr --- testcases/kernel/containers/netns/netns_helper.sh +++ testcases/kernel/containers/netns/netns_helper.sh @@ -47,16 +47,21 @@ IFCONF_IN6_ARG= tst_check_iproute() { - local cur_ipver="$(ip -V)" - local spe_ipver="$1" + local current_ver="$(ip -V)" + local expected_ver="111010" - cur_ipver=${cur_ipver##*s} + current_ver=${current_ver##*s} - if [ -z "$cur_ipver" -o -z "$spe_ipver" ]; then + if [ -z "$current_ver" -o -z "$expected_ver" ]; then tst_brk TBROK "failed to obtain valid iproute version" fi - if [ $cur_ipver -lt $spe_ipver ]; then + # new version scheme since v5.7.0-77-gb687d1067169 + if echo "$current_ver" | grep -q 'iproute2-v\?[0-9]\+\.'; then + return + fi + + if [ $current_ver -lt $expected_ver ]; then tst_brk TCONF "too old iproute version" fi }
Hi Zhu, ... > FYI my shell version. > Kind regards, > Petr > --- testcases/kernel/containers/netns/netns_helper.sh > +++ testcases/kernel/containers/netns/netns_helper.sh > @@ -47,16 +47,21 @@ IFCONF_IN6_ARG= > tst_check_iproute() > { > - local cur_ipver="$(ip -V)" > - local spe_ipver="$1" > + local current_ver="$(ip -V)" > + local expected_ver="111010" > - cur_ipver=${cur_ipver##*s} > + current_ver=${current_ver##*s} > - if [ -z "$cur_ipver" -o -z "$spe_ipver" ]; then > + if [ -z "$current_ver" -o -z "$expected_ver" ]; then I'm sorry, this was supposed to be only this: if [ -z "$current_ver" ]; then ($expected_ver is set few lines above). Kind regards, Petr > tst_brk TBROK "failed to obtain valid iproute version" > fi > - if [ $cur_ipver -lt $spe_ipver ]; then > + # new version scheme since v5.7.0-77-gb687d1067169 > + if echo "$current_ver" | grep -q 'iproute2-v\?[0-9]\+\.'; then > + return > + fi > + > + if [ $current_ver -lt $expected_ver ]; then > tst_brk TCONF "too old iproute version" > fi > } Kind regards, Petr
diff --git a/testcases/kernel/containers/netns/netns_helper.h b/testcases/kernel/containers/netns/netns_helper.h index 8b87645..8337051 100644 --- a/testcases/kernel/containers/netns/netns_helper.h +++ b/testcases/kernel/containers/netns/netns_helper.h @@ -37,6 +37,7 @@ static void check_iproute(unsigned int spe_ipver) FILE *ipf; int n; unsigned int ipver = 0; + char ver; ipf = popen("ip -V", "r"); if (ipf == NULL) @@ -44,7 +45,14 @@ static void check_iproute(unsigned int spe_ipver) "Failed while opening pipe for iproute check"); n = fscanf(ipf, "ip utility, iproute2-ss%u", &ipver); + pclose(ipf); if (n < 1) { + ipf = popen("ip -V", "r"); + n = fscanf(ipf, "ip utility, iproute2-%s", &ver); + if (n >= 1) { + pclose(ipf); + return; + } tst_brkm(TCONF, NULL, "Failed while obtaining version for iproute check"); } @@ -52,8 +60,6 @@ static void check_iproute(unsigned int spe_ipver) tst_brkm(TCONF, NULL, "The commands in iproute tools do " "not support required objects"); } - - pclose(ipf); } static int dummy(void *arg) diff --git a/testcases/kernel/containers/netns/netns_helper.sh b/testcases/kernel/containers/netns/netns_helper.sh index a5b77a0..bec43ac 100755 --- a/testcases/kernel/containers/netns/netns_helper.sh +++ b/testcases/kernel/containers/netns/netns_helper.sh @@ -50,6 +50,15 @@ tst_check_iproute() local cur_ipver="$(ip -V)" local spe_ipver="$1" + echo $cur_ipver | grep "ip utility, iproute2-ss" > /dev/null + ret1=$? + echo $cur_ipver | grep "ip utility, iproute2-" > /dev/null + ret2=$? + + if [ $ret1 -ne 0 -a $ret2 -eq 0 ]; then + return + fi + cur_ipver=${cur_ipver##*s} if [ -z "$cur_ipver" -o -z "$spe_ipver" ]; then
Since the iproute2 patch[1]("replace SNAPSHOT with auto-generated version string"), the output format of "ip -V" has changed form "ip utility, iproute2-ss*" to "ip utility, iproute2-*", which leads to an exception when checking the version of iproute. Use return to avoid unexpected TCONF. [1]https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=fbef655568 Signed-off-by: Feiyu Zhu <zhufy.jy@cn.fujitsu.com> --- testcases/kernel/containers/netns/netns_helper.h | 10 ++++++++-- testcases/kernel/containers/netns/netns_helper.sh | 9 +++++++++ 2 files changed, 17 insertions(+), 2 deletions(-)