[LTP] [PATCH] nfs_flock: fail the test if lock/unlock ops fail

[LTP] [PATCH] nfs_flock: fail the test if lock/unlock ops fail

[Nikita Yushchenko via ltp]

Since nfs_flock is testing locking operations, ignoring errors returned
from those operations is nonsense.

Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
---
 testcases/network/nfs/nfslock01/nfs_flock.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/testcases/network/nfs/nfslock01/nfs_flock.c b/testcases/network/nfs/nfslock01/nfs_flock.c
index a7673c751..a13ddd251 100644
--- a/testcases/network/nfs/nfslock01/nfs_flock.c
+++ b/testcases/network/nfs/nfslock01/nfs_flock.c
@@ -63,16 +63,20 @@ int main(int argc, char **argv)
 				continue;
 		}
 
-		if (writeb_lock(fd, offset, SEEK_SET, BYTES) < 0)
-			printf("failed in writeb_lock, Errno = %d", errno);
+		if (writeb_lock(fd, offset, SEEK_SET, BYTES) < 0) {
+			printf("failed in writeb_lock, Errno = %d\n", errno);
+			exit(1);
+		}
 
 		lseek(fd, offset, SEEK_SET);
 
 		/* write to the test file */
 		write(fd, buf, BYTES);
 
-		if (unb_lock(fd, offset, SEEK_SET, BYTES) < 0)
-			printf("failed in unb_lock, Errno = %d", errno);
+		if (unb_lock(fd, offset, SEEK_SET, BYTES) < 0) {
+			printf("failed in unb_lock, Errno = %d\n", errno);
+			exit(1);
+		}
 	}
 	exit(0);
 }
-- 
2.30.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH] nfs_lib.sh: run exportfs at "server side" in LTP_NETNS case

[Nikita Yushchenko via ltp]

In LTP_NETNS case, nfs server is the root namespace and nfs client is
the ltp namespace.

Then, exportfs shall be executed locally, without tst_rhost_run.

Otherwise, things implicitly depend on /var/lib/nfs being the same in
the root namespace and the ltp namespace.

Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
---
 testcases/network/nfs/nfs_stress/nfs_lib.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh
index b01215136..b50ccf196 100644
--- a/testcases/network/nfs/nfs_stress/nfs_lib.sh
+++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh
@@ -81,8 +81,14 @@ nfs_setup_server()
 {
 	local export_cmd="exportfs -i -o fsid=$$,no_root_squash,rw *:$remote_dir"
 
-	if ! tst_rhost_run -c "test -d $remote_dir"; then
-		tst_rhost_run -s -c "mkdir -p $remote_dir; $export_cmd"
+	if [ -n "$LTP_NETNS" ]; then
+		if ! test -d $remote_dir; then
+			mkdir -p $remote_dir; $export_cmd
+		fi
+	else
+		if ! tst_rhost_run -c "test -d $remote_dir"; then
+			tst_rhost_run -s -c "mkdir -p $remote_dir; $export_cmd"
+		fi
 	fi
 }
 
-- 
2.30.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_lib.sh: run exportfs at "server side" in LTP_NETNS case

[Petr Vorel]

Hi Nikita,

[ Cc Alexey ]

> In LTP_NETNS case, nfs server is the root namespace and nfs client is
> the ltp namespace.

> Then, exportfs shall be executed locally, without tst_rhost_run.

> Otherwise, things implicitly depend on /var/lib/nfs being the same in
> the root namespace and the ltp namespace.
Not sure if I understand your use case. Do you run rpc.statd (or what is using
/var/lib/nfs) in non-default net namespace?

> Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
> ---
>  testcases/network/nfs/nfs_stress/nfs_lib.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

> diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh
> index b01215136..b50ccf196 100644
> --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh
> +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh
> @@ -81,8 +81,14 @@ nfs_setup_server()
>  {
>  	local export_cmd="exportfs -i -o fsid=$$,no_root_squash,rw *:$remote_dir"

> -	if ! tst_rhost_run -c "test -d $remote_dir"; then
> -		tst_rhost_run -s -c "mkdir -p $remote_dir; $export_cmd"
> +	if [ -n "$LTP_NETNS" ]; then
Please use tst_net_use_netns (as in the patch I Cc you just now).

Shouldn't be also $LTP_NFS_NETNS_USE_LO considered?

Kind regards,
Petr

> +		if ! test -d $remote_dir; then
> +			mkdir -p $remote_dir; $export_cmd
> +		fi
> +	else
> +		if ! tst_rhost_run -c "test -d $remote_dir"; then
> +			tst_rhost_run -s -c "mkdir -p $remote_dir; $export_cmd"
> +		fi
>  	fi
>  }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_lib.sh: run exportfs at "server side" in LTP_NETNS case

[Nikita Yushchenko via ltp]

13.01.2022 18:50, Petr Vorel wrote:
> Hi Nikita,
> 
> [ Cc Alexey ]
> 
>> In LTP_NETNS case, nfs server is the root namespace and nfs client is
>> the ltp namespace.
> 
>> Then, exportfs shall be executed locally, without tst_rhost_run.
> 
>> Otherwise, things implicitly depend on /var/lib/nfs being the same in
>> the root namespace and the ltp namespace.
> Not sure if I understand your use case. Do you run rpc.statd (or what is using
> /var/lib/nfs) in non-default net namespace?

'exportfs' command maintains /var/lib/nfs/etab file, to be read by rpc.mountd when processing mount 
requests.

'exportfs' must be executed in the same environment where rpc.mountd runs

In LTP_NETNS case, rpc.mountd runs on the host's root namespaces
(and mount runs in ltp's non-root namespace).

Thus for correctness, must execute 'exportfs' in the root namespace.

Currently ltp runs 'exportfs' in ltpns, which works only because ltpns does not unshare /var from root 
namespace.

But not unsharing /var makes AF_UNIX socket for host's rpcbind to become available inside ltpns. Then, 
at nfs3 mount time, kernel creates an instance of lockd for ltpns, and ports for that instance leak to 
host's rpcbind and overwrite ports for lockd already active for root namespace. This breaks nfs3 file 
locking. But that is not found by nfslock01 test because that test ignores the errors from the very 
operations it is intended to test.

This patch, and the patch that makes nfslock01 to actually fail on errors, is part of fixing all that mess.

>> -	if ! tst_rhost_run -c "test -d $remote_dir"; then
>> -		tst_rhost_run -s -c "mkdir -p $remote_dir; $export_cmd"
>> +	if [ -n "$LTP_NETNS" ]; then
> Please use tst_net_use_netns (as in the patch I Cc you just now).
> 
> Shouldn't be also $LTP_NFS_NETNS_USE_LO considered?

The rule is - run exportfs in the environment that plays the 'nfs server' role.

I'm not sure about exact semantics of $LTP_NFS_NETNS_USE_LO, but per
what I see in the code, it does not affect how address in $mount_dir is configured. Then, it also shall 
not affect the choice of where 'exportfs' runs.

Nikita

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_flock: fail the test if lock/unlock ops fail

[Petr Vorel]

Hi Nikita,

[ Cc Alexey ]
> Since nfs_flock is testing locking operations, ignoring errors returned
> from those operations is nonsense.

Makes sense.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
> ---
>  testcases/network/nfs/nfslock01/nfs_flock.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

> diff --git a/testcases/network/nfs/nfslock01/nfs_flock.c b/testcases/network/nfs/nfslock01/nfs_flock.c
> index a7673c751..a13ddd251 100644
> --- a/testcases/network/nfs/nfslock01/nfs_flock.c
> +++ b/testcases/network/nfs/nfslock01/nfs_flock.c
> @@ -63,16 +63,20 @@ int main(int argc, char **argv)
>  				continue;
>  		}

> -		if (writeb_lock(fd, offset, SEEK_SET, BYTES) < 0)
> -			printf("failed in writeb_lock, Errno = %d", errno);
> +		if (writeb_lock(fd, offset, SEEK_SET, BYTES) < 0) {
> +			printf("failed in writeb_lock, Errno = %d\n", errno);
> +			exit(1);
> +		}

