diff --git a/virtio-devices/src/vsock/csm/connection.rs b/virtio-devices/src/vsock/csm/connection.rs index fe89365e3..067b19859 100644 --- a/virtio-devices/src/vsock/csm/connection.rs +++ b/virtio-devices/src/vsock/csm/connection.rs @@ -215,15 +215,12 @@ where match self.stream.read(&mut buf[..max_len]) { Ok(read_cnt) => { if read_cnt == 0 { - // A 0-length read means the host stream was closed down. In that case, - // we'll ask our peer to shut down the connection. We can neither send nor - // receive any more data. - self.state = ConnState::LocalClosed; - self.expiry = Some( - Instant::now() + Duration::from_millis(defs::CONN_SHUTDOWN_TIMEOUT_MS), - ); + // A 0-length read means the host stream's read side was closed (e.g. + // the host peer did shutdown(SHUT_WR)). We can no longer send data to + // the guest, but the guest may still send data to us (to be written + // to the host stream). + self.state = ConnState::LocalClosed(false); pkt.set_op(uapi::VSOCK_OP_SHUTDOWN) - .set_flag(uapi::VSOCK_FLAGS_SHUTDOWN_RCV) .set_flag(uapi::VSOCK_FLAGS_SHUTDOWN_SEND); } else { // On a successful data read, we fill in the packet with the RW op, and @@ -288,7 +285,9 @@ where // Most frequent case: this is an established connection that needs to forward some // data to the host stream. Also works for a connection that has begun shutting // down, but the peer still has some data to send. - ConnState::Established | ConnState::PeerClosed(_, false) + ConnState::Established + | ConnState::PeerClosed(_, false) + | ConnState::LocalClosed(false) if pkt.op() == uapi::VSOCK_OP_RW => { if pkt.buf().is_none() { @@ -359,6 +358,26 @@ where } } + // The peer wants to shut down while the local stream's read side is already closed. + // We only care about the send indication: if the peer will no longer send data, both + // directions are now closed and we can schedule termination. + ConnState::LocalClosed(ref mut recv_off) + if pkt.op() == uapi::VSOCK_OP_SHUTDOWN => + { + let send_off = pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_SEND != 0; + if send_off && !*recv_off { + *recv_off = true; + if self.tx_buf.is_empty() { + self.pending_rx.insert(PendingRx::Rst); + } else { + self.expiry = Some( + Instant::now() + + Duration::from_millis(defs::CONN_SHUTDOWN_TIMEOUT_MS), + ); + } + } + } + // A credit update from our peer is valid only in a state which allows data // transfer towards the peer. ConnState::Established | ConnState::PeerInit | ConnState::PeerClosed(false, _) @@ -369,7 +388,10 @@ where // A credit request from our peer is valid only in a state which allows data // transfer from the peer. We'll respond with a credit update packet. - ConnState::Established | ConnState::PeerInit | ConnState::PeerClosed(_, false) + ConnState::Established + | ConnState::PeerInit + | ConnState::PeerClosed(_, false) + | ConnState::LocalClosed(false) if pkt.op() == uapi::VSOCK_OP_CREDIT_REQUEST => { self.pending_rx.insert(PendingRx::CreditUpdate); @@ -424,7 +446,7 @@ where // We're generally interested in being notified when data can be read from the host // stream, unless we're in a state which doesn't allow moving data from host to guest. match self.state { - ConnState::Killed | ConnState::LocalClosed | ConnState::PeerClosed(true, _) => (), + ConnState::Killed | ConnState::LocalClosed(_) | ConnState::PeerClosed(true, _) => (), _ if self.need_credit_update_from_peer() => (), _ => evset.insert(epoll::Events::EPOLLIN), } @@ -468,7 +490,10 @@ where // If this connection was shutting down, but is waiting to drain the TX buffer // before forceful termination, the wait might be over. - if self.state == ConnState::PeerClosed(true, true) && self.tx_buf.is_empty() { + if (self.state == ConnState::PeerClosed(true, true) + || self.state == ConnState::LocalClosed(true)) + && self.tx_buf.is_empty() + { self.pending_rx.insert(PendingRx::Rst); } else if self.peer_needs_credit_update() { // If we've freed up some more buffer space, we may need to let the peer know it @@ -973,7 +998,7 @@ mod unit_tests { } // A recv attempt in an invalid state should yield an instant reset packet. - ctx.conn.state = ConnState::LocalClosed; + ctx.conn.state = ConnState::LocalClosed(true); ctx.notify_epollin(); ctx.recv(); assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST); @@ -981,25 +1006,39 @@ mod unit_tests { #[test] fn test_local_close() { + // When the host stream's read side is closed (e.g. peer did shutdown(SHUT_WR)), + // we should only indicate SHUTDOWN_SEND (we won't send anymore), not SHUTDOWN_RCV, + // since the guest can still send data to us. let mut ctx = CsmTestContext::new_established(); let mut stream = TestStream::new(); stream.read_state = StreamState::Closed; ctx.set_stream(stream); ctx.notify_epollin(); ctx.recv(); - // When the host-side stream is closed, we can neither send not receive any more data. - // Therefore, the vsock shutdown packet that we'll deliver to the guest must contain both - // the no-more-send and the no-more-recv indications. assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_SHUTDOWN); assert_ne!(ctx.pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_SEND, 0); - assert_ne!(ctx.pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_RCV, 0); + assert_eq!(ctx.pkt.flags() & uapi::VSOCK_FLAGS_SHUTDOWN_RCV, 0); - // The kill timer should now be armed. - assert!(ctx.conn.will_expire()); - assert!( - ctx.conn.expiry().unwrap() - < Instant::now() + Duration::from_millis(defs::CONN_SHUTDOWN_TIMEOUT_MS) - ); + // The connection should be in half-closed state, not fully closed. + assert_eq!(ctx.conn.state, ConnState::LocalClosed(false)); + + // No kill timer should be set for half-close. + assert!(!ctx.conn.will_expire()); + + // Guest should still be able to send data to us. + let data = &[1, 2, 3, 4]; + ctx.init_data_pkt(data); + ctx.send(); + assert_eq!(ctx.conn.stream.write_buf, data.to_vec()); + + // When the guest also shuts down sending, the connection should be fully closed. + ctx.init_pkt(uapi::VSOCK_OP_SHUTDOWN, 0) + .set_flags(uapi::VSOCK_FLAGS_SHUTDOWN_SEND); + ctx.send(); + assert_eq!(ctx.conn.state, ConnState::LocalClosed(true)); + assert!(ctx.conn.has_pending_rx()); + ctx.recv(); + assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST); } #[test] diff --git a/virtio-devices/src/vsock/csm/mod.rs b/virtio-devices/src/vsock/csm/mod.rs index 69de7073d..6a4d0959b 100644 --- a/virtio-devices/src/vsock/csm/mod.rs +++ b/virtio-devices/src/vsock/csm/mod.rs @@ -52,8 +52,10 @@ pub enum ConnState { PeerInit, /// The connection handshake has been performed successfully, and data can now be exchanged. Established, - /// The host (AF_UNIX) socket was closed. - LocalClosed, + /// The host (AF_UNIX) socket's read side was closed (e.g. the host peer did + /// shutdown(SHUT_WR)). The bool indicates whether receiving from the guest is also shut down + /// (i.e. the connection is fully closed from the local perspective). + LocalClosed(bool), /// A VSOCK_OP_SHUTDOWN packet was received from the guest. The tuple represents the guest R/W /// indication: (will_not_recv_anymore_data, will_not_send_anymore_data). PeerClosed(bool, bool),