From 0a33168733601c7bbd0e08194a2ac8a2f553afe9 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Thu, 30 May 2024 14:29:56 +0200 Subject: [PATCH] nic_router: mark tcp open only with full handshake The TCP connection state "ESTABLISHED" (in the router "OPEN") is a privileged one for peers because it lasts very long without any peer interaction (in the NIC router it's only 10 minutes, but RFCs recommend not less than 2 hours and 4 minutes). Furthermore, TCP connections in this state are normally not available for early-drop on resource exhaustion. This means that this state binds resources to a connection potentially for a long time without the option of regaining them under stress. Therefore, this state should be entered with care. Up to now, the router marked a TCP connection with this state as soon as it had seen one matching packet in both directions, which is rather quick. However, implementing a very precise tracking of the exact TCP states of both peers and only marking the connection "ESTABLISHED" when both peers are "ESTABLISHED" is a difficult task with lots of corner cases. That said, this commit implements a compromise. The router now has two flags for each peer of a TCP connection - FIN sent and FIN acked - and sets them according to the observed TCP flags. The "ESTABLISHED" state is entered only when FIN acked is set for both peers (without having observed an RST or FIN flag meanwhile). Ref #4729 --- repos/os/src/server/nic_router/interface.cc | 7 ++--- repos/os/src/server/nic_router/interface.h | 1 + repos/os/src/server/nic_router/link.cc | 19 ++++++++++---- repos/os/src/server/nic_router/link.h | 29 +++++++++++++-------- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/repos/os/src/server/nic_router/interface.cc b/repos/os/src/server/nic_router/interface.cc index 7e2138458b..fe53083d00 100644 --- a/repos/os/src/server/nic_router/interface.cc +++ b/repos/os/src/server/nic_router/interface.cc @@ -509,6 +509,7 @@ void Interface::_try_emergency_free_quota() Packet_result Interface::_new_link(L3_protocol const protocol, + void *const prot_base, Domain &local_domain, Link_side_id const &local, Port_allocator_guard *remote_port_alloc_ptr, @@ -521,8 +522,8 @@ Packet_result Interface::_new_link(L3_protocol const protocol, retry_once( [&] { new (_alloc) - Tcp_link { *this, local_domain, local, remote_port_alloc_ptr, remote_domain, - remote, _timer, *_config_ptr, protocol, _tcp_stats }; }, + Tcp_link { *this, local_domain, local, remote_port_alloc_ptr, remote_domain, + remote, _timer, *_config_ptr, protocol, _tcp_stats, *(Tcp_packet *)prot_base }; }, [&] { _try_emergency_free_quota(); }, [&] { _tcp_stats.refused_for_ram++; @@ -666,7 +667,7 @@ Packet_result Interface::_nat_link_and_pass(Ethernet_frame ð, Link_side_id const remote_id = { ip.dst(), _dst_port(prot, prot_base), ip.src(), _src_port(prot, prot_base) }; - result = _new_link(prot, local_domain, local_id, remote_port_alloc_ptr, remote_domain, remote_id); + result = _new_link(prot, prot_base, local_domain, local_id, remote_port_alloc_ptr, remote_domain, remote_id); if (result.valid()) return result; diff --git a/repos/os/src/server/nic_router/interface.h b/repos/os/src/server/nic_router/interface.h index d5524c827d..f580922c83 100644 --- a/repos/os/src/server/nic_router/interface.h +++ b/repos/os/src/server/nic_router/interface.h @@ -179,6 +179,7 @@ class Net::Interface : private Interface_list::Element Interface &operator = (Interface const &); [[nodiscard]] Packet_result _new_link(L3_protocol const protocol, + void *const prot_base, Domain &local_domain, Link_side_id const &local_id, Port_allocator_guard *remote_port_alloc_ptr, diff --git a/repos/os/src/server/nic_router/link.cc b/repos/os/src/server/nic_router/link.cc index 4d6c8ff5f9..5e3080d9e9 100644 --- a/repos/os/src/server/nic_router/link.cc +++ b/repos/os/src/server/nic_router/link.cc @@ -205,11 +205,14 @@ Tcp_link::Tcp_link(Interface &cln_interface, Cached_timer &timer, Configuration &config, L3_protocol const protocol, - Interface_link_stats &stats) + Interface_link_stats &stats, + Tcp_packet &tcp) : Link(cln_interface, cln_domain, cln_id, srv_port_alloc_ptr, srv_domain, srv_id, timer, config, protocol, config.tcp_idle_timeout(), stats) -{ } +{ + client_packet(tcp); +} void Tcp_link::_closing() @@ -234,6 +237,9 @@ void Tcp_link::_tcp_packet(Tcp_packet &tcp, Peer &sender, Peer &receiver) { + if (_state == State::OPENING) + _opening_tcp_packet(tcp, sender, receiver); + if (_state == State::CLOSED) { return; } @@ -262,16 +268,19 @@ void Tcp_link::_tcp_packet(Tcp_packet &tcp, } -void Tcp_link::server_packet(Tcp_packet &tcp) +void Tcp_link::_opening_tcp_packet(Tcp_packet const &tcp, Peer &sender, Peer &receiver) { - if (_opening) { + if (tcp.syn()) + sender.syn = true; + if (tcp.ack() && receiver.syn && !receiver.syn_acked) + receiver.syn_acked = true; + if (sender.syn_acked && receiver.syn_acked) { _opening = false; _state = State::OPEN; (*_stats_ptr)--; if (_stats_ptr == &_stats.opening) { _stats_ptr = &_stats.open; } (*_stats_ptr)++; } - _tcp_packet(tcp, _server, _client); } diff --git a/repos/os/src/server/nic_router/link.h b/repos/os/src/server/nic_router/link.h index a2a52bcfd3..493b9a039e 100644 --- a/repos/os/src/server/nic_router/link.h +++ b/repos/os/src/server/nic_router/link.h @@ -265,6 +265,8 @@ class Net::Tcp_link : public Link struct Peer { + bool syn { false }; + bool syn_acked { false }; bool fin { false }; bool fin_acked { false }; }; @@ -277,26 +279,31 @@ class Net::Tcp_link : public Link Peer &sender, Peer &receiver); + void _opening_tcp_packet(Tcp_packet const &tcp, + Peer &sender, + Peer &receiver); + void _closing(); void _closed(); public: - Tcp_link(Interface &cln_interface, - Domain &cln_domain, - Link_side_id const &cln_id, - Port_allocator_guard *srv_port_alloc_ptr, - Domain &srv_domain, - Link_side_id const &srv_id, - Cached_timer &timer, - Configuration &config, - L3_protocol const protocol, - Interface_link_stats &stats); + Tcp_link(Interface &cln_interface, + Domain &cln_domain, + Link_side_id const &cln_id, + Port_allocator_guard *srv_port_alloc_ptr, + Domain &srv_domain, + Link_side_id const &srv_id, + Cached_timer &timer, + Configuration &config, + L3_protocol const protocol, + Interface_link_stats &stats, + Tcp_packet &tcp); void client_packet(Tcp_packet &tcp) { _tcp_packet(tcp, _client, _server); } - void server_packet(Tcp_packet &tcp); + void server_packet(Tcp_packet &tcp) { _tcp_packet(tcp, _server, _client); } bool can_early_drop() { return _state != State::OPEN; } };