| Message ID | 20200617154926.32588-1-alexey.kodanev@oracle.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | network/nsf_lib.sh: TCONF on mount (udp/udp6) failure for Linux v5.6+ | expand |
Alexey Kodanev <alexey.kodanev@oracle.com> wrote: ... --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh > +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh > @@ -87,11 +87,17 @@ nfs_mount() > > tst_res TINFO "Mounting NFS: $mnt_cmd" > if [ -n "$LTP_NETNS" ] && [ -z "$LTP_NFS_NETNS_USE_LO" ]; then > - tst_rhost_run -s -c "$mnt_cmd" > Or, maybe we can have a new function naming as 'tst_kconifg_check' to parse kernel .config in shell library as well? i.e tst_kconfig_check "NFS_DISABLE_UDP_SUPPORT=y" if [$? -qe 0 ]; then tst_rhost_run -s -c "$mnt_cmd" else exit with TCONF ... What do you think? > - return > + tst_rhost_run -c "$mnt_cmd" > + else > + $mnt_cmd > /dev/null > fi > > - ROD $mnt_cmd > + if [ $? -ne 0 ]; then > + if [ "$type" = "udp" -o "$type" = "udp6" ] && tst_kvcmp > -ge 5.6; then > + tst_brk TCONF "UDP support disabled with the > kernel config NFS_DISABLE_UDP_SUPPORT?" > + fi > + tst_brk TBROK "mount command failed" > + fi > } > > nfs_setup() > -- > 2.20.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > >
Hi Li, Alexey, > Alexey Kodanev <alexey.kodanev@oracle.com> wrote: > ... > --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh > > +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh > > @@ -87,11 +87,17 @@ nfs_mount() > > tst_res TINFO "Mounting NFS: $mnt_cmd" > > if [ -n "$LTP_NETNS" ] && [ -z "$LTP_NFS_NETNS_USE_LO" ]; then > > - tst_rhost_run -s -c "$mnt_cmd" > Or, maybe we can have a new function naming as 'tst_kconifg_check' to parse > kernel .config in shell library as well? +1, I was thinking about it for a long time. > i.e > tst_kconfig_check "NFS_DISABLE_UDP_SUPPORT=y" > if [$? -qe 0 ]; then > tst_rhost_run -s -c "$mnt_cmd" > else > exit with TCONF ... > What do you think? And also have NEEDS_KCONFIGS (space separated). Kind regards, Petr
Hi Li, Alexey, Cyril, Reviewed-by: Petr Vorel <pvorel@suse.cz> > > > +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh > > > @@ -87,11 +87,17 @@ nfs_mount() > > > tst_res TINFO "Mounting NFS: $mnt_cmd" > > > if [ -n "$LTP_NETNS" ] && [ -z "$LTP_NFS_NETNS_USE_LO" ]; then > > > - tst_rhost_run -s -c "$mnt_cmd" > > Or, maybe we can have a new function naming as 'tst_kconifg_check' to parse > > kernel .config in shell library as well? > +1, I was thinking about it for a long time. Thinking about the balance between base TCONF decision on kernel version vs. require kernel config to be presented I think for cases like this I'd prefer kernel version based check (i.e. the original patch). Requiring kernel config is ok for traditional distros (and even here is sometimes readable only for root, e.g. Debian/Ubuntu), but it's still rare on arm (other embedded archs). I guess it'd be nice to have some variable, which would turn kernel config based requirement into warning. Kind regards, Petr
On 18.06.2020 12:36, Petr Vorel wrote: > Hi Li, Alexey, Cyril, > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > >>>> +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh >>>> @@ -87,11 +87,17 @@ nfs_mount() > >>>> tst_res TINFO "Mounting NFS: $mnt_cmd" >>>> if [ -n "$LTP_NETNS" ] && [ -z "$LTP_NFS_NETNS_USE_LO" ]; then >>>> - tst_rhost_run -s -c "$mnt_cmd" > > >>> Or, maybe we can have a new function naming as 'tst_kconifg_check' to parse >>> kernel .config in shell library as well? >> +1, I was thinking about it for a long time. > Thinking about the balance between base TCONF decision on kernel version vs. > require kernel config to be presented I think for cases like this I'd prefer > kernel version based check (i.e. the original patch). > > Requiring kernel config is ok for traditional distros (and even here is > sometimes readable only for root, e.g. Debian/Ubuntu), but it's still rare on > arm (other embedded archs). I guess it'd be nice to have some variable, which > would turn kernel config based requirement into warning. > Also the option's name can be renamed in new releases, so the decision should be made only if the option is found (i.e. if we check without the the kernel version). tst_check_kconfig - will be really nice to have. Then, we will tconf earlier in the setup if this option is found. What about TCONF_IF_KCONFIG, in addition to NEEDS_KCONFIG? > Kind regards, > Petr >
Hi Li, Alexey, Cyril, > > Requiring kernel config is ok for traditional distros (and even here is > > sometimes readable only for root, e.g. Debian/Ubuntu), but it's still rare on > > arm (other embedded archs). I guess it'd be nice to have some variable, which > > would turn kernel config based requirement into warning. > Also the option's name can be renamed in new releases, so the decision > should be made only if the option is found (i.e. if we check without the > the kernel version). FYI (you may have noticed) there was an effort to add boolean OR for C implementation: https://patchwork.ozlabs.org/project/ltp/list/?series=149804&state=* > tst_check_kconfig - will be really nice to have. +1. Is anybody planning to work on it? > Then, we will tconf > earlier in the setup if this option is found. > What about TCONF_IF_KCONFIG, in addition to NEEDS_KCONFIG? +1 (+ add it to C as well). Kind regards, Petr
Hi Petr, Alexey, On Thu, Jun 18, 2020 at 9:49 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, Alexey, Cyril, > > > > Requiring kernel config is ok for traditional distros (and even here is > > > sometimes readable only for root, e.g. Debian/Ubuntu), but it's still > rare on > > > arm (other embedded archs). I guess it'd be nice to have some > variable, which > > > would turn kernel config based requirement into warning. > > > > Also the option's name can be renamed in new releases, so the decision > > should be made only if the option is found (i.e. if we check without the > > the kernel version). > FYI (you may have noticed) there was an effort to add boolean OR for C > implementation: > https://patchwork.ozlabs.org/project/ltp/list/?series=149804&state=* > > > tst_check_kconfig - will be really nice to have. > +1. Is anybody planning to work on it? > I have no plan to work on this, but my pleasure to help review/test the patch if possible. > > > Then, we will tconf > > earlier in the setup if this option is found. > > > What about TCONF_IF_KCONFIG, in addition to NEEDS_KCONFIG? > +1 (+ add it to C as well). > NEEDS_KCONFIG - this is optional to me, my previous thought is only to have tst_check_kconfig to match the situation we need to perform some special commands for the kernel with different kconfig, but it is also acceptable to TCONF in setup phase like what we do in C library. TCONF_IF_KCONFIG - And I haven't realized what the TCONF_IF_KCONFIG the behavior here, maybe needs read code then.
Hi all, > > Also the option's name can be renamed in new releases, so the decision > > should be made only if the option is found (i.e. if we check without the > > the kernel version). > FYI (you may have noticed) there was an effort to add boolean OR for C > implementation: > https://patchwork.ozlabs.org/project/ltp/list/?series=149804&state=* > > tst_check_kconfig - will be really nice to have. > +1. Is anybody planning to work on it? https://github.com/linux-test-project/ltp/issues/700 I'll try to implement it in July, unless somebody does it before. > > Then, we will tconf > > earlier in the setup if this option is found. > > What about TCONF_IF_KCONFIG, in addition to NEEDS_KCONFIG? > +1 (+ add it to C as well). I'd prefer TWARN_IF_NO_CONFIG, more info in the ticket. Kind regards, Petr
diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh index e236cd485..1bd057717 100644 --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh @@ -87,11 +87,17 @@ nfs_mount() tst_res TINFO "Mounting NFS: $mnt_cmd" if [ -n "$LTP_NETNS" ] && [ -z "$LTP_NFS_NETNS_USE_LO" ]; then - tst_rhost_run -s -c "$mnt_cmd" - return + tst_rhost_run -c "$mnt_cmd" + else + $mnt_cmd > /dev/null fi - ROD $mnt_cmd + if [ $? -ne 0 ]; then + if [ "$type" = "udp" -o "$type" = "udp6" ] && tst_kvcmp -ge 5.6; then + tst_brk TCONF "UDP support disabled with the kernel config NFS_DISABLE_UDP_SUPPORT?" + fi + tst_brk TBROK "mount command failed" + fi } nfs_setup()
Most likely support is disabled with NFS_DISABLE_UDP_SUPPORT config option (default y). commit b24ee6c64ca7 ("NFS: allow deprecation of NFS UDP protocol") Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> --- testcases/network/nfs/nfs_stress/nfs_lib.sh | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)