microsoft
[pdpix] Fix Network Types to be Compatible With Each Other
Now that we've replaced our home-grown Port16 type with u16, and the associated Ipv4Endpoint struct with std::net::SocketAddrV4, we should have an easier time of interfacing our code with Rust standard library code (which uses standard types).
But there are some places where existing code was converted in place that are no longer needed and/or make sense. And there are some compatibility functions we still have that are no longer needed (or are now inefficient).
Example inefficiency problem: catcollar/mod.rs (and catnap/mod.rs) contain a parse_addr function that (now) converts a std::net::SockAddrV4 into a ::nix::sys::socket::SockaddrStorage. It does this by pulling the addr and port out of the input SockAddrV4 and then creating a new SockAddrV4 from that addr and port. And then uses SockaddrStorage::from() to create the SockaddrStorage from the new SockAddrV4. There is no point to creating this additional SockAddrV4, as the SockaddrStorage::from() could just be called using the input SockAddrV4. Moreover, there is no real need for parse_addr anymore, as SockaddrStorage::from() by itself does the same thing directly.
Another example: catcollar/mod.rs (and catnap/mod.rs) contain a pack_result function that (essentially) converts an OperationResult into a demi_qresult_t. For OperationResult::Pop, it goes through some unnecessary convolutions to create the sga_addr field (a sockaddr_in) from the popped SocketAddrV4 (which is also just a sockadd_in), when it could just copy the bits over (if Rust would let it).
@BrianZill and @anandbonde this is related to https://github.com/demikernel/demikernel/issues/204, right?
@BrianZill and @anandbonde this is related to demikernel/demikernel#204, right?
Somewhat. It's really about cleaning up properly after the removal of the Ipv4Endpoint type rather than having a polymorphic socket address. But they're related since we replaced the Ipv4Endpoint type with SocketAddrV4s for the most part.
Your recent PR [demikernel/demikernel#145] fixed an inefficiency where we were doing an extra pair of byte swaps. I don't know for sure that that particular case came about because of the removal of the Ipv4Endpoint type, but it seems likely.
FYI, the Rust 1.64.0 release notes contain this little gem:
Rust 1.64.0 changes the memory layout of Ipv4Addr, Ipv6Addr, SocketAddrV4 and SocketAddrV6 to be more compact and memory efficient. This internal representation was never exposed, but some crates relied on it anyway by using std::mem::transmute, resulting in invalid memory accesses.
I don't think we have anything that depends upon the internal representation, but if we do, I suppose we shouldn't be doing that.
FYI, the Rust 1.64.0 release notes contain this little gem:
Rust 1.64.0 changes the memory layout of Ipv4Addr, Ipv6Addr, SocketAddrV4 and SocketAddrV6 to be more compact and memory efficient. This internal representation was never exposed, but some crates relied on it anyway by using std::mem::transmute, resulting in invalid memory accesses.
I don't think we have anything that depends upon the internal representation, but if we do, I suppose we shouldn't be doing that.
Thanks for keeping an eye on this, @BrianZill.
We don't rely on the internal layout of these structures and if we do, this should be reported as a bug.