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

Change TCPServer backlog to 1 #2785

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Conversation

Lex-ari
Copy link
Contributor

@Lex-ari Lex-ari commented Jun 21, 2024

Related Issue(s) #2603
Has Unit Tests (y/n) n
Documentation Included (y/n) n

Change Description

Change TCP backlog to 1 from 0 in TcpServerSocket.cpp

Rationale

Some Linux kernels would accept 0 as a backlog size, which would drop all incoming TCP SYN connections and prevent anything from connecting to FSW. Changing the backlog will allow SYN packets to be accepted.
This issue may have been the cause of #2603 (I am using the exact same hardware).

Huge props to @Joshua-Anderson and @kevin-f-ortega for helping and finding this fix!

Testing/Review Recommendations

Hardware: Nvidia Jetson Nano 2GB, Ubuntu 18.04, Jetson Linux 4.9.337-tegra R32.7.4
Project and deployment build for aarch64 and running FSW and GDS, verified TCP connection.
May also use other web / Linux tools to connect to the IP and port, with corresponding accept message.

Copy link
Contributor

@Joshua-Anderson Joshua-Anderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Alex!

@LeStarch
Copy link
Collaborator

Am I to interpret this PR to mean that some Linux kernel implementations interpret 0 to mean "deny all incoming connections"?

@Joshua-Anderson
Copy link
Contributor

Am I to interpret this PR to mean that some Linux kernel implementations interpret 0 to mean "deny all incoming connections"?

Yup - exactly! Just don't ask how many hours of TCP debugging it took for us figure it out

@LeStarch
Copy link
Collaborator

Reading around, it seems 0 means "use some minimum" and this value may be a "hint" anyway. Thus this change should not cause harm. I'd like to know the answer to my question, but it is not blocking.

@LeStarch LeStarch merged commit c63a951 into nasa:devel Jun 25, 2024
34 checks passed
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.

3 participants