>  		lseek(fd, offset, SEEK_SET);

>  		/* write to the test file */
>  		write(fd, buf, BYTES);

> -		if (unb_lock(fd, offset, SEEK_SET, BYTES) < 0)
> -			printf("failed in unb_lock, Errno = %d", errno);
> +		if (unb_lock(fd, offset, SEEK_SET, BYTES) < 0) {
> +			printf("failed in unb_lock, Errno = %d\n", errno);
> +			exit(1);
> +		}
>  	}
>  	exit(0);
>  }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_flock: fail the test if lock/unlock ops fail

[Martin Doucha]

Hello,

On 12. 01. 22 17:19, Nikita Yushchenko via ltp wrote:
> Since nfs_flock is testing locking operations, ignoring errors returned
> from those operations is nonsense.
> 
> Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
> ---
>  testcases/network/nfs/nfslock01/nfs_flock.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/network/nfs/nfslock01/nfs_flock.c b/testcases/network/nfs/nfslock01/nfs_flock.c
> index a7673c751..a13ddd251 100644
> --- a/testcases/network/nfs/nfslock01/nfs_flock.c
> +++ b/testcases/network/nfs/nfslock01/nfs_flock.c
> @@ -63,16 +63,20 @@ int main(int argc, char **argv)
>  				continue;
>  		}
>  
> -		if (writeb_lock(fd, offset, SEEK_SET, BYTES) < 0)
> -			printf("failed in writeb_lock, Errno = %d", errno);
> +		if (writeb_lock(fd, offset, SEEK_SET, BYTES) < 0) {
> +			printf("failed in writeb_lock, Errno = %d\n", errno);
> +			exit(1);
> +		}
>  
>  		lseek(fd, offset, SEEK_SET);
>  
>  		/* write to the test file */
>  		write(fd, buf, BYTES);
>  
> -		if (unb_lock(fd, offset, SEEK_SET, BYTES) < 0)
> -			printf("failed in unb_lock, Errno = %d", errno);
> +		if (unb_lock(fd, offset, SEEK_SET, BYTES) < 0) {
> +			printf("failed in unb_lock, Errno = %d\n", errno);
> +			exit(1);
> +		}
>  	}
>  	exit(0);
>  }

The test program is quite short and simple. Why not rewrite it using the
current LTP API and make it even better? Some useful links to help with
that:

https://github.com/linux-test-project/ltp/wiki/C-Test-Case-Tutorial
https://github.com/linux-test-project/ltp/wiki/C-Test-API
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_flock: fail the test if lock/unlock ops fail

[Petr Vorel]

Hi Martin, Nikita,

> The test program is quite short and simple. Why not rewrite it using the
> current LTP API and make it even better? Some useful links to help with
> that:

+1

It might not make it into this release due git freeze [1],
but definitely worth of doing it.
Thus I'd accept this patch for upcoming release.

Kind regards,
Petr

> https://github.com/linux-test-project/ltp/wiki/C-Test-Case-Tutorial
> https://github.com/linux-test-project/ltp/wiki/C-Test-API
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines

[1] https://lore.kernel.org/ltp/YeFGwLzrR3t%2FVQLq@yuki/

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_lib.sh: run exportfs at "server side" in LTP_NETNS case

[Petr Vorel]

> 13.01.2022 18:50, Petr Vorel wrote:
> > Hi Nikita,

> > [ Cc Alexey ]

> > > In LTP_NETNS case, nfs server is the root namespace and nfs client is
> > > the ltp namespace.

> > > Then, exportfs shall be executed locally, without tst_rhost_run.

> > > Otherwise, things implicitly depend on /var/lib/nfs being the same in
> > > the root namespace and the ltp namespace.
> > Not sure if I understand your use case. Do you run rpc.statd (or what is using
> > /var/lib/nfs) in non-default net namespace?

