From 7d576b4f15538039a623f0eb9efd08d96c3558e5 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Wed, 19 Jun 2024 07:46:28 +0200 Subject: [PATCH] nic_router: re-use ARP waiters for same IP address For each packet that got stuck with an ARP-cache miss, the router used to send one ARP request and create one ARP waiter. However, in situations where many packets target the same IP at one destination domain and during a short period of time, this causes unnecessary session-quota consumption and network traffic. This issue becomes especially pressing when taking malicious source peers, absent destination peers, and packet batching into account. Therefore, with this commit, the router can accumulate multiple source packets with the same destination IP at one ARP waiter. This means, that only the first packet with an ARP-cache for a certain IP sends an ARP request and creates an ARP waiter. For situations where the ARP request is not answered, this essentially rate-limits ARP requests for one IP at one destination domain according to the lifetime of ARP waiters (default: 10s) Ref #4534 --- repos/os/src/server/nic_router/README | 6 +- repos/os/src/server/nic_router/arp_waiter.cc | 15 +++-- repos/os/src/server/nic_router/arp_waiter.h | 67 ++++++++++++++++---- repos/os/src/server/nic_router/interface.cc | 47 ++++++++++---- 4 files changed, 100 insertions(+), 35 deletions(-) diff --git a/repos/os/src/server/nic_router/README b/repos/os/src/server/nic_router/README index e004e9ae46..ac702b7a55 100644 --- a/repos/os/src/server/nic_router/README +++ b/repos/os/src/server/nic_router/README @@ -868,8 +868,10 @@ following attribute (default value shown): ! When the router does not receive a reply for a sent ARP request within this -timeout, the packet that caused the ARP request and the ARP request state are -dropped in order to re-use the resources. +timeout, the packets that were waiting for the reply and the ARP request state +are dropped in order to re-use the resources. Note that this is also the +rate-limit for the router sending ARP requests for one IP address at one +domain. Behavior regarding the NIC-session link state ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/repos/os/src/server/nic_router/arp_waiter.cc b/repos/os/src/server/nic_router/arp_waiter.cc index f1a932fd9b..95b34b04b4 100644 --- a/repos/os/src/server/nic_router/arp_waiter.cc +++ b/repos/os/src/server/nic_router/arp_waiter.cc @@ -20,20 +20,21 @@ using namespace Net; using namespace Genode; -Arp_waiter::Arp_waiter(Interface &src, - Domain &dst, - Ipv4_address const &ip, - Packet_descriptor const &packet, - Microseconds timeout, - Cached_timer &timer) +Arp_waiter::Arp_waiter(Interface &src, + Domain &dst, + Ipv4_address const &ip, + Packet_list_element &packet_le, + Microseconds timeout, + Cached_timer &timer) : _src_le(this), _src(src), _dst_le(this), _dst_ptr(&dst), _ip(ip), - _packet(packet), _timeout(timer, *this, &Arp_waiter::_handle_timeout, timeout) + _timeout(timer, *this, &Arp_waiter::_handle_timeout, timeout) { _src.arp_stats().alive++; _src.own_arp_waiters().insert(&_src_le); _dst_ptr->foreign_arp_waiters().insert(&_dst_le); _timeout.schedule(timeout); + add_packet(packet_le); } diff --git a/repos/os/src/server/nic_router/arp_waiter.h b/repos/os/src/server/nic_router/arp_waiter.h index 8ec8b68160..72146b5701 100644 --- a/repos/os/src/server/nic_router/arp_waiter.h +++ b/repos/os/src/server/nic_router/arp_waiter.h @@ -17,6 +17,7 @@ /* local includes */ #include #include +#include /* Genode includes */ #include @@ -31,10 +32,20 @@ namespace Net { class Configuration; class Arp_waiter; using Arp_waiter_list_element = Genode::List_element; - using Arp_waiter_list = List; + struct Packet_list_element; + using Packet_list = Genode::List; + struct Arp_waiter_list; } +struct Net::Packet_list_element : Genode::List::Element +{ + Packet_descriptor const packet; + + Packet_list_element(Packet_descriptor const &packet) : packet(packet) { } +}; + + class Net::Arp_waiter { private: @@ -44,7 +55,7 @@ class Net::Arp_waiter Arp_waiter_list_element _dst_le; Domain *_dst_ptr; Ipv4_address const _ip; - Packet_descriptor const _packet; + Packet_list _packets { }; Lazy_one_shot_timeout _timeout; /* @@ -59,17 +70,28 @@ class Net::Arp_waiter public: - Arp_waiter(Interface &src, - Domain &dst, - Ipv4_address const &ip, - Packet_descriptor const &packet, - Genode::Microseconds dissolve_timeout, - Cached_timer &timer); + Arp_waiter(Interface &src, + Domain &dst, + Ipv4_address const &ip, + Packet_list_element &packet_le, + Genode::Microseconds dissolve_timeout, + Cached_timer &timer); ~Arp_waiter(); void handle_config(Domain &dst); + void add_packet(Packet_list_element &le) { _packets.insert(&le); } + + void flush_packets(Genode::Deallocator &dealloc, auto const &fn) + { + while (Packet_list_element *le = _packets.first()) { + _packets.remove(le); + fn(le->packet); + destroy(dealloc, le); + } + } + /********* ** Log ** @@ -82,10 +104,31 @@ class Net::Arp_waiter ** Accessors ** ***************/ - Interface &src() const { return _src; } - Ipv4_address const &ip() const { return _ip; } - Packet_descriptor const &packet() const { return _packet; } - Domain &dst() { return *_dst_ptr; } + Interface &src() const { return _src; } + Ipv4_address const &ip() const { return _ip; } + Domain &dst() { return *_dst_ptr; } +}; + + +struct Net::Arp_waiter_list : List +{ + void find_by_ip(Ipv4_address const &ip, auto const &found_fn, auto const ¬_found_fn) + { + bool found = false; + for_each([&] (Arp_waiter_list_element &elem) { + if (found) + return; + + Arp_waiter &waiter = *elem.object(); + if (ip != waiter.ip()) + return; + + found_fn(waiter); + found = true; + }); + if (!found) + not_found_fn(); + } }; #endif /* _ARP_WAITER_H_ */ diff --git a/repos/os/src/server/nic_router/interface.cc b/repos/os/src/server/nic_router/interface.cc index ea9e1f524e..ce58bc84f3 100644 --- a/repos/os/src/server/nic_router/interface.cc +++ b/repos/os/src/server/nic_router/interface.cc @@ -596,7 +596,30 @@ Packet_result Interface::_adapt_eth(Ethernet_frame ð, if (!remote_domain.use_arp()) return result; - auto with_next_hop = [&] (Ipv4_address const &hop_ip) { + auto with_next_hop = [&] (Ipv4_address const &hop_ip) + { + auto send_arp_fn = [&] + { + Packet_list_element &packet_le = *new (_alloc) Packet_list_element(pkt); + auto create_new_arp_waiter = [&] + { + new (_alloc) Arp_waiter( + *this, remote_domain, hop_ip, packet_le, _config_ptr->arp_request_timeout(), _timer); + + remote_domain.interfaces().for_each([&] (Interface &interface) { + interface._broadcast_arp_request(remote_ip_cfg.interface().address, hop_ip); }); + + result = packet_postponed(); + }; + remote_domain.foreign_arp_waiters().find_by_ip(hop_ip, + [&] (Arp_waiter &waiter) { waiter.add_packet(packet_le); }, + [&] { + retry_once( + create_new_arp_waiter, + [&] { _try_emergency_free_quota(); }, + [&] { result = packet_drop("out of quota while creating ARP waiter"); }); + }); + }; remote_domain.arp_cache().find_by_ip( hop_ip, [&] /* handle_match */ (Arp_cache_entry const &entry) @@ -606,17 +629,9 @@ Packet_result Interface::_adapt_eth(Ethernet_frame ð, [&] /* handle_no_match */ () { retry_once( - [&] { - new (_alloc) Arp_waiter { *this, remote_domain, hop_ip, pkt, _config_ptr->arp_request_timeout(), _timer }; - remote_domain.interfaces().for_each([&] (Interface &interface) - { - interface._broadcast_arp_request( - remote_ip_cfg.interface().address, hop_ip); - }); - result = packet_postponed(); - }, + send_arp_fn, [&] { _try_emergency_free_quota(); }, - [&] { result = packet_drop("out of quota while creating ARP waiter"); }); + [&] { result = packet_drop("out of quota while creating ARP packet list element"); }); } ); }; @@ -1491,7 +1506,8 @@ void Interface::_handle_arp_reply(Ethernet_frame ð, Arp_waiter &waiter = *waiter_le->object(); waiter_le = waiter_le->next(); if (ip != waiter.ip()) { continue; } - waiter.src()._continue_handle_eth(waiter.packet()); + waiter.flush_packets(waiter.src()._alloc, [&] (Packet_descriptor const &packet) { + waiter.src()._continue_handle_eth(packet); }); destroy(waiter.src()._alloc, &waiter); } } @@ -1737,7 +1753,8 @@ void Interface::_destroy_timed_out_arp_waiters() { while (Arp_waiter_list_element *le = _timed_out_arp_waiters.first()) { Arp_waiter &waiter = *le->object(); - _drop_packet(waiter.packet(), "ARP request timed out"); + waiter.flush_packets(_alloc, [&] (Packet_descriptor const &packet) { + _drop_packet(packet, "ARP request timed out"); }); _timed_out_arp_waiters.remove(le); destroy(_alloc, &waiter); } @@ -2264,7 +2281,9 @@ void Interface::_ack_packet(Packet_descriptor const &pkt) void Interface::cancel_arp_waiting(Arp_waiter &waiter) { - _drop_packet(waiter.packet(), "ARP got cancelled"); + + waiter.flush_packets(_alloc, [&] (Packet_descriptor const &packet) { + _drop_packet(packet, "ARP got cancelled"); }); destroy(_alloc, &waiter); }