From 00cfdf86c8a32fe77ff8a0c134d22edac2a57277 Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Mon, 8 Jan 2024 23:32:04 +0800 Subject: [PATCH] Fix error codes in TCP `sendto` and `recvfrom` --- .../src/net/socket/ip/stream/connected.rs | 34 +++++++++----- .../aster-nix/src/net/socket/ip/stream/mod.rs | 31 ++++++++++--- regression/apps/network/tcp_err.c | 45 +++++++++++++++++++ 3 files changed, 91 insertions(+), 19 deletions(-) diff --git a/kernel/aster-nix/src/net/socket/ip/stream/connected.rs b/kernel/aster-nix/src/net/socket/ip/stream/connected.rs index 3a70f2c97..4ca60ae1c 100644 --- a/kernel/aster-nix/src/net/socket/ip/stream/connected.rs +++ b/kernel/aster-nix/src/net/socket/ip/stream/connected.rs @@ -2,6 +2,8 @@ use alloc::sync::Weak; +use smoltcp::socket::tcp::{RecvError, SendError}; + use crate::{ events::{IoEvents, Observer}, net::{ @@ -34,25 +36,33 @@ impl ConnectedStream { } pub fn try_recvfrom(&self, buf: &mut [u8], flags: SendRecvFlags) -> Result { - let recv_bytes = self + let result = self .bound_socket - .raw_with(|socket: &mut RawTcpSocket| socket.recv_slice(buf)) - .map_err(|_| Error::with_message(Errno::ENOTCONN, "fail to recv packet"))?; - if recv_bytes == 0 { - return_errno_with_message!(Errno::EAGAIN, "try to recv again"); + .raw_with(|socket: &mut RawTcpSocket| socket.recv_slice(buf)); + match result { + Ok(0) => return_errno_with_message!(Errno::EAGAIN, "the receive buffer is empty"), + Ok(recv_bytes) => Ok(recv_bytes), + Err(RecvError::Finished) => Ok(0), + Err(RecvError::InvalidState) => { + return_errno_with_message!(Errno::ECONNRESET, "the connection is reset") + } } - Ok(recv_bytes) } pub fn try_sendto(&self, buf: &[u8], flags: SendRecvFlags) -> Result { - let sent_bytes = self + let result = self .bound_socket - .raw_with(|socket: &mut RawTcpSocket| socket.send_slice(buf)) - .map_err(|_| Error::with_message(Errno::ENOBUFS, "cannot send packet"))?; - if sent_bytes == 0 { - return_errno_with_message!(Errno::EAGAIN, "try to send again"); + .raw_with(|socket: &mut RawTcpSocket| socket.send_slice(buf)); + match result { + Ok(0) => return_errno_with_message!(Errno::EAGAIN, "the send buffer is full"), + Ok(sent_bytes) => Ok(sent_bytes), + Err(SendError::InvalidState) => { + // FIXME: `EPIPE` is another possibility, which means that the socket is shut down + // for writing. In that case, we should also trigger a `SIGPIPE` if `MSG_NOSIGNAL` + // is not specified. + return_errno_with_message!(Errno::ECONNRESET, "the connection is reset"); + } } - Ok(sent_bytes) } pub fn local_endpoint(&self) -> IpEndpoint { diff --git a/kernel/aster-nix/src/net/socket/ip/stream/mod.rs b/kernel/aster-nix/src/net/socket/ip/stream/mod.rs index d235c84ad..b7be8c645 100644 --- a/kernel/aster-nix/src/net/socket/ip/stream/mod.rs +++ b/kernel/aster-nix/src/net/socket/ip/stream/mod.rs @@ -184,9 +184,16 @@ impl StreamSocket { fn try_recvfrom(&self, buf: &mut [u8], flags: SendRecvFlags) -> Result<(usize, SocketAddr)> { let state = self.state.read(); - let State::Connected(connected_stream) = state.as_ref() else { - return_errno_with_message!(Errno::EINVAL, "the socket is not connected"); + let connected_stream = match state.as_ref() { + State::Connected(connected_stream) => connected_stream, + State::Init(_) | State::Listen(_) => { + return_errno_with_message!(Errno::ENOTCONN, "the socket is not connected") + } + State::Connecting(_) => { + return_errno_with_message!(Errno::EAGAIN, "the socket is connecting") + } }; + let recv_bytes = connected_stream.try_recvfrom(buf, flags)?; connected_stream.update_io_events(&self.pollee); Ok((recv_bytes, connected_stream.remote_endpoint().into())) @@ -195,9 +202,19 @@ impl StreamSocket { fn try_sendto(&self, buf: &[u8], flags: SendRecvFlags) -> Result { let state = self.state.read(); - let State::Connected(connected_stream) = state.as_ref() else { - return_errno_with_message!(Errno::EINVAL, "the socket is not connected"); + let connected_stream = match state.as_ref() { + State::Connected(connected_stream) => connected_stream, + State::Init(_) | State::Listen(_) => { + // TODO: Trigger `SIGPIPE` if `MSG_NOSIGNAL` is not specified + return_errno_with_message!(Errno::EPIPE, "the socket is not connected"); + } + State::Connecting(_) => { + // FIXME: Linux indeed allows data to be buffered at this point. Can we do + // something similar? + return_errno_with_message!(Errno::EAGAIN, "the socket is connecting") + } }; + let sent_bytes = connected_stream.try_sendto(buf, flags)?; connected_stream.update_io_events(&self.pollee); Ok(sent_bytes) @@ -427,9 +444,9 @@ impl Socket for StreamSocket { ) -> Result { debug_assert!(flags.is_all_supported()); - if remote.is_some() { - return_errno_with_message!(Errno::EINVAL, "tcp socked should not provide remote addr"); - } + // According to the Linux man pages, `EISCONN` _may_ be returned when the destination + // address is specified for a connection-mode socket. In practice, the destination address + // is simply ignored. We follow the same behavior as the Linux implementation to ignore it. let sent_bytes = if self.is_nonblocking() { self.try_sendto(buf, flags)? diff --git a/regression/apps/network/tcp_err.c b/regression/apps/network/tcp_err.c index 49c9b1af3..ead55d4fd 100644 --- a/regression/apps/network/tcp_err.c +++ b/regression/apps/network/tcp_err.c @@ -136,3 +136,48 @@ FN_TEST(peername_is_peer_sockname) addrlen == sizeof(saddr) && saddr.sin_port == em_port); } END_TEST() + +FN_TEST(send) +{ + char buf[1] = { 'z' }; + + TEST_ERRNO(send(sk_unbound, buf, 1, 0), EPIPE); + + TEST_ERRNO(send(sk_bound, buf, 1, 0), EPIPE); + + TEST_ERRNO(send(sk_listen, buf, 1, 0), EPIPE); +} +END_TEST() + +FN_TEST(recv) +{ + char buf[1] = { 'z' }; + + TEST_ERRNO(recv(sk_unbound, buf, 1, 0), ENOTCONN); + + TEST_ERRNO(recv(sk_bound, buf, 1, 0), ENOTCONN); + + TEST_ERRNO(recv(sk_listen, buf, 1, 0), ENOTCONN); +} +END_TEST() + +FN_TEST(send_and_recv) +{ + char buf[1]; + + buf[0] = 'a'; + TEST_RES(send(sk_connected, buf, 1, 0), _ret == 1); + + buf[0] = 'b'; + sk_addr.sin_port = 0xbeef; + TEST_RES(sendto(sk_accepted, buf, 1, 0, (struct sockaddr *)&sk_addr, + sizeof(sk_addr)), + _ret == 1); + + TEST_RES(recv(sk_accepted, buf, 1, 0), buf[0] == 'a'); + + TEST_RES(recv(sk_connected, buf, 1, 0), buf[0] == 'b'); + + TEST_ERRNO(recv(sk_connected, buf, 1, 0), EAGAIN); +} +END_TEST()