[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Junchi Chen]

ISSUE:
  The case is designed to test the behaviour diverse caused by kernel
  commit 0fb44559ffd6. And it failed for the new behaviour.

ACTION:
  Add a version compare to split part of the test.

Signed-off-by: Junchi Chen <junchi.chen@intel.com>
---
 testcases/kernel/syscalls/bind/bind03.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/bind/bind03.c b/testcases/kernel/syscalls/bind/bind03.c
index 2fe342a54..955a69dd2 100644
--- a/testcases/kernel/syscalls/bind/bind03.c
+++ b/testcases/kernel/syscalls/bind/bind03.c
@@ -13,6 +13,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "tst_kvercmp.h"
 #include "tst_test.h"
 #include "tst_safe_net.h"
 
@@ -37,16 +38,28 @@ void run(void)
 
 	/*
 	 * Once a STREAM UNIX domain socket has been bound, it can't be
-	 * rebound. Expected error is EINVAL.
+	 * rebound.
 	 */
 	if (bind(sock1, (struct sockaddr *)&sun, sizeof(sun)) == 0) {
 		tst_res(TFAIL, "re-binding of socket succeeded");
 		return;
 	}
 
-	if (errno != EINVAL) {
-		tst_res(TFAIL | TERRNO, "expected EINVAL");
-		return;
+	/*
+	 * The behavious diverse according to kernel version
+	 * for v4.10 or later, the expected error is EADDRINUSE,
+	 * otherwise EINVAL.
+	 */
+	if (tst_kvercmp(4, 10, 0) < 0) {
+		if (errno != EINVAL) {
+			tst_res(TFAIL | TERRNO, "expected EINVAL");
+			return;
+		}
+	} else {
+		if (errno != EADDRINUSE) {
+			tst_res(TFAIL | TERRNO, "expected EADDRINUSE");
+			return;
+		}
 	}
 
 	sock2 = SAFE_SOCKET(PF_UNIX, SOCK_STREAM, 0);
-- 
2.17.1

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Petr Vorel]

Hi Junchi,

> ISSUE:
>   The case is designed to test the behaviour diverse caused by kernel
>   commit 0fb44559ffd6. And it failed for the new behaviour.

> ACTION:
>   Add a version compare to split part of the test.

> Signed-off-by: Junchi Chen <junchi.chen@intel.com>

Merged, thanks!


Kind regards,
Petr

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Cyril Hrubis]

Hi!
> > ISSUE:
> >   The case is designed to test the behaviour diverse caused by kernel
> >   commit 0fb44559ffd6. And it failed for the new behaviour.
> 
> > ACTION:
> >   Add a version compare to split part of the test.
> 
> > Signed-off-by: Junchi Chen <junchi.chen@intel.com>
> 
> Merged, thanks!

I think that this should be reverted, until at least we got a clear
reply from kernel developers...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Petr Vorel]

Hi,

> > > ISSUE:
> > >   The case is designed to test the behaviour diverse caused by kernel
> > >   commit 0fb44559ffd6. And it failed for the new behaviour.

> > > ACTION:
> > >   Add a version compare to split part of the test.

> > > Signed-off-by: Junchi Chen <junchi.chen@intel.com>

> > Merged, thanks!

> I think that this should be reverted, until at least we got a clear
> reply from kernel developers...
Michal Kubecek doesn't plan to force his patch, so we can keep the fix.

Although we should improve the test of EINVAL (not to pass two invalid
parameters to the syscall at once).


Kind regards,
Petr

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Xiao Yang]

On 2018/08/31 16:37, Petr Vorel wrote:
> Hi Junchi,
>
>> ISSUE:
>>    The case is designed to test the behaviour diverse caused by kernel
>>    commit 0fb44559ffd6. And it failed for the new behaviour.
>> ACTION:
>>    Add a version compare to split part of the test.
>> Signed-off-by: Junchi Chen<junchi.chen@intel.com>
> Merged, thanks!
Hi Petr,

According to bind(2) manpge, bind() should return EINVAL when the socket 
is already
bound to an address.  Commit 0fb4455 actually change the errno to 
EADDRINUSE, but
i am not sure if this change is a expeceted behavior.

Thanks,
Xiao Yang
>
> Kind regards,
> Petr
>

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Cyril Hrubis]

Hi!
> According to bind(2) manpge, bind() should return EINVAL when the socket 
> is already
> bound to an address.  Commit 0fb4455 actually change the errno to 
> EADDRINUSE, but
> i am not sure if this change is a expeceted behavior.

As Michal Kubecek writes in the commit message of the patch I've linked,
the kernel has changed the order of checks which returns different errno
than it used to for no good reason. I would like to be conservative in
this case and have the previous behavior restored, but I suppose that's
up to kernel guys to decide.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Cyril Hrubis]

Hi!
> ISSUE:
>   The case is designed to test the behaviour diverse caused by kernel
>   commit 0fb44559ffd6. And it failed for the new behaviour.
> 
> ACTION:
>   Add a version compare to split part of the test.

This is actually a not-yet fixed kernel bug as the correct errno to be
reported in this case is EADDRINUSE at least that is what I understand
after reading POSIX.

See:

https://patchwork.ozlabs.org/patch/782692/

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Cyril Hrubis]

Hi!
> > ISSUE:
> >   The case is designed to test the behaviour diverse caused by kernel
> >   commit 0fb44559ffd6. And it failed for the new behaviour.
> > 
> > ACTION:
> >   Add a version compare to split part of the test.
> 
> This is actually a not-yet fixed kernel bug as the correct errno to be
> reported in this case is EADDRINUSE at least that is what I understand
> after reading POSIX.

Sigh, not enough coffee I would say, the rationale is correctly written
in the patch from Michal.

"in general, we do not want to change return code for given testcase and
 old error (-EINVAL) is consistent with AF_INET(6)"

Sorry for the confusion.

> See:
> 
> https://patchwork.ozlabs.org/patch/782692/

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.

[Petr Vorel]

Hi,

> > > ISSUE:
> > >   The case is designed to test the behaviour diverse caused by kernel
> > >   commit 0fb44559ffd6. And it failed for the new behaviour.

> > > ACTION:
> > >   Add a version compare to split part of the test.

> > This is actually a not-yet fixed kernel bug as the correct errno to be
> > reported in this case is EADDRINUSE at least that is what I understand
> > after reading POSIX.

> Sigh, not enough coffee I would say, the rationale is correctly written
> in the patch from Michal.

> "in general, we do not want to change return code for given testcase and
>  old error (-EINVAL) is consistent with AF_INET(6)"

> Sorry for the confusion.

> > See:

> > https://patchwork.ozlabs.org/patch/782692/

I've already merged this fix, but as Cyril pointed out it might not be what is
wanted. I'll ask David Miller and Michal Kubecek at netdev ML.


Kind regards,
Petr