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

Allow linux users to turn off delayed ACK for TCP #21104

Closed
ORESoftware opened this issue Jun 2, 2018 · 10 comments
Closed

Allow linux users to turn off delayed ACK for TCP #21104

ORESoftware opened this issue Jun 2, 2018 · 10 comments
Labels
feature request Issues that request new features to be added to Node.js. libuv Issues and PRs related to the libuv dependency or the uv binding. stale

Comments

@ORESoftware
Copy link
Contributor

ORESoftware commented Jun 2, 2018

This is for all Node.js versions

re: https://jvns.ca/blog/2015/11/21/why-you-should-understand-a-little-about-tcp/

AFAIK, delayed ACK is the default on all Linux flavors. It would be interesting to have a way to turn off that flag, via a Node.js call.

I am not sure if there is a way to persistently set the flag, or if you need to set it for each TCP request. If the latter, perhaps it's prohibitively expensive. So perhaps it's only worth setting the flag on systems where the flag can be set persistently.

@richardlau richardlau added the feature request Issues that request new features to be added to Node.js. label Jun 2, 2018
@ORESoftware
Copy link
Contributor Author

ORESoftware commented Jun 2, 2018

I see this:
https://vertx.io/docs/vertx-core/js/

In the above, do a ctrl-f for "quickack", see:

screenshot from 2018-06-02 15-46-21

perhaps setting QUICKACK is possible with Node.js HTTP, but not a Node.js TCP server? I didn't see any mention of QUICKACK in the Node.js net or http docs.

@targos
Copy link
Member

targos commented Jun 2, 2018

@nodejs/libuv

@ORESoftware
Copy link
Contributor Author

ORESoftware commented Jun 2, 2018

So here is what the Vertx docs say:

Native on Linux gives you extra networking options:

SO_REUSEPORT

TCP_QUICKACK

TCP_CORK

TCP_FASTOPEN

// Available on Linux
vertx.createHttpServer({
"tcpFastOpen" : fastOpen,
"tcpCork" : cork,
"tcpQuickAck" : quickAck,
"reusePort" : reusePort
});

With these Node.js docs in mind:
https://nodejs.org/api/net.html#net_net_createserver_options_connectionlistener

right now there seem to be only two options for a Node.js net server:

allowHalfOpen
pauseOnConnect

so this feature request is for at least one more option:

quickAck

@ORESoftware
Copy link
Contributor Author

ORESoftware commented Jun 3, 2018

I tried creating a Node.js addon to change the setting:

#include <nan.h>
#include <node.h>
#include <string>
#include <regex.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <unistd.h>

//using namespace v8;
using namespace std;


void run(const v8::FunctionCallbackInfo<v8::Value>& args) {

    int socket_fd = (int)(args[0]->Int32Value());
    int i = 1;

    if(setsockopt(socket_fd, IPPROTO_TCP, TCP_QUICKACK,  (void *)&i, sizeof(i)) < 0){
        printf("setsockopt failed\n");
        args.GetReturnValue().Set(false);
    }
    else{
        args.GetReturnValue().Set(true);
    }

}

void init(v8::Local<v8::Object> exports) {
    NODE_SET_METHOD(exports, "run", run);
}

NODE_MODULE(run, init)

but that didn't really do anything according to my testing

@silverwind
Copy link
Contributor

Not sure about addons, but generally, this would have to be added to libuv first, similar to uv_tcp_nodelay which is exposed via socket.setNoDelay.

It'd probably also make sense to have per-server settings for socket options, but right now, you have to set them on each connection's socket manually.

@silverwind silverwind added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Jun 3, 2018
@ORESoftware
Copy link
Contributor Author

ORESoftware commented Jun 3, 2018

Beyond, TCP_QUICKACK, I believe there are a few other TCP flags/settings worth incorporating as well:

TCP fast open might be useful, but that might be more of a system thing, not a per process thing.

https://bradleyf.id.au/nix/shaving-your-rtt-wth-tfo/
https://blog.wasin.io/blog/2016/12/26/how-to-enable-fast-tcp-open-on-ubuntu.html

Here is a thread on Golang for TFO:
golang/go#4842

so maybe a more dynamic way to set any flag might be useful:

socket.setOption(opts.X, value);

@prog1dev
Copy link
Contributor

prog1dev commented Nov 1, 2018

I did a bit of research on TCP_QUICKACK and it looks like this flag is not permanent and resets every time you send or receive data.
In other words this flag needs to be set with setsockopt() after each operation of TCP on a given socket.

Refs
https://stackoverflow.com/a/1615591/4950680
https://access.redhat.com/solutions/407743
https://linux.die.net/man/7/tcp

@gireeshpunathil
Copy link
Member

IMO abstracting setsockopt (and potentially getsockopt too) into a Node.js API is a good idea, that will cater to all sort of behavior control requirements of the TCP stack.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Mar 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2022

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as completed Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. libuv Issues and PRs related to the libuv dependency or the uv binding. stale
Projects
None yet
Development

No branches or pull requests

6 participants