Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: splice fails on ppc64x and arm5spacemonkey with invalid argument #27513

Closed
bradfitz opened this issue Sep 5, 2018 · 7 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Sep 5, 2018

https://build.golang.org/log/4ea83ef01cc0d411fde525df7f2504a08f24f03b

--- FAIL: TestSplice (10.30s)
    --- FAIL: TestSplice/unix-to-tcp (10.02s)
        --- FAIL: TestSplice/unix-to-tcp/simple (0.00s)
            splice_test.go:86: readfrom tcp4 127.0.0.1:37825->127.0.0.1:48570: splice: invalid argument
        --- FAIL: TestSplice/unix-to-tcp/multipleWrite (5.00s)
            splice_test.go:86: readfrom tcp4 127.0.0.1:34067->127.0.0.1:49674: splice: invalid argument
        --- FAIL: TestSplice/unix-to-tcp/big (5.01s)
            splice_test.go:86: readfrom tcp4 127.0.0.1:40900->127.0.0.1:44471: splice: invalid argument
        --- FAIL: TestSplice/unix-to-tcp/honorsLimitedReader (0.00s)
            splice_test.go:86: readfrom tcp4 127.0.0.1:34359->127.0.0.1:47883: splice: invalid argument
        --- FAIL: TestSplice/unix-to-tcp/updatesLimitedReaderN (0.00s)
            splice_test.go:86: readfrom tcp4 127.0.0.1:41462->127.0.0.1:49635: splice: invalid argument
        --- FAIL: TestSplice/unix-to-tcp/limitedReaderAtLimit (0.00s)
            splice_test.go:86: readfrom tcp4 127.0.0.1:40804->127.0.0.1:47978: splice: invalid argument
FAIL
FAIL	net	18.375s

Since https://go-review.googlesource.com/113997

/cc @laboger @tklauser @benburkert

@bradfitz
Copy link
Contributor Author

bradfitz commented Sep 5, 2018

I suspect that kernel is just too old to support splice from unix sockets?

So either splice needs to fall back more gracefully to just doing a normal copy, or the tests should detect the kernel is too old and t.Skip.

@bradfitz bradfitz added this to the Go1.12 milestone Sep 5, 2018
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 5, 2018
@bradfitz
Copy link
Contributor Author

bradfitz commented Sep 5, 2018

bradfitz@gdev:~/go/src/net$ gomote create linux-ppc64-buildlet
user-bradfitz-linux-ppc64-buildlet-0
bradfitz@gdev:~/go/src/net$ gomote run -system user-bradfitz-linux-ppc64-buildlet-0 uname -a
Linux go-be-3 3.16.0-4-powerpc64 #1 SMP Debian 3.16.36-1+deb8u1 (2016-09-03) ppc64 GNU/Linux
bradfitz@gdev:~/go/src/net$ gomote run -system user-bradfitz-linux-ppc64-buildlet-0 lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 8.6 (jessie)
Release:        8.6
Codename:       jessie
bradfitz@gdev:~/go/src/net$ gomote create linux-ppc64le-buildlet
user-bradfitz-linux-ppc64le-buildlet-0
bradfitz@gdev:~/go/src/net$ gomote run -system user-bradfitz-linux-ppc64le-buildlet-0 uname -a
Linux go-le-1 3.16.0-4-powerpc64le #1 SMP Debian 3.16.7-ckt25-1 (2016-03-06) ppc64le GNU/Linux
bradfitz@gdev:~/go/src/net$ gomote run -system user-bradfitz-linux-ppc64le-buildlet-0 lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 8.4 (jessie)
Release:        8.4
Codename:       jessie

@acln0
Copy link
Contributor

acln0 commented Sep 5, 2018

It seems like splicing from UNIX sockets was introduced in torvalds/linux@2b51457, which looks to be somewhere between 4.0 and 4.1, from what I can tell. It's by no means ancient.

The current code does not handle this correctly:

// From here on, the operation should be considered handled,
// even if Splice doesn't transfer any data.

We assume that the copy operation is handled if we get to create a pipe, which is not accurate. Perhaps testing for EINVAL in poll.Splice and bailing out to a userspace copy would be robust enough.

@laboger
Copy link
Contributor

laboger commented Sep 5, 2018

I can get it to fail on a Debian 8 machine, but I've tried several others (Debian 9, Ubuntu 16.04, 18.04) and it passes everywhere else. I also tried it on a RHEL7 with an old kernel (3.10) and that worked.

@acln0
Copy link
Contributor

acln0 commented Sep 5, 2018

I have made some further investigations. I used CentOS 7 because I don't have access to an RHEL7 system.

[acln@localhost ~]$ uname -a
Linux localhost.localdomain 3.10.0-862.el7.x86_64 #1 SMP Fri Apr 20 16:44:24 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

It seems like the 3.10 kernel they use has backported splice support for UNIX sockets, as far as I could tell from downloading the official sources. I think this is the reason why the tests pass on RHEL 7 (3.10), but not on Debian 8 (3.16).

In any case, we need a fix to handle this case, since I'm pretty sure it's not ppc64x-specific. I believe the builders for ppc64x just happened to be running old enough kernels for this to surface.

I'm not sure testing for EINVAL inside poll.Splice is the most elegant way, but it may be good enough. If nothing else happens in the meantime, I'll send a CL tomorrow, and perhaps we can discuss if the fix is appropriate there.

@gopherbot
Copy link

Change https://golang.org/cl/133575 mentions this issue: internal/poll: graceful fallback for unsupport splice on unix sockets

@mvdan
Copy link
Member

mvdan commented Sep 6, 2018

Also affecting linux-arm-arm5spacemonkey: https://build.golang.org/log/b275caaae0e2e0c92b4441256f7a1c03fa36b655

@bcmills bcmills changed the title net: splice fails on ppc64x with invalid argument net: splice fails on ppc64x and apwm with invalid argument Sep 14, 2018
@bcmills bcmills changed the title net: splice fails on ppc64x and apwm with invalid argument net: splice fails on ppc64x and arm5spacemonkey with invalid argument Sep 14, 2018
@golang golang locked and limited conversation to collaborators Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants