From baf4a85d23ac94249904f8db323fbd2dced42e09 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Mon, 27 Jun 2022 12:19:54 +0200 Subject: [PATCH] nic_router: find link sides w/o exceptions Replaces the former implementation of find_by_id at the data structure for links. This method used to return a reference to the found object and threw an exception if no matching object was found. The new implementation doesn't return anything and doesn't throw exceptions. It takes two lambda arguments instead. One for handling the case that a match was found with a reference to the matching object as argument and another for handling the case that no object matches. This way, expensive exception handling can be avoided and object references stay in a local scope. Ref #4536 --- repos/os/src/server/nic_router/interface.cc | 184 +++++++++++--------- repos/os/src/server/nic_router/link.cc | 32 +--- repos/os/src/server/nic_router/link.h | 47 ++++- 3 files changed, 149 insertions(+), 114 deletions(-) diff --git a/repos/os/src/server/nic_router/interface.cc b/repos/os/src/server/nic_router/interface.cc index d609e27c1c..a204153747 100644 --- a/repos/os/src/server/nic_router/interface.cc +++ b/repos/os/src/server/nic_router/interface.cc @@ -978,29 +978,37 @@ void Interface::_handle_icmp_query(Ethernet_frame ð, ip.dst(), _dst_port(prot, prot_base) }; /* try to route via existing ICMP links */ - try { - Link_side const &local_side = local_domain.links(prot).find_by_id(local_id); - Link &link = local_side.link(); - bool const client = local_side.is_client(); - Link_side &remote_side = client ? link.server() : link.client(); - Domain &remote_domain = remote_side.domain(); - if (_config().verbose()) { - log("[", local_domain, "] using ", l3_protocol_name(prot), - " link: ", link); - } - _adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain); - ip.src(remote_side.dst_ip()); - ip.dst(remote_side.src_ip()); - _src_port(prot, prot_base, remote_side.dst_port()); - _dst_port(prot, prot_base, remote_side.src_port()); + bool done { false }; + local_domain.links(prot).find_by_id( + local_id, + [&] /* handle_match */ (Link_side const &local_side) + { + Link &link = local_side.link(); + bool const client = local_side.is_client(); + Link_side &remote_side = client ? link.server() : link.client(); + Domain &remote_domain = remote_side.domain(); + if (_config().verbose()) { + log("[", local_domain, "] using ", l3_protocol_name(prot), + " link: ", link); + } + _adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain); + ip.src(remote_side.dst_ip()); + ip.dst(remote_side.src_ip()); + _src_port(prot, prot_base, remote_side.dst_port()); + _dst_port(prot, prot_base, remote_side.src_port()); - remote_domain.interfaces().for_each([&] (Interface &interface) { - interface._pass_prot(eth, size_guard, ip, prot, prot_base, prot_size); - }); - _link_packet(prot, prot_base, link, client); + remote_domain.interfaces().for_each([&] (Interface &interface) { + interface._pass_prot( + eth, size_guard, ip, prot, prot_base, prot_size); + }); + _link_packet(prot, prot_base, link, client); + done = true; + }, + [&] /* handle_no_match */ () { } + ); + if (done) { return; } - catch (Link_side_tree::No_match) { } /* try to route via ICMP rules */ try { @@ -1041,48 +1049,54 @@ void Interface::_handle_icmp_error(Ethernet_frame ð, void *const embed_prot_base = _prot_base(embed_prot, size_guard, embed_ip); Link_side_id const local_id = { embed_ip.dst(), _dst_port(embed_prot, embed_prot_base), embed_ip.src(), _src_port(embed_prot, embed_prot_base) }; - try { - /* lookup a link state that matches the embedded transport packet */ - Link_side const &local_side = local_domain.links(embed_prot).find_by_id(local_id); - Link &link = local_side.link(); - bool const client = local_side.is_client(); - Link_side &remote_side = client ? link.server() : link.client(); - Domain &remote_domain = remote_side.domain(); - /* print out that the link is used */ - if (_config().verbose()) { - log("[", local_domain, "] using ", l3_protocol_name(embed_prot), - " link: ", link); + /* lookup a link state that matches the embedded transport packet */ + local_domain.links(embed_prot).find_by_id( + local_id, + [&] /* handle_match */ (Link_side const &local_side) + { + Link &link = local_side.link(); + bool const client = local_side.is_client(); + Link_side &remote_side = client ? link.server() : link.client(); + Domain &remote_domain = remote_side.domain(); + + /* print out that the link is used */ + if (_config().verbose()) { + log("[", local_domain, "] using ", + l3_protocol_name(embed_prot), " link: ", link); + } + /* adapt source and destination of Ethernet frame and IP packet */ + _adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain); + if (remote_side.dst_ip() == remote_domain.ip_config().interface().address) { + ip.src(remote_side.dst_ip()); + } + ip.dst(remote_side.src_ip()); + + /* adapt source and destination of embedded IP and transport packet */ + embed_ip.src(remote_side.src_ip()); + embed_ip.dst(remote_side.dst_ip()); + _src_port(embed_prot, embed_prot_base, remote_side.src_port()); + _dst_port(embed_prot, embed_prot_base, remote_side.dst_port()); + + /* update checksum of both IP headers and the ICMP header */ + embed_ip.update_checksum(); + icmp.update_checksum(icmp_sz - sizeof(Icmp_packet)); + ip.update_checksum(); + + /* send adapted packet to all interfaces of remote domain */ + remote_domain.interfaces().for_each([&] (Interface &interface) { + interface.send(eth, size_guard); + }); + /* refresh link only if the error is not about an ICMP query */ + if (embed_prot != L3_protocol::ICMP) { + _link_packet(embed_prot, embed_prot_base, link, client); } + }, + [&] /* handle_no_match */ () + { + throw Drop_packet("no link that matches packet embedded in " + "ICMP error"); } - /* adapt source and destination of Ethernet frame and IP packet */ - _adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain); - if (remote_side.dst_ip() == remote_domain.ip_config().interface().address) { - ip.src(remote_side.dst_ip()); - } - ip.dst(remote_side.src_ip()); - - /* adapt source and destination of embedded IP and transport packet */ - embed_ip.src(remote_side.src_ip()); - embed_ip.dst(remote_side.dst_ip()); - _src_port(embed_prot, embed_prot_base, remote_side.src_port()); - _dst_port(embed_prot, embed_prot_base, remote_side.dst_port()); - - /* update checksum of both IP headers and the ICMP header */ - embed_ip.update_checksum(); - icmp.update_checksum(icmp_sz - sizeof(Icmp_packet)); - ip.update_checksum(); - - /* send adapted packet to all interfaces of remote domain */ - remote_domain.interfaces().for_each([&] (Interface &interface) { - interface.send(eth, size_guard); - }); - /* refresh link only if the error is not about an ICMP query */ - if (embed_prot != L3_protocol::ICMP) { - _link_packet(embed_prot, embed_prot_base, link, client); } - } - /* drop packet if there is no matching link */ - catch (Link_side_tree::No_match) { - throw Drop_packet("no link that matches packet embedded in ICMP error"); } + ); } @@ -1206,29 +1220,39 @@ void Interface::_handle_ip(Ethernet_frame ð, ip.dst(), _dst_port(prot, prot_base) }; /* try to route via existing UDP/TCP links */ - try { - Link_side const &local_side = local_domain.links(prot).find_by_id(local_id); - Link &link = local_side.link(); - bool const client = local_side.is_client(); - Link_side &remote_side = client ? link.server() : link.client(); - Domain &remote_domain = remote_side.domain(); - if (_config().verbose()) { - log("[", local_domain, "] using ", l3_protocol_name(prot), - " link: ", link); - } - _adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain); - ip.src(remote_side.dst_ip()); - ip.dst(remote_side.src_ip()); - _src_port(prot, prot_base, remote_side.dst_port()); - _dst_port(prot, prot_base, remote_side.src_port()); + bool done { false }; + local_domain.links(prot).find_by_id( + local_id, + [&] /* handle_match */ (Link_side const &local_side) + { + Link &link = local_side.link(); + bool const client = local_side.is_client(); + Link_side &remote_side = + client ? link.server() : link.client(); - remote_domain.interfaces().for_each([&] (Interface &interface) { - interface._pass_prot(eth, size_guard, ip, prot, prot_base, prot_size); - }); - _link_packet(prot, prot_base, link, client); + Domain &remote_domain = remote_side.domain(); + if (_config().verbose()) { + log("[", local_domain, "] using ", l3_protocol_name(prot), + " link: ", link); + } + _adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain); + ip.src(remote_side.dst_ip()); + ip.dst(remote_side.src_ip()); + _src_port(prot, prot_base, remote_side.dst_port()); + _dst_port(prot, prot_base, remote_side.src_port()); + + remote_domain.interfaces().for_each([&] (Interface &interface) { + interface._pass_prot( + eth, size_guard, ip, prot, prot_base, prot_size); + }); + _link_packet(prot, prot_base, link, client); + done = true; + }, + [&] /* handle_no_match */ () { } + ); + if (done) { return; } - catch (Link_side_tree::No_match) { } /* try to route via forward rules */ if (local_id.dst_ip == local_intf.address) { diff --git a/repos/os/src/server/nic_router/link.cc b/repos/os/src/server/nic_router/link.cc index 33636d5546..37210359aa 100644 --- a/repos/os/src/server/nic_router/link.cc +++ b/repos/os/src/server/nic_router/link.cc @@ -33,9 +33,9 @@ constexpr size_t Link_side_id::data_size() } -bool Link_side_id::operator == (Link_side_id const &id) const +bool Link_side_id::operator != (Link_side_id const &id) const { - return memcmp(id.data_base(), data_base(), data_size()) == 0; + return memcmp(id.data_base(), data_base(), data_size()) != 0; } @@ -62,20 +62,6 @@ Link_side::Link_side(Domain &domain, } -Link_side const &Link_side::find_by_id(Link_side_id const &id) const -{ - if (id == _id) { - return *this; } - - bool const side = id > _id; - Link_side *const link_side = Avl_node::child(side); - if (!link_side) { - throw Link_side_tree::No_match(); } - - return link_side->find_by_id(id); -} - - void Link_side::print(Output &output) const { Genode::print(output, "src ", src_ip(), ":", src_port(), @@ -89,20 +75,6 @@ bool Link_side::is_client() const } -/******************** - ** Link_side_tree ** - ********************/ - -Link_side const &Link_side_tree::find_by_id(Link_side_id const &id) const -{ - Link_side *const link_side = first(); - if (!link_side) { - throw No_match(); } - - return link_side->find_by_id(id); -} - - /********** ** Link ** **********/ diff --git a/repos/os/src/server/nic_router/link.h b/repos/os/src/server/nic_router/link.h index f46d3509cb..e3bf938c7a 100644 --- a/repos/os/src/server/nic_router/link.h +++ b/repos/os/src/server/nic_router/link.h @@ -78,7 +78,7 @@ struct Net::Link_side_id ** Standard operators ** ************************/ - bool operator == (Link_side_id const &id) const; + bool operator != (Link_side_id const &id) const; bool operator > (Link_side_id const &id) const; } @@ -101,7 +101,33 @@ class Net::Link_side : public Genode::Avl_node Link_side_id const &id, Link &link); - Link_side const &find_by_id(Link_side_id const &id) const; + template + + void find_by_id(Link_side_id const &id, + HANDLE_MATCH_FN && handle_match, + HANDLE_NO_MATCH_FN && handle_no_match) const + { + if (id != _id) { + + Link_side *const link_side_ptr { + Avl_node::child(id > _id) }; + + if (link_side_ptr != nullptr) { + + link_side_ptr->find_by_id( + id, handle_match, handle_no_match); + + } else { + + handle_no_match(); + } + + } else { + + handle_match(*this); + } + } bool is_client() const; @@ -135,9 +161,22 @@ class Net::Link_side : public Genode::Avl_node struct Net::Link_side_tree : Genode::Avl_tree { - struct No_match : Genode::Exception { }; + template - Link_side const &find_by_id(Link_side_id const &id) const; + void find_by_id(Link_side_id const &id, + HANDLE_MATCH_FN && handle_match, + HANDLE_NO_MATCH_FN && handle_no_match) const + { + if (first() != nullptr) { + + first()->find_by_id(id, handle_match, handle_no_match); + + } else { + + handle_no_match(); + } + } };