Support unix sockets #13

Open
networkException wants to merge 2 commits from networkException/netapp:unix-sockets into main
First-time contributor

This pull request adds support for listening on and connecting to unix sockets.

This requires having wrapper types for various tokio specific network abstractions while also supporting things like serialization and deserialization.

Unfortionately i was unable to find a published crate fulfilling these
requirements.

For this reason I've published a crate myself. Called tokio-unix-tcp, it serves as a drop in replacement for Tokio's TCP and Unix network types.

I plan to maintain this library outside the scope of this project as well, in general the code should be simple and stable enough however to not require maintainance going forward.

As i said this crate aims to support the requirement mentioned above. In addition to this it also strives to be more correct about handling the different types of unix sockets, which the libraries i reviewed were weak at. A list of these crates can be found in the crate README under "Related work".


The changes to netapp can be summarized as the following:

  • std::net::SocketAddr has been replaced by tokio_unix_tcp::NamedSocketAddr in most places. This enum encapsulates a IP address and port as well as a path in its variants and describes a concrete socket address netapp can bind or connect to.

  • In some places tokio_unix_tcp::SocketAddr is used instead of tokio_unix_tcp::NamedSocketAddr as mentioned above. This is due to the way unix sockets work:

    The remote peer of a client from the perspective of a server is not a concrete path but unnamed. They just share a file descriptor for the actual communication channel. The local address of the server is the actual file system path the server is listening on.

    In some cases netapp might be configured to connect to another peer using a unix socket and to not send a reachable IP address and port or unix socket path using the HelloMessage.

    As per the above (the client's remote address will be unnamed), we have no way of connecting back to that peer. This will currently cause the connection to be aborted by the server.

  • Listening on Unix sockets requires some additional handling like removing a previous file at the bind path and setting a correct mode (defaulting to 0o222 currently). This is handled by tokio_unix_tcp.


I've tested these changes by including them in garage and running basic administration commands against a node and by running the unit tests here.

Basalt peering is currently lacking a proper cost calculation for unix sockets - I'm sadly not familiar with this code.

This pull request adds support for listening on and connecting to unix sockets. This requires having wrapper types for various tokio specific network abstractions while also supporting things like serialization and deserialization. Unfortionately i was unable to find a published crate fulfilling these requirements. For this reason I've published a crate myself. Called [`tokio-unix-tcp`](https://crates.io/crates/tokio-unix-tcp), it serves as a drop in replacement for Tokio's TCP and Unix network types. I plan to maintain this library outside the scope of this project as well, in general the code should be simple and stable enough however to not require maintainance going forward. As i said this crate aims to support the requirement mentioned above. In addition to this it also strives to be more correct about handling the different types of unix sockets, which the libraries i reviewed were weak at. A list of these crates can be found in the crate [README](https://github.com/networkException/tokio-unix-tcp) under ["Related work"](https://github.com/networkException/tokio-unix-tcp#related-work). --- The changes to netapp can be summarized as the following: - `std::net::SocketAddr` has been replaced by `tokio_unix_tcp::NamedSocketAddr` in most places. This enum encapsulates a IP address and port as well as a path in its variants and describes a concrete socket address netapp can bind or connect to. - In some places `tokio_unix_tcp::SocketAddr` is used instead of `tokio_unix_tcp::NamedSocketAddr` as mentioned above. This is due to the way unix sockets work: The remote peer of a client from the perspective of a server is not a concrete path but `unnamed`. They just share a file descriptor for the actual communication channel. The local address of the server is the actual file system path the server is listening on. In some cases netapp might be configured to connect to another peer using a unix socket and to not send a reachable IP address and port or unix socket path using the `HelloMessage`. As per the above (the client's remote address will be `unnamed`), we have no way of connecting back to that peer. This will currently cause the connection to be aborted by the server. - Listening on Unix sockets requires some additional handling like removing a previous file at the bind path and setting a correct mode (defaulting to `0o222` currently). This is handled by `tokio_unix_tcp`. --- I've tested these changes by including them in garage and running basic administration commands against a node and by running the unit tests here. Basalt peering is currently lacking a proper cost calculation for unix sockets - I'm sadly not familiar with this code.
networkException force-pushed unix-sockets from 25c2376c6c to ee643a61ee 2023-10-20 19:42:13 +00:00 Compare
networkException force-pushed unix-sockets from ee643a61ee to 47a470b281 2023-11-05 21:29:23 +00:00 Compare
Owner

Hi @networkException, I have not forgotten this PR, it's on my radar for inclusion at some point in the next few months. Sorry for the wait.

Hi @networkException, I have not forgotten this PR, it's on my radar for inclusion at some point in the next few months. Sorry for the wait.
Author
First-time contributor

Thats alright, I'm quite happy with the unix socket support that landed in garage so far. I can also fully understand if the complexity of this is not worth it

Thats alright, I'm quite happy with the unix socket support that landed in garage so far. I can also fully understand if the complexity of this is not worth it
Some checks failed
continuous-integration/drone/pr Build is failing
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u unix-sockets:networkException-unix-sockets
git checkout networkException-unix-sockets

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout main
git merge --no-ff networkException-unix-sockets
git checkout networkException-unix-sockets
git rebase main
git checkout main
git merge --ff-only networkException-unix-sockets
git checkout networkException-unix-sockets
git rebase main
git checkout main
git merge --no-ff networkException-unix-sockets
git checkout main
git merge --squash networkException-unix-sockets
git checkout main
git merge --ff-only networkException-unix-sockets
git checkout main
git merge networkException-unix-sockets
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lx/netapp#13
No description provided.