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

Add switch to disable TCP_NODELAY #112

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Add switch to disable TCP_NODELAY #112

merged 1 commit into from
Feb 9, 2024

Conversation

benjojo
Copy link
Collaborator

@benjojo benjojo commented Feb 9, 2024

This in an attempt quell CPU usage, since PCCW is having issues when their {N} number of routers all start RTR sessions at the same time.

A perf of the system suggests all of the CPU is going to sending TCP traffic, and since golang enables NO_DELAY (infamously?) by default, this is a good smoking gun, since this would imply that stayrtr is sending 1 TCP packet per RTR PDU, something that would indeed cause a lot of CPU usage in aggergate!

This patch was written some time ago, but reports from PCCW states that this does actually help their network a noticeable amount, So I think this is a worthy switch to be introduced

This in an attempt quell CPU usage, since PCCW is having issues when
their {N} number of routers all start RTR sessions at the same time.

A `perf` of the system suggests all of the CPU is going to sending
TCP traffic, and since golang enables NO_DELAY (infamously?) by
default, this is a good smoking gun, since this would imply that
stayrtr is sending 1 TCP packet per RTR PDU, something that would
indeed cause a lot of CPU usage in aggergate!
@job
Copy link
Member

job commented Feb 9, 2024

Yup

@job job merged commit 7bec069 into master Feb 9, 2024
3 checks passed
@job
Copy link
Member

job commented Feb 9, 2024

It's actually a switch to enable NODELAY, but it is still morning so we'll let that slide ;-)

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

Successfully merging this pull request may close these issues.

2 participants