All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31 13:24 [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference Junchi Chen
@ 2018-08-31  8:37 ` Petr Vorel
  2018-08-31  8:42   ` Cyril Hrubis
  2018-08-31  8:48   ` Xiao Yang
  2018-08-31  8:41 ` Cyril Hrubis
  1 sibling, 2 replies; 9+ messages in thread
From: Petr Vorel @ 2018-08-31  8:37 UTC (permalink / raw)
  To: ltp

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31 13:24 [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference Junchi Chen
  2018-08-31  8:37 ` Petr Vorel
@ 2018-08-31  8:41 ` Cyril Hrubis
  2018-08-31  9:37   ` Cyril Hrubis
  1 sibling, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2018-08-31  8:41 UTC (permalink / raw)
  To: ltp

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31  8:37 ` Petr Vorel
@ 2018-08-31  8:42   ` Cyril Hrubis
  2018-09-07 10:12     ` Petr Vorel
  2018-08-31  8:48   ` Xiao Yang
  1 sibling, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2018-08-31  8:42 UTC (permalink / raw)
  To: ltp

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31  8:37 ` Petr Vorel
  2018-08-31  8:42   ` Cyril Hrubis
@ 2018-08-31  8:48   ` Xiao Yang
  2018-08-31  9:34     ` Cyril Hrubis
  1 sibling, 1 reply; 9+ messages in thread
From: Xiao Yang @ 2018-08-31  8:48 UTC (permalink / raw)
  To: ltp

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
>




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31  8:48   ` Xiao Yang
@ 2018-08-31  9:34     ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2018-08-31  9:34 UTC (permalink / raw)
  To: ltp

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31  8:41 ` Cyril Hrubis
@ 2018-08-31  9:37   ` Cyril Hrubis
  2018-08-31 10:39     ` Petr Vorel
  0 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2018-08-31  9:37 UTC (permalink / raw)
  To: ltp

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31  9:37   ` Cyril Hrubis
@ 2018-08-31 10:39     ` Petr Vorel
  0 siblings, 0 replies; 9+ messages in thread
From: Petr Vorel @ 2018-08-31 10:39 UTC (permalink / raw)
  To: ltp

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
@ 2018-08-31 13:24 Junchi Chen
  2018-08-31  8:37 ` Petr Vorel
  2018-08-31  8:41 ` Cyril Hrubis
  0 siblings, 2 replies; 9+ messages in thread
From: Junchi Chen @ 2018-08-31 13:24 UTC (permalink / raw)
  To: ltp

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference.
  2018-08-31  8:42   ` Cyril Hrubis
@ 2018-09-07 10:12     ` Petr Vorel
  0 siblings, 0 replies; 9+ messages in thread
From: Petr Vorel @ 2018-09-07 10:12 UTC (permalink / raw)
  To: ltp

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-09-07 10:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-31 13:24 [LTP] [PATCH ltp] syscalls/bind03: Add version compare according to behaviour difference Junchi Chen
2018-08-31  8:37 ` Petr Vorel
2018-08-31  8:42   ` Cyril Hrubis
2018-09-07 10:12     ` Petr Vorel
2018-08-31  8:48   ` Xiao Yang
2018-08-31  9:34     ` Cyril Hrubis
2018-08-31  8:41 ` Cyril Hrubis
2018-08-31  9:37   ` Cyril Hrubis
2018-08-31 10:39     ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.