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

Excessive memory usage for QUIC connections #157

Open
bwelling opened this issue Apr 29, 2024 · 0 comments
Open

Excessive memory usage for QUIC connections #157

bwelling opened this issue Apr 29, 2024 · 0 comments

Comments

@bwelling
Copy link

The problem that I’m seeing is that every QUIC connection is using far more memory than seemingly necessary. I narrowed a bunch of this down to this code in SSL_provide_quic_data:

    if (sc->quic_buf == NULL) {
        BUF_MEM *buf;
        if ((buf = BUF_MEM_new()) == NULL) {
            ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
            return 0;
        }
        if (!BUF_MEM_grow(buf, SSL3_RT_MAX_PLAIN_LENGTH)) {
            ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
            BUF_MEM_free(buf);
            return 0;
        }
        sc->quic_buf = buf;
        /* We preallocated storage, but there's still no *data*. */
        sc->quic_buf->length = 0;
        buf = NULL;
    }

Specifically, this code is preallocating a buffer to accumulate potentially fragmented TLS packets from QUIC packets, in order to parse the TLS packets. It’s passing a size of SSL3_RT_MAX_PLAIN_LENGTH, which is 16k, and for reasons that I don’t understand, the BUF code scales this by 4/3, causing an allocation of over 21k. In the cases I’m testing (DoQ and DoH), even if I send a large number of DNS queries over a QUIC connection, TLS data is only written into the buffer during the initial handshake, and it’s only a few hundred bytes. That means that most of the 21k allocated ends up unused for the lifetime of the connection.

To reduce this, I made a local change to comment out the preallocation. It seemed to work, although I suspect that it potentially isn’t safe if the buffer is reallocated. It looks like the code previously attached the TLS data directly onto the QUIC_DATA structures (changed in 075ede1), and that was removed because it was involved incomplete QUIC_DATA objects.

The older code, which would use far less memory, didn’t look all that complicated, but could potentially be simplified by storing the partial buffer on the connection, instead of on an incomplete QUIC_DATA.

I don’t think there’s anything specific to my use case that’s causing this, but am also not sure if the memory usage is an issue for anyone else. If it’s not, I’ll stick with a local workaround, but it might be good if it were improved for everyone.

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

No branches or pull requests

1 participant