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

crypto/rand: Reader is internally buffered inconsistently on different platforms #16593

Open
takeyourhatoff opened this issue Aug 4, 2016 · 4 comments
Assignees
Milestone

Comments

@takeyourhatoff
Copy link

@takeyourhatoff takeyourhatoff commented Aug 4, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    1.6.3
  2. What operating system and processor architecture are you using (go env)?
    GOARCH="amd64"
    GOBIN=""
    GOEXE=""
    GOHOSTARCH="amd64"
    GOHOSTOS="linux"
    GOOS="linux"
    GOPATH="/home/tyho/go"
    GORACE=""
    GOROOT="/usr/lib/golang"
    GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
    GO15VENDOREXPERIMENT="1"
    CC="gcc"
    GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
    CXX="g++"
    CGO_ENABLED="1"
  3. What did you expect to see?
    rand.Reader to consistently internally buffer, or not internally buffer calls to Read regardless of platform.
  4. What did you see instead?
    On generic Unix, including old versions of Linux (without a getrandom syscall), rand.Reader reads from /dev/urandom, and buffers calls to Read using the bufio package. On modern Linux however, rand.Read requests are implemented using the new getrandom syscall, but the calls are not buffered.
  5. What did you do?
    I tested to see if internally buffering output on platforms with the getrandom syscall available would make a significant difference to the performance of small 16B reads:

https://play.golang.org/p/80fyLlBFQb

On my machine, internally buffering the output of Read doubled performance compared with raw Reads. Tv` suggested that the unbuffered Reads may be quicker when multithreaded due to bufio adding a mutex. He wrote a benchmark to test this theory but it turned out not to be true:

https://play.golang.org/p/3sO47iNhK5

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 4, 2016

Dup of #16124 ?

@josharian
Copy link
Contributor

@josharian josharian commented Aug 4, 2016

Not a dup, I think. That was math/rand; this is crypto/rand.

@bradfitz bradfitz added the Performance label Aug 4, 2016
@bradfitz bradfitz changed the title crypto/rand: Reader is internally buffered inconsistantly on different platforms crypto/rand: Reader is internally buffered inconsistently on different platforms Aug 4, 2016
@bradfitz bradfitz added this to the Unplanned milestone Aug 4, 2016
@bradfitz bradfitz added the help wanted label Aug 4, 2016
@agl agl self-assigned this Aug 19, 2016
@agl
Copy link
Contributor

@agl agl commented Sep 30, 2016

Not buffering is actually a nice feature of the getrandom code. Although Go programs don't fork, thus eliminating one of the classic sources of problems, these days there's hibernation, VM migrations etc.

What's the situation where the syscall performance is a problem? (Is it CBC-mode in TLS again?)

@CAFxX
Copy link
Contributor

@CAFxX CAFxX commented Aug 2, 2019

Still applies to 1.12 and tip. See #33256 for benchmarks showing order-of-magnitude differences for small reads via a bufio.Reader.

Although Go programs don't fork, thus eliminating one of the classic sources of problems, these days there's hibernation, VM migrations etc.

Don't these apply equally to the entropy pool in the kernel? Why is it a concern only for entropy buffered in process? (Also note that the /dev/urandom fallback already buffers.)

What's the situation where the syscall performance is a problem?

We ran into this in the profile of a workload where we need to generate tokens for authentication (unrelated to TLS).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.