> 'exportfs' command maintains /var/lib/nfs/etab file, to be read by
> rpc.mountd when processing mount requests.

> 'exportfs' must be executed in the same environment where rpc.mountd runs

> In LTP_NETNS case, rpc.mountd runs on the host's root namespaces
> (and mount runs in ltp's non-root namespace).

> Thus for correctness, must execute 'exportfs' in the root namespace.

Ah, you're right.

> Currently ltp runs 'exportfs' in ltpns, which works only because ltpns does
> not unshare /var from root namespace.

> But not unsharing /var makes AF_UNIX socket for host's rpcbind to become
> available inside ltpns. Then, at nfs3 mount time, kernel creates an instance
> of lockd for ltpns, and ports for that instance leak to host's rpcbind and
> overwrite ports for lockd already active for root namespace. This breaks
> nfs3 file locking. But that is not found by nfslock01 test because that test
> ignores the errors from the very operations it is intended to test.

> This patch, and the patch that makes nfslock01 to actually fail on errors, is part of fixing all that mess.
+1. FYI I get errno ENOLCK for NFSv3 on both unpatched and patched nfs_lib.sh.

> > > -	if ! tst_rhost_run -c "test -d $remote_dir"; then
> > > -		tst_rhost_run -s -c "mkdir -p $remote_dir; $export_cmd"
> > > +	if [ -n "$LTP_NETNS" ]; then
> > Please use tst_net_use_netns (as in the patch I Cc you just now).

> > Shouldn't be also $LTP_NFS_NETNS_USE_LO considered?

> The rule is - run exportfs in the environment that plays the 'nfs server' role.

> I'm not sure about exact semantics of $LTP_NFS_NETNS_USE_LO, but per
> what I see in the code, it does not affect how address in $mount_dir is
> configured. Then, it also shall not affect the choice of where 'exportfs'
> runs.
Sorry, $LTP_NFS_NETNS_USE_LO is separated thing (mounting does not affect server
setup).

Waiting for Alexey or Martin if they have any comment before merging both fixes on Monday.

Kind regards,
Petr

> Nikita

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_lib.sh: run exportfs at "server side" in LTP_NETNS case

[Nikita Yushchenko via ltp]

> +1. FYI I get errno ENOLCK for NFSv3 on both unpatched and patched nfs_lib.sh.

Same here.

AFAIU, to make it work, need to unshare filesystem containing rpcbind's AF_UNIX socket from ltpns. Then, 
ltpns instance of kernel lockd server will no longer overwrite ports set by host's instance, and kernel 
lockd client for ltpns will be able to contact host's lockd server.

Nikita

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] nfs_flock: fail the test if lock/unlock ops fail

[Petr Vorel]

Hi all,

> Hi Martin, Nikita,

> > The test program is quite short and simple. Why not rewrite it using the
> > current LTP API and make it even better? Some useful links to help with
> > that:

> +1

> It might not make it into this release due git freeze [1],
> but definitely worth of doing it.
> Thus I'd accept this patch for upcoming release.

I'm sorry I completely forgot on this patch. I merged it, because it
immediately stops broken test on NFS v3, which would otherwise timeout.

The problem itself would deserve debugging. If it's WONFTFIX due rpcbind
just not working on netns on NFS v3 [2], we should remove nfslock3t_01 and
nfslock3t_ipv6_01 from runtest/net.nfs and disable with TCONF NFS v3 (i.e. -v 3)
in nfslock01.sh. I'll try to not forget on posting a RFC patch to linux-nfs ML.

Kind regards,
Petr

[2] https://lore.kernel.org/ltp/3cb5de6e-6f8f-e46a-96bd-a3d88a871f3a@virtuozzo.com/


> Kind regards,
> Petr

> > https://github.com/linux-test-project/ltp/wiki/C-Test-Case-Tutorial
> > https://github.com/linux-test-project/ltp/wiki/C-Test-API
> > https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines

> [1] https://lore.kernel.org/ltp/YeFGwLzrR3t%2FVQLq@yuki/

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp