Concurrent io.Copy(tcpConn, file) and tcpConn.Write(...)

[Liam]

During an io.Copy() where the Writer is a TCPConn and the Reader is a 200K disk file, my code may concurrently Write() on the same TCPConn.

I see the result of the Write() inserted into the result of the io.Copy(). I had the impression that was impossible, but I must be mistaken, as the sendfile(2) docs read:

Note that a successful call to sendfile() may write fewer bytes than requested; the caller should be prepared to retry the call if there were unsent bytes.

Could someone confirm that one must indeed synchronize concurrent use of tcpConn.Write() and io.Copy(tcpConn, file)?

$ uname -a

Linux ... 5.0.6-200.fc29.x86_64 #1 SMP Wed Apr 3 15:09:51 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

$ go version

go version go1.13.3 linux/amd64

[Tamás Gulácsi]

Yes you should. Just test it with -race!

[Ian Lance Taylor]

On Sun, Apr 26, 2020 at 4:55 PM Liam <networ...@gmail.com> wrote:
>
> During an io.Copy() where the Writer is a TCPConn and the Reader is a 200K disk file, my code may concurrently Write() on the same TCPConn.
>
> I see the result of the Write() inserted into the result of the io.Copy(). I had the impression that was impossible, but I must be mistaken, as the sendfile(2) docs read:
>
> Note that a successful call to sendfile() may write fewer bytes than requested; the caller should be prepared to retry the call if there were unsent bytes.
>
> Could someone confirm that one must indeed synchronize concurrent use of tcpConn.Write() and io.Copy(tcpConn, file)?

Synchronization should not be required. internal/poll.Sendfile
acquires a write lock on dstFD, which is the TCP socket. That should
ensure that the contents of an ordinary Write (which also acquires a
write lock) should not interleave with the sendfile data.

That said, if the sendfile system call cannot be used for whatever
reason, the net package will fall back on doing ordinary Read and
Write calls. And those Write calls can be interleaved with other
Write calls done by a different goroutine. I think that is probably
permitted, in that io.Copy doesn't promise to not interleave with
simultaneous Write calls on the destination.

So in the general case you should indeed use your own locking to avoid
interleaving between io.Copy and a concurrent Write.

Ian

[Liam]

Thanks for the details. Where could I add a Println() to reveal why it doesn't call poll.Sendfile()?

I expect this system to use sendfile(2). The file is a normal file on a local partition (running on a Digital Ocean Droplet).

/etc/fstab has:

UUID=[omitted] /                       ext4    defaults        1 1

$ df -h

Filesystem      Size  Used Avail Use% Mounted on
devtmpfs        981M     0  981M   0% /dev
tmpfs           996M     0  996M   0% /dev/shm
tmpfs           996M  436K  995M   1% /run
tmpfs           996M     0  996M   0% /sys/fs/cgroup
/dev/vda1        59G  5.7G   51G  11% /
tmpfs           200M     0  200M   0% /run/user/0

[Ian Lance Taylor]

Well, I don't know for that it doesn't use sendfile, I just can't
explain the results you're seeing if it does use sendfile.

The place to start is to find out why (or whether)
internal/poll.Sendfile is returning 0, nil.

Ian

[Liam]

Erm, feeling stupid, but I didn't find any invocations of poll.Sendfile() in stdlib...

