[WIP] liblokinet changes #2141
Draft
[WIP] liblokinet changes #2141
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
majestrate
commented
Feb 27, 2023
- add new format with category - add set_data_dir method to liblokinet
bump ngtcp2 dep to the latest tagged version. The changes to get liblokinet/quic back to "working" (insofar as it was before) are complete in this commit, but there appear to be further changes in ngtcp2 apart from simple API changes which could merit reworking how we use it somewhat.
- fix some ngtcp2 usage and conn issue detection - clients now get an ERR_PROTO packet when something goes wrong and the server is unhappy. - ngtcp2_accept changed between our last update of it and current, this fixes its usage. - with a minor change to the C API, liblokinet userland will now be able to receive notification when a connection is (un)successfully opened, and when it closes. TODO: supply a reason for the close, if we decide it should
- pkt tx timer update + max pkt count - added pkt tx time updates to message handling logic - setting destlen param to at least max_tx_UDP_Payload size
majestrate
commented
Mar 1, 2023
- removed fprintf debug statements used for xcode debugging that were failing CI - removed unused variables also failing CI - deb stable i386 (and others) broke with logging of variants
dr7ana
force-pushed
the
liblokinet-dev
branch
4 times, most recently
from
d816999
to
596dd72
Compare
- handshake timeouts was incorrect: must call ngtcp2_write_connection_close if NGTCP2_ERR_HANDSHAKE_TIMEOUT is returned - packet handling fix: do not close stream on congestion - bumped clang-format version to latest (16) - format-version.sh checks for version 14 or greater now, removed old comment block
majestrate
commented
Apr 6, 2023
| @@ -122,7 +122,7 @@ namespace llarp | |||
| virtual bool | |||
| EnsurePathTo( | |||
| AddressVariant_t addr, | |||
| std::function<void(std::optional<service::ConvoTag>)> hook, | |||
| std::function<void(std::optional<std::variant<service::Address, RouterID>>)> hook, | |||
There was a problem hiding this comment.
std::function<void(bool)>
majestrate
commented
Apr 6, 2023
llarp/net/ip_address.hpp
Outdated
| @@ -24,7 +24,7 @@ namespace llarp | |||
| /// Empty constructor. | |||
| IpAddress() = default; | |||
| /// move construtor | |||
| IpAddress(IpAddress &&) = default; | |||
There was a problem hiding this comment.
bin this entire file if you can
We changed the quic code to refer to its remote endpoint using an Address Variant and this resulted in a crash when sending a connection close packet. This code was incorrectly supplying an uninitialized value for ConvoTag before but this was silently ignored; after switching to a variant it became a crash.
this still feels incorrect, but more correct than before
locking in lokinet_shared needs revisited in general. In addition, user-supplied callbacks we call from the main thread currently should be called on a separate thread instead, both to keep the main loop fast and to avoid complex locking issues.
In commit a76acd4 we attempted to fix issues on service nodes that related to spamming service nodes with traffc, but this caused a regression where clients could not connect to the network. However, we also did something idiotic with the protocol handshake. The handshake logic was modified and caused client connections to fail. This was due to a change in how we decided what a timed out session is. We redo the close bug and revert the change to the protocol handshake. (cherry picked from commit b0b6e2b)
when we queue cpu heavy work in lokinet to worker threads we make 1 job per function call. we call a lot of jobs so this coleases the jobs into 1 job that we push off at the end of the event loop cycle, reducing the number of jobs going across the omq proxy thread, in theory reducing cpu usage. (cherry picked from commit bbbb07d)
Merging cherry-picks back to testnet branch: - oxen-io#2164 - oxen-io#2134
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
placeholder for dan's branch for liblokinet used for code review.