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

Convert edge to use a new mainloop #59

Merged
merged 85 commits into from
Dec 9, 2024
Merged

Conversation

hamishcoleman
Copy link
Contributor

Refactoring the mainloop to be in one place allows the tracking of file handles and the protocol associated with them better. It also allows multiple sockets to exist for each protocol and will help in the future with allowing IPv4 and IPv6 to co-exist. It also paves the way for adding different activity waiting implementations in the future (eg, kqueue, epoll, WaitForMultipleObjects)

Finally, this refactoring has improved TCP mode on Windows. Limited testing has shown that it appears to work fine.

Nothing in the usual peer_info or n2n_sock_t indicates that a peer is to be
connected via TCP - it is all done by a separate connection table.  This
makes it hard to get status output that shows that a given peer should
be reachable via TCP.

This patch was an attempt to connect some of the dots, sadly it was not
enough.  It is probably useful framework, but more still needs to be
done.

Of most annoyance is that the new TCP test in test_integration_packets.sh
is sending and receiving over TCP, but gets a wire sock structure that
is still flagged as UDP
Note that this not ideal, and we should use an OS pre-define macro,
which may have some type safety in it.

But this is another case where Windows did something subtly different to
basically everyone else.
Demonstrate that all the features are working by converting the tun fd read to
use the new mainloop process.
It also reminds us that this n2nsock structure is used in memcmp
operations for some of the hash lookups - which could possibly be
optimised in the IPv4 case.
…it may be used on streams that are not reading
A send path that queues via the mainloop still needs to be built.
This introduces a memcpy() operation on the TCP send path, but the
intent is that a packet buffer allocation system will be used in the
future that will allow removing that.
@hamishcoleman hamishcoleman self-assigned this Dec 9, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 54.63415% with 186 lines in your changes missing coverage. Please review.

Project coverage is 46.81%. Comparing base (f94e148) to head (74be511).
Report is 227 commits behind head on main.

Files with missing lines Patch % Lines
src/mainloop.c 66.38% 80 Missing ⚠️
src/edge_utils.c 45.71% 57 Missing ⚠️
src/metrics.c 6.66% 28 Missing ⚠️
src/management.c 7.69% 12 Missing ⚠️
src/n2n.c 66.66% 4 Missing ⚠️
src/sn_utils.c 33.33% 4 Missing ⚠️
src/wire.c 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
- Coverage   48.94%   46.81%   -2.13%     
==========================================
  Files          40       41       +1     
  Lines        6038     7880    +1842     
==========================================
+ Hits         2955     3689     +734     
- Misses       3083     4191    +1108     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hamishcoleman hamishcoleman merged commit b0b6944 into n42n:main Dec 9, 2024
30 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.

2 participants