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

Comments

Projects
None yet
5 participants
@bradfitz
Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

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 label Sep 5, 2018

@bradfitz

This comment has been minimized.

Copy link
Member Author

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

commented Sep 5, 2018

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

@mvdan

This comment has been minimized.

Copy link
Member

commented Sep 6, 2018

@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

@gopherbot gopherbot closed this in 1bf5796 Sep 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.