more work on net::tcp and net::ip #2731
Closed
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
.. confounded resolve!
* updated rustdoc info for several functions * changed read_stop to take control of the port returned by read_start * made write_future do an explicit data copy with the binary vector it is passed
…yet. need to do some other work, in the subsequent commit, to add io::writer, then tests.
…fails. needs work. probably the slice code and/or the way that the tcp_socket_buf is wrapped in a shared box
…test i mistook an "unconstrained type" error, due to type-inference messup because i didnt have return vals in some closure wired-up right, for being due to not having a str as a str/& (a str will actually auto-coerce to a str/&, so str::as_slice was erroneously added. my bad).
…dr_in6 .. but the test is kind of broken.. it appears that rust pads structs for alignment purposes? I can't get the struct to == 28.. that appears to be the native size of sockaddr_in6.. so we have a size 32 struct, for now.
replaces net::ip's previously, hand-rolled impl for ipv4 addr parsing.. we're relying on libuv, now
.. stub out some brokeness in net::tcp as a result of ipv6 coming online
…eats) libuv's own ip vetting code appears to in a somewhat woeful state, for both ipv4 and ipv6 (there are some notes in the tests for net_ip, as well as stuff added in uv_ll). They are aware of this and welcome patches. I have rudimentary code in place that can verify whether the provided str ip was, in fact, validly parsed by libuv, making a few assumptions: * for ipv4, we assume that the platform's INADDR_NONE val is 0xffffffff , I should write a helper to return this value from the platform's libc headers instead of hard-coding it in rust. * for ipv6, we assume that the library will always return '::' for malformed inputs.. as is the case in 64bit ubuntu. I need to verify this on other platforms.. but at least the debugging output is in place, so if expectations don't line up, it'll be straightforward to address
still needs tests
now the best of what we had prior to libuv integration (proper validation of an ipv4 string), along with libuv support (initial ipv6 support) libuv has even weaker facilities for validating an input ipv6 (but still more than what we had), so eventually the "right" answer would be to roll a proper ipv6 address string parser in rust
.. there are some additional FIXME nags in net_tcp (L 1012) about blocking because libuv is holding unsafe ptrs to task local data. the proposed fix going is not really feasible w/ the current design, IMO, but i'll leave it there in case someone really wants to make the case without creating more hassle than it's worth.
|
closing and re-opening as #2751 |
RalfJung
pushed a commit
to RalfJung/rust
that referenced
this pull request
Dec 24, 2022
Aaron1011
pushed a commit
to Aaron1011/rust
that referenced
this pull request
Jan 6, 2023
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.
(hopefully) just in time for 0.3!
What's included
net::ipto utilize libuv's facilities for representing IP addresses. As a consequence, we have IPv6 support, now! (see caveat below)net::ip::get_addr()that will return an[ip_addr]/~, utilizing libuv'suv_getaddrinfocall under the hood. We have (albeit simplistic) name resolution!net::tcpAPI (the server workflow that returned a port-like object to listen on for new connections).. we're still stuck with the callback that runs on the libuv thread .. I was going to add another overload that will justaccept()every new connection, sidestepping the need for running a blocking callback, but then I took an array to the knee (just kidding.. I ran into a wild CC bug and decided to back out the work). But the code around the tcp server workflow was pretty well refactored that I can add this new workflow with relatively little additional LOCs.impls forio::readerandio::writerthat wrapnet::tcp::tcp_socketin a simple buffer. This is happening as part of a larger conversation/proof-of-concept for revisiting Streams as a generalized concept, in rust.What's next?
caveat about IPv6:
By the admission of libuv's devs, IPv6 support is somewhat spotty, atm. Insofar as a malformed address that is passed in to
uv_ip6_addr/namewill still parse correctly. So users don't have any sanity checks that what they're plugging in will actually work. We're still in a better place than we were, though. The "right answer", in the mid-term, is to roll a pure-rust string parser for IPv6 addresses, like what we already have for IPv4. That way we can be sure we're passing valid input to libuv.questions/feedback apprecaited, as always.