$ grep -r poll.Sendfile /usr/local/go/src/
$ grep -r 'Sendfile(' /usr/local/go/src/
/usr/local/go/src/net/sendfile_test.go:func TestSendfile(t *testing.T) {
/usr/local/go/src/net/http/fs_test.go:func TestLinuxSendfile(t *testing.T) {
/usr/local/go/src/internal/poll/sendfile_linux.go:              n, err1 := syscall.Sendfile(dst, src, nil, n)
/usr/local/go/src/internal/poll/sendfile_solaris.go:            n, err1 := syscall.Sendfile(dst, src, &pos1, n)
/usr/local/go/src/internal/poll/sendfile_bsd.go:                n, err1 := syscall.Sendfile(dst, src, &pos1, n)
/usr/local/go/src/syscall/syscall_js.go:func Sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) {
/usr/local/go/src/syscall/syscall_nacl.go:func Sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) {
/usr/local/go/src/syscall/syscall_unix.go:func Sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) {

[Liam]

Found the invocations, wrong letter case...

$ grep -r 'SendFile(' /usr/local/go/src/
/usr/local/go/src/net/sendfile_windows.go:      done, err := poll.SendFile(&fd.pfd, syscall.Handle(f.Fd()), n)
/usr/local/go/src/net/sendfile_linux.go:                written, werr = poll.SendFile(&c.pfd, int(fd), remain)
/usr/local/go/src/net/sendfile_unix_alt.go:             written, werr = poll.SendFile(&c.pfd, int(fd), pos, remain)
/usr/local/go/src/internal/poll/sendfile_windows.go:func SendFile(fd *FD, src syscall.Handle, n int64) (int64, error) {
/usr/local/go/src/internal/poll/sendfile_linux.go:func SendFile(dstFD *FD, src int, remain int64) (int64, error) {
/usr/local/go/src/internal/poll/sendfile_solaris.go:func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error) {
/usr/local/go/src/internal/poll/sendfile_bsd.go:func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error) {

$ grep -r 'sendFile(' /usr/local/go/src/net/
/usr/local/go/src/net/sendfile_windows.go:func sendFile(fd *netFD, r io.Reader) (written int64, err error, handled bool) {
/usr/local/go/src/net/sendfile_linux.go:func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) {
/usr/local/go/src/net/sendfile_unix_alt.go:func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) {
/usr/local/go/src/net/sendfile_stub.go:func sendFile(c *netFD, r io.Reader) (n int64, err error, handled bool) {
/usr/local/go/src/net/tcpsock_posix.go: if n, err, handled := sendFile(c.fd, r); handled {

Should I check the 'handled' result of sendFile() in net/tcpsock_posix.go ?

[Ian Lance Taylor]

Sure. Should be the same thing as the result of poll.SendFile.

Ian

[Liam]

Well this is a surprise... Added some println()s

// my network setup

   aCfgTcp := net.ListenConfig{KeepAlive: -1}
   aListener, err := aCfgTcp.Listen(nil, iConf.Listen.Net, iConf.Listen.Laddr)
   if err != nil { return err }
   aCert, err := tls.LoadX509KeyPair(iConf.Listen.CertPath, iConf.Listen.KeyPath)
   if err != nil { return err }
   aCfgTls := tls.Config{Certificates: []tls.Certificate{aCert}}
   aListener = tls.NewListener(aListener, &aCfgTls)

// conn writer goroutine, before io.Copy

   println(".. io.Copy to net")

// conn reader goroutine, before io.CopyN

   println(".. io.CopyN to file")

// package io

func copyBuffer(dst Writer, src Reader, buf []byte) (written int64, err error) {
        // If the reader has a WriteTo method, use it to do the copy.
        // Avoids an allocation and a copy.
        if wt, ok := src.(WriterTo); ok {
println(".. WriteTo")
                return wt.WriteTo(dst)
        }
        // Similarly, if the writer has a ReadFrom method, use it to do the copy.
        if rt, ok := dst.(ReaderFrom); ok {
println(".. ReadFrom")
                return rt.ReadFrom(src)
        }
println(".. manual")
[etc]

// package net

func (c *TCPConn) readFrom(r io.Reader) (int64, error) {
        if n, err, handled := splice(c.fd, r); handled {
println(".. spliced")
                return n, err

        }
        if n, err, handled := sendFile(c.fd, r); handled {

println(".. sendFiled")
                return n, err
        }
println(".. generic")
        return genericReadFrom(c, r)

LOG:

.. io.CopyN to file
.. manual
.. io.Copy to net
.. manual
.. io.CopyN to file
.. manual
.. io.Copy to net
.. manual
.. io.CopyN to file
.. manual
.. io.Copy to net
.. manual
.. io.Copy to net
.. manual
.. io.Copy to net
.. manual
.. io.Copy to net
.. manual

Not a sendfile(2) or splice(2) in sight :-(

[Liam]

I left out my accept loop:

   var aConn net.Conn
   for {
      aConn, err = aListener.Accept()
      ...

Is this the root cause, as net.Listener only provides Accept() (Conn, error), and net.Conn doesn't provide ReadFrom()?

[Liam]

Adding these to the Accept() loop shows that I see a tls.Conn but not a net.TCPConn:

if _, ok := aConn.(*net.TCPConn); ok { fmt.Println(".. have tcpconn") }
if _, ok := aConn.(*tls.Conn); ok { fmt.Println(".. have tlsconn") }

Why doesn't tls.Conn either implement ReadFrom() or provide a way to obtain the underlying TCPConn?

[Tamás Gulácsi]

TLS needs encyption, not jost "shoveling the bytes" to the underlying connection.

[Robert Engels]

Depends on how the file descriptor is implemented. But the end result probably has the same performance unless the network card is doing the TLS - which is possible. 

On Apr 28, 2020, at 2:21 PM, Tamás Gulácsi <tgula...@gmail.com> wrote:



TLS needs encyption, not jost "shoveling the bytes" to the underlying connection.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/3721e316-bfa1-433e-a3a6-0ad90339f5a6%40googlegroups.com.

[Liam]

The Linux kernel has TLS; one reason is to allow sendfile(2) with TLS. But I guess Go doesn't enable that yet?

https://www.kernel.org/doc/html/latest/networking/tls.html

[Brian Candler]

On Tuesday, 28 April 2020 21:09:38 UTC+1, Liam wrote:

The Linux kernel has TLS; one reason is to allow sendfile(2) with TLS. But I guess Go doesn't enable that yet?

https://www.kernel.org/doc/html/latest/networking/tls.html

https://blog.filippo.io/playing-with-kernel-tls-in-linux-4-13-and-go/

"So yeah, not a huge fan. But of course, it wouldn't be me if I didn't fork the Go crypto/tls package to work with it."