Kernel/Net: Make IPSocket generic #25074
Conversation
github-actions
bot
added
the
👀 pr-needs-review
kleinesfilmroellchen
force-pushed
the
ipv6/generic-socket
branch
from
3212b34
to
5f3fe7f
Compare
This abstraction is useful in code that simply moves IP addresses around without looking at them, like the socket code.
The renamed IPSocket handles both IP types mostly transparently. It contains an IP-version specific delegate that handles the respective details where this is necessary. With future changes to TCPSocket and UDPSocket, this allows us to support IPv6 with minimal changes to the transport layer sockets.
kleinesfilmroellchen
force-pushed
the
ipv6/generic-socket
branch
from
5f3fe7f
to
2290b20
Compare
| // For the generic port retrieval code to work (see below in bind()), port fields need to be identical between IP versions. | ||
| static_assert(__builtin_offsetof(sockaddr_in, sin_port) == __builtin_offsetof(sockaddr_in6, sin6_port) | ||
| && sizeof(decltype(declval<sockaddr_in>().sin_port)) == sizeof(decltype(declval<sockaddr_in6>().sin6_port))); | ||
|
|
||
| static ErrorOr<in_port_t> copy_port_from_user(Userspace<sockaddr const*> user_address) | ||
| { | ||
| in_port_t requested_local_port_network_order; | ||
| auto* address = reinterpret_cast<sockaddr_in const*>(user_address.unsafe_userspace_ptr()); | ||
| // Since the port is in the same place for IPv4 and IPv6, this will always be correct. | ||
| TRY(copy_from_user(&requested_local_port_network_order, &address->sin_port, sizeof(in_port_t))); | ||
| auto requested_local_port = AK::convert_between_host_and_network_endian(requested_local_port_network_order); | ||
| return requested_local_port; | ||
| } |
There was a problem hiding this comment.
Passerby comment: this feels overly complicated for what it achieves. Maybe add an additional method in the delegate that extracts port or just make set_local_address return port since you're reading sockaddr there already?
(Feel free to ignore)
|
|
||
| namespace AK { | ||
|
|
||
| enum class IPVersion : bool { |
There was a problem hiding this comment.
Not really a blocker, but not sure if bool is worth it here. It's going to be 1 byte, might as well make it an u8 so that we don't have to update it for IPv10 (and boolean values don't really make sense for enumerations anyways).
(Given that this PR doesn't currently pass CI and therefore needs to be updated anyways, might as well include that in the set of changes.)
| in_port_t requested_local_port_network_order; | ||
| auto* address = reinterpret_cast<sockaddr_in const*>(user_address.unsafe_userspace_ptr()); | ||
| // Since the port is in the same place for IPv4 and IPv6, this will always be correct. | ||
| TRY(copy_from_user(&requested_local_port_network_order, &address->sin_port, sizeof(in_port_t))); |
There was a problem hiding this comment.
I'm sure it won't actually compile to anything critical, but is &address->sin_port allowed by specification if we are not supposed to access address?
| if (address.sin_family != AF_INET) | ||
| return EINVAL; | ||
|
|
||
| m_local_address = IPv4Address((u8 const*)&address.sin_addr.s_addr); |
| if (m_local_address.to_u32() == 0) | ||
| m_local_address = routing_decision.adapter->ipv4_address(); | ||
| RoutingDecision routing_decision; | ||
| // TODO: Make this IPv6-capable once the routing supports it. |
There was a problem hiding this comment.
Should this have an else VERIFY_NOT_REACHED(); then?
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
A split of the IPv4Socket struct into an IPv4-specific delegate and the generic IPSocket. This split is not yet complete, as it depends on IPv6 support in the routing and some reshuffling of how packets are created. However, the general design should go through review already and be merged so that other work on IPSocket, NetworkInterface, TCPSocket/UDPSocket and others can proceed.
Part of SIP6TF @famfo @sdomi
AK: Add version-independent IPAddress abstraction
This abstraction is useful in code that simply moves IP addresses around
without looking at them, like the socket code.
Kernel/Net: Make IPv4Socket generic
The renamed IPSocket handles both IP types mostly
transparently. It contains an IP-version specific
delegate that handles the respective details where
this is necessary. With future changes to
TCPSocket and UDPSocket, this allows us to support
IPv6 with minimal changes to the transport layer
sockets.