| Message ID | 20180831132453.6461-1-junchi.chen@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | syscalls/bind03: Add version compare according to behaviour difference. | expand |
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
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/
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...
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 >
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.
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/
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
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
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);
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(-)