From ef8c98cb717de36dca6ef7f7494920eca4114dc9 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Tue, 5 Apr 2022 13:41:25 +0200 Subject: [PATCH] nic_router: merge packet stream signal handlers The NIC router used to handle each type of packet-stream signal with a distinct method in the Interface class. However, merging those methods has advantages. It ensures that sent packets that were already acknowledged by the counter side are always released before handling received packets. This frees packet stream memory which facilitates the potential allocation of response packets while handling received packets. Furthermore, it simplifies the code and reduces the number of entry points into the router. This commit also removes the installation of signal handlers at packet streams for events that are of no interest for the router (TX-ready-to-ack / RX-ready-to-submit at NIC sessions and RX-ready-to-ack / TX-ready-to-submit at Uplink sessions). Fixes #4470 --- repos/os/src/server/nic_router/interface.cc | 57 +++++++++++-------- repos/os/src/server/nic_router/interface.h | 40 +++++-------- repos/os/src/server/nic_router/nic_client.cc | 6 +- .../src/server/nic_router/nic_session_root.cc | 7 +-- .../server/nic_router/uplink_session_root.cc | 7 +-- 5 files changed, 55 insertions(+), 62 deletions(-) diff --git a/repos/os/src/server/nic_router/interface.cc b/repos/os/src/server/nic_router/interface.cc index 6e277c3f2b..d609e27c1c 100644 --- a/repos/os/src/server/nic_router/interface.cc +++ b/repos/os/src/server/nic_router/interface.cc @@ -1480,21 +1480,42 @@ void Interface::_handle_pkt() } -void Interface::_ready_to_submit() +void Interface::_handle_pkt_stream_signal() { + /* + * Release all sent packets that were already acknowledged by the counter + * side. Doing this first frees packet-stream memory which facilitates + * sending new packets in the subsequent steps of this handler. + */ + while (_source.ack_avail()) { + _source.release_packet(_source.get_acked_packet()); + } + + /* + * Handle packets received from the counter side. If the user configured + * a limit for the number of packets to be handled at once, this limit gets + * applied. If there is no such limit, received packets are handled until + * none is left. + */ unsigned long const max_pkts = _config().max_packets_per_signal(); if (max_pkts) { for (unsigned long i = 0; _sink.packet_avail(); i++) { if (i >= max_pkts) { - Signal_transmitter(_sink_submit).submit(); + + /* + * Ensure that this handler is called again in order to handle + * the packets left unhandled due to the configured limit. + */ + Signal_transmitter(_pkt_stream_signal_handler).submit(); break; } _handle_pkt(); } } else { while (_sink.packet_avail()) { - _handle_pkt(); } + _handle_pkt(); + } } } @@ -1517,13 +1538,6 @@ void Interface::_continue_handle_eth(Domain const &domain, } -void Interface::_ready_to_ack() -{ - while (_source.ack_avail()) { - _source.release_packet(_source.get_acked_packet()); } -} - - void Interface::_destroy_dhcp_allocation(Dhcp_allocation &allocation, Domain &local_domain) { @@ -1761,19 +1775,16 @@ Interface::Interface(Genode::Entrypoint &ep, Packet_stream_source &source, Interface_policy &policy) : - _sink { sink }, - _source { source }, - _sink_ack { ep, *this, &Interface::_ack_avail }, - _sink_submit { ep, *this, &Interface::_ready_to_submit }, - _source_ack { ep, *this, &Interface::_ready_to_ack }, - _source_submit { ep, *this, &Interface::_packet_avail }, - _router_mac { router_mac }, - _mac { mac }, - _config { config }, - _policy { policy }, - _timer { timer }, - _alloc { alloc }, - _interfaces { interfaces } + _sink { sink }, + _source { source }, + _pkt_stream_signal_handler { ep, *this, &Interface::_handle_pkt_stream_signal }, + _router_mac { router_mac }, + _mac { mac }, + _config { config }, + _policy { policy }, + _timer { timer }, + _alloc { alloc }, + _interfaces { interfaces } { _interfaces.insert(this); } diff --git a/repos/os/src/server/nic_router/interface.h b/repos/os/src/server/nic_router/interface.h index 0968c05394..6ac3caf146 100644 --- a/repos/os/src/server/nic_router/interface.h +++ b/repos/os/src/server/nic_router/interface.h @@ -135,10 +135,7 @@ class Net::Interface : private Interface_list::Element Packet_stream_sink &_sink; Packet_stream_source &_source; - Signal_handler _sink_ack; - Signal_handler _sink_submit; - Signal_handler _source_ack; - Signal_handler _source_submit; + Signal_handler _pkt_stream_signal_handler; Mac_address const _router_mac; Mac_address const _mac; Reference _config; @@ -357,15 +354,7 @@ class Net::Interface : private Interface_list::Element bool link_state() const; - - /*********************************** - ** Packet-stream signal handlers ** - ***********************************/ - - void _ready_to_submit(); - void _ack_avail() { } - void _ready_to_ack(); - void _packet_avail() { } + void _handle_pkt_stream_signal(); public: @@ -456,20 +445,17 @@ class Net::Interface : private Interface_list::Element ** Accessors ** ***************/ - Configuration const &config() const { return _config(); } - Domain &domain() { return _domain(); } - Mac_address const &router_mac() const { return _router_mac; } - Mac_address const &mac() const { return _mac; } - Arp_waiter_list &own_arp_waiters() { return _own_arp_waiters; } - Signal_handler &sink_ack() { return _sink_ack; } - Signal_handler &sink_submit() { return _sink_submit; } - Signal_handler &source_ack() { return _source_ack; } - Signal_handler &source_submit() { return _source_submit; } - Interface_link_stats &udp_stats() { return _udp_stats; } - Interface_link_stats &tcp_stats() { return _tcp_stats; } - Interface_link_stats &icmp_stats() { return _icmp_stats; } - Interface_object_stats &arp_stats() { return _arp_stats; } - Interface_object_stats &dhcp_stats() { return _dhcp_stats; } + Configuration const &config() const { return _config(); } + Domain &domain() { return _domain(); } + Mac_address const &router_mac() const { return _router_mac; } + Mac_address const &mac() const { return _mac; } + Arp_waiter_list &own_arp_waiters() { return _own_arp_waiters; } + Signal_context_capability pkt_stream_signal_handler() const { return _pkt_stream_signal_handler; } + Interface_link_stats &udp_stats() { return _udp_stats; } + Interface_link_stats &tcp_stats() { return _tcp_stats; } + Interface_link_stats &icmp_stats() { return _icmp_stats; } + Interface_object_stats &arp_stats() { return _arp_stats; } + Interface_object_stats &dhcp_stats() { return _dhcp_stats; } }; #endif /* _INTERFACE_H_ */ diff --git a/repos/os/src/server/nic_router/nic_client.cc b/repos/os/src/server/nic_router/nic_client.cc index a7d55e4290..f3fad8b0fe 100644 --- a/repos/os/src/server/nic_router/nic_client.cc +++ b/repos/os/src/server/nic_router/nic_client.cc @@ -162,10 +162,8 @@ Net::Nic_client_interface::Nic_client_interface(Env &env, *this } { /* install packet stream signal handlers */ - rx_channel()->sigh_ready_to_ack (_interface.sink_ack()); - rx_channel()->sigh_packet_avail (_interface.sink_submit()); - tx_channel()->sigh_ack_avail (_interface.source_ack()); - tx_channel()->sigh_ready_to_submit(_interface.source_submit()); + rx_channel()->sigh_packet_avail(_interface.pkt_stream_signal_handler()); + tx_channel()->sigh_ack_avail (_interface.pkt_stream_signal_handler()); /* initialize link state handling */ Nic::Connection::link_state_sigh(_session_link_state_handler); diff --git a/repos/os/src/server/nic_router/nic_session_root.cc b/repos/os/src/server/nic_router/nic_session_root.cc index e567dc42d7..3efa739f42 100644 --- a/repos/os/src/server/nic_router/nic_session_root.cc +++ b/repos/os/src/server/nic_router/nic_session_root.cc @@ -253,10 +253,9 @@ Nic_session_component(Session_env &session_env, { _interface.attach_to_domain(); - _tx.sigh_ready_to_ack (_interface.sink_ack()); - _tx.sigh_packet_avail (_interface.sink_submit()); - _rx.sigh_ack_avail (_interface.source_ack()); - _rx.sigh_ready_to_submit(_interface.source_submit()); + /* install packet stream signal handlers */ + _tx.sigh_packet_avail(_interface.pkt_stream_signal_handler()); + _rx.sigh_ack_avail (_interface.pkt_stream_signal_handler()); } diff --git a/repos/os/src/server/nic_router/uplink_session_root.cc b/repos/os/src/server/nic_router/uplink_session_root.cc index 5b64a9c13b..fb3beaf4de 100644 --- a/repos/os/src/server/nic_router/uplink_session_root.cc +++ b/repos/os/src/server/nic_router/uplink_session_root.cc @@ -101,10 +101,9 @@ Net::Uplink_session_component::Uplink_session_component(Session_env { _interface.attach_to_domain(); - _tx.sigh_ready_to_ack (_interface.sink_ack()); - _tx.sigh_packet_avail (_interface.sink_submit()); - _rx.sigh_ack_avail (_interface.source_ack()); - _rx.sigh_ready_to_submit(_interface.source_submit()); + /* install packet stream signal handlers */ + _tx.sigh_packet_avail(_interface.pkt_stream_signal_handler()); + _rx.sigh_ack_avail (_interface.pkt_stream_signal_handler()); }