nic_router: simplify routing rule classes

* Get rid of the base classes Rule and Leaf_rule,
* Make log output about initiated or invalid routing rules conform to the rest
  of the router log, and
* Ensure that each type of routing rule when being invalid invalidates its
  whole domain.

Issue #2840

repos/os/src/server/nic_router/direct_rule.h

@@ -16,7 +16,6 @@
 
 /* local includes */
 #include <ipv4_address_prefix.h>
-#include <rule.h>
 #include <list.h>
 
 /* Genode includes */
@@ -33,7 +32,7 @@ namespace Net {
 }
 
 
-class Net::Direct_rule_base : public Rule
+class Net::Direct_rule_base
 {
 	protected:
 
@@ -41,6 +40,8 @@ class Net::Direct_rule_base : public Rule
 
 	public:
 
+		struct Invalid : Genode::Exception { };
+
 		Direct_rule_base(Genode::Xml_node const node);
 
 

repos/os/src/server/nic_router/domain.cc

@@ -143,16 +143,21 @@ void Domain::_read_forward_rules(Cstring  const    &protocol,
 			Forward_rule &rule = *new (_alloc) Forward_rule(domains, node);
 			rules.insert(&rule);
 			if (_config.verbose()) {
-				log("[", *this, "] forward rule: ", protocol, " ", rule); }
-		}
-		catch (Rule::Invalid) {
-			log("[", *this, "] invalid domain (invalid forward rule)");
-			throw Invalid();
+				log("[", *this, "] ", protocol, " forward rule: ", rule); }
 		}
+		catch (Forward_rule::Invalid) { _invalid("invalid forward rule"); }
 	});
 }
 
 
+void Domain::_invalid(char const *reason) const
+{
+	if (_config.verbose()) {
+		log("[", *this, "] invalid domain (", reason, ")"); }
+	throw Invalid();
+}
+
+
 void Domain::_read_transport_rules(Cstring  const      &protocol,
                                    Domain_tree         &domains,
                                    Xml_node const       node,
@@ -161,13 +166,12 @@ void Domain::_read_transport_rules(Cstring  const      &protocol,
 {
 	node.for_each_sub_node(type, [&] (Xml_node const node) {
 		try {
-			rules.insert(*new (_alloc) Transport_rule(domains, node, _alloc,
-			                                          protocol, _config));
-		}
-		catch (Rule::Invalid) {
-			log("[", *this, "] invalid domain (invalid transport rule)");
-			throw Invalid();
+			rules.insert(*new (_alloc)
+				Transport_rule(domains, node, _alloc, protocol, _config, *this));
 		}
+		catch (Transport_rule::Invalid)     { _invalid("invalid transport rule"); }
+		catch (Permit_any_rule::Invalid)    { _invalid("invalid permit-any rule"); }
+		catch (Permit_single_rule::Invalid) { _invalid("invalid permit rule"); }
 	});
 }
 
@@ -253,9 +257,8 @@ void Domain::init(Domain_tree &domains)
 	try {
 		Xml_node const dhcp_server_node = _node.sub_node("dhcp-server");
 		if (_ip_config_dynamic) {
-			log("[", *this, "] invalid domain (DHCP server and client at once)");
-			throw Invalid();
-		}
+			_invalid("DHCP server and client at once"); }
+
 		Dhcp_server &dhcp_server = *new (_alloc)
 			Dhcp_server(dhcp_server_node, *this, _alloc,
 			            ip_config().interface, domains);
@@ -268,11 +271,7 @@ void Domain::init(Domain_tree &domains)
 			log("[", *this, "] DHCP server: ", _dhcp_server()); }
 	}
 	catch (Xml_node::Nonexistent_sub_node) { }
-	catch (Dhcp_server::Invalid) {
-		if (_config.verbose()) {
-			log("[", *this, "] invalid domain (invalid DHCP server)"); }
-		throw Invalid();
-	}
+	catch (Dhcp_server::Invalid) { _invalid("invalid DHCP server"); }
 
 	/* read forward rules */
 	_read_forward_rules(tcp_name(), domains, _node, "tcp-forward",
@@ -287,31 +286,24 @@ void Domain::init(Domain_tree &domains)
 	/* read NAT rules */
 	_node.for_each_sub_node("nat", [&] (Xml_node const node) {
 		try {
-			_nat_rules.insert(
-				new (_alloc) Nat_rule(domains, _tcp_port_alloc,
-				                      _udp_port_alloc, _icmp_port_alloc,
-				                      node));
-		}
-		catch (Rule::Invalid) {
-			log("[", *this, "] invalid domain (invalid NAT rule)");
-			throw Invalid();
+			Nat_rule &rule = *new (_alloc)
+				Nat_rule(domains, _tcp_port_alloc, _udp_port_alloc,
+				         _icmp_port_alloc, node);
+			_nat_rules.insert(&rule);
+			if (_config.verbose()) {
+				log("[", *this, "] NAT rule: ", rule); }
 		}
+		catch (Nat_rule::Invalid) { _invalid("invalid NAT rule"); }
 	});
 	/* read ICMP rules */
 	_node.for_each_sub_node("icmp", [&] (Xml_node const node) {
 		try { _icmp_rules.insert(*new (_alloc) Ip_rule(domains, node)); }
-		catch (Rule::Invalid) {
-			log("[", *this, "] invalid domain (invalid ICMP rule)");
-			throw Invalid();
-		}
+		catch (Ip_rule::Invalid) { _invalid("invalid ICMP rule"); }
 	});
 	/* read IP rules */
 	_node.for_each_sub_node("ip", [&] (Xml_node const node) {
 		try { _ip_rules.insert(*new (_alloc) Ip_rule(domains, node)); }
-		catch (Rule::Invalid) {
-			log("[", *this, "] invalid domain (invalid IP rule)");
-			throw Invalid();
-		}
+		catch (Ip_rule::Invalid) { _invalid("invalid IP rule"); }
 	});
 }
 

repos/os/src/server/nic_router/domain.h

@@ -125,6 +125,8 @@ class Net::Domain : public Domain_base,
 		                           char             const *type,
 		                           Transport_rule_list    &rules);
 
+		void _invalid(char const *reason) const;
+
 		void __FIXME__dissolve_foreign_arp_waiters();
 
 	public:

repos/os/src/server/nic_router/forward_rule.cc

@@ -26,18 +26,29 @@ using namespace Genode;
  ** Forward_rule **
  ******************/
 
+
+Domain &Forward_rule::_find_domain(Domain_tree    &domains,
+                                   Xml_node const  node)
+{
+	try {
+		return domains.find_by_name(
+			node.attribute_value("domain", Domain_name()));
+	}
+	catch (Domain_tree::No_match) { throw Invalid(); }
+}
+
+
 void Forward_rule::print(Output &output) const
 {
-	Genode::print(output, "port ", _port, " requests to ", _to,
-	              " at ", _domain);
+	Genode::print(output, "port ", _port, " domain ", _domain, " to ", _to);
 }
 
 
 Forward_rule::Forward_rule(Domain_tree &domains, Xml_node const node)
 :
-	Leaf_rule(domains, node),
 	_port(node.attribute_value("port", Port(0))),
-	_to(node.attribute_value("to", Ipv4_address()))
+	_to(node.attribute_value("to", Ipv4_address())),
+	_domain(_find_domain(domains, node))
 {
 	if (_port == Port(0) || !_to.valid() || dynamic_port(_port)) {
 		throw Invalid(); }

repos/os/src/server/nic_router/forward_rule.h

@@ -15,7 +15,6 @@
 #define _FORWARD_RULE_H_
 
 /* local includes */
-#include <leaf_rule.h>
 #include <avl_tree.h>
 
 /* Genode includes */
@@ -23,8 +22,13 @@
 #include <net/ipv4.h>
 #include <net/port.h>
 
+namespace Genode { class Xml_node; }
+
 namespace Net {
 
+	class Domain;
+	class Domain_tree;
+
 	class Forward_rule;
 	class Forward_rule_tree;
 	class Forward_link;
@@ -32,16 +36,21 @@ namespace Net {
 }
 
 
-class Net::Forward_rule : public Leaf_rule,
-                          public Genode::Avl_node<Forward_rule>
+class Net::Forward_rule : public Genode::Avl_node<Forward_rule>
 {
 	private:
 
 		Port         const  _port;
 		Ipv4_address const  _to;
+		Domain             &_domain;
+
+		static Domain &_find_domain(Domain_tree            &domains,
+		                            Genode::Xml_node const  node);
 
 	public:
 
+		struct Invalid : Genode::Exception { };
+
 		Forward_rule(Domain_tree &domains, Genode::Xml_node const node);
 
 		Forward_rule const &find_by_port(Port const port) const;
@@ -67,6 +76,7 @@ class Net::Forward_rule : public Leaf_rule,
 		 ***************/
 
 		Ipv4_address const &to()     const { return _to; }
+		Domain             &domain() const { return _domain; }
 };
 
 

repos/os/src/server/nic_router/interface.h

@@ -33,7 +33,6 @@ namespace Net {
 	using Packet_stream_sink   = ::Nic::Packet_stream_sink< ::Nic::Session::Policy>;
 	using Packet_stream_source = ::Nic::Packet_stream_source< ::Nic::Session::Policy>;
 	using Domain_name          = Genode::String<160>;
-	class Leaf_rule;
 	class Ipv4_config;
 	class Forward_rule_tree;
 	class Transport_rule_list;

repos/os/src/server/nic_router/ip_rule.cc

@@ -13,13 +13,25 @@
 
 /* local includes */
 #include <ip_rule.h>
+#include <domain.h>
 
 using namespace Net;
 using namespace Genode;
 
 
+Domain &Ip_rule::_find_domain(Domain_tree    &domains,
+                              Xml_node const  node)
+{
+	try {
+		return domains.find_by_name(
+			node.attribute_value("domain", Domain_name()));
+	}
+	catch (Domain_tree::No_match) { throw Invalid(); }
+}
+
+
 Ip_rule::Ip_rule(Domain_tree &domains, Xml_node const node)
 :
-	Leaf_rule(domains, node),
-	Direct_rule(node)
+	Direct_rule(node),
+	_domain(_find_domain(domains, node))
 { }

repos/os/src/server/nic_router/ip_rule.h

@@ -15,21 +15,37 @@
 #define _IP_RULE_H_
 
 /* local includes */
-#include <leaf_rule.h>
 #include <direct_rule.h>
 
 namespace Net {
 
+	class Domain;
+	class Domain_tree;
+
 	class  Ip_rule;
 	struct Ip_rule_list : Direct_rule_list<Ip_rule> { };
 }
 
 
-struct Net::Ip_rule : Leaf_rule, Direct_rule<Ip_rule>
+class Net::Ip_rule : public Direct_rule<Ip_rule>
 {
+	private:
+
+		Domain &_domain;
+
+		static Domain &_find_domain(Domain_tree            &domains,
+		                            Genode::Xml_node const  node);
+
 	public:
 
 		Ip_rule(Domain_tree &domains, Genode::Xml_node const node);
+
+
+		/***************
+		 ** Accessors **
+		 ***************/
+
+		Domain &domain() const { return _domain; }
 };
 
 #endif /* _IP_RULE_H_ */

repos/os/src/server/nic_router/leaf_rule.cc

@@ -1,36 +0,0 @@
-/*
- * \brief  Routing rule that defines a target interface
- * \author Martin Stein
- * \date   2016-08-19
- */
-
-/*
- * Copyright (C) 2016-2017 Genode Labs GmbH
- *
- * This file is part of the Genode OS framework, which is distributed
- * under the terms of the GNU Affero General Public License version 3.
- */
-
-/* local includes */
-#include <leaf_rule.h>
-#include <domain.h>
-
-using namespace Net;
-using namespace Genode;
-
-
-Domain &Leaf_rule::_find_domain(Domain_tree    &domains,
-                                Xml_node const  node)
-{
-	try {
-		return domains.find_by_name(
-			node.attribute_value("domain", Domain_name()));
-	}
-	catch (Domain_tree::No_match) { throw Invalid(); }
-}
-
-
-Leaf_rule::Leaf_rule(Domain_tree &domains, Xml_node const node)
-:
-	_domain(_find_domain(domains, node))
-{ }

repos/os/src/server/nic_router/leaf_rule.h

@@ -1,50 +0,0 @@
-/*
- * \brief  Routing rule that defines a target interface
- * \author Martin Stein
- * \date   2016-08-19
- */
-
-/*
- * Copyright (C) 2016-2017 Genode Labs GmbH
- *
- * This file is part of the Genode OS framework, which is distributed
- * under the terms of the GNU Affero General Public License version 3.
- */
-
-#ifndef _LEAF_RULE_H_
-#define _LEAF_RULE_H_
-
-/* local includes */
-#include <rule.h>
-
-namespace Genode { class Xml_node; }
-
-namespace Net {
-
-	class Domain;
-	class Domain_tree;
-	class Leaf_rule;
-}
-
-class Net::Leaf_rule : public Rule
-{
-	protected:
-
-		Domain &_domain;
-
-		static Domain &_find_domain(Domain_tree            &domains,
-		                            Genode::Xml_node const  node);
-
-	public:
-
-		Leaf_rule(Domain_tree &domains, Genode::Xml_node const node);
-
-
-		/***************
-		 ** Accessors **
-		 ***************/
-
-		Domain &domain() const { return _domain; }
-};
-
-#endif /* _LEAF_RULE_H_ */

repos/os/src/server/nic_router/nat_rule.cc

@@ -26,6 +26,17 @@ using namespace Net;
 using namespace Genode;
 
 
+Domain &Nat_rule::_find_domain(Domain_tree    &domains,
+                               Xml_node const  node)
+{
+	try {
+		return domains.find_by_name(
+			node.attribute_value("domain", Domain_name()));
+	}
+	catch (Domain_tree::No_match) { throw Invalid(); }
+}
+
+
 bool Nat_rule::higher(Nat_rule *rule)
 {
 	return (addr_t)&rule->domain() > (addr_t)&_domain;
@@ -38,7 +49,7 @@ Nat_rule::Nat_rule(Domain_tree    &domains,
                    Port_allocator &icmp_port_alloc,
                    Xml_node const  node)
 :
-	Leaf_rule(domains, node),
+	_domain(_find_domain(domains, node)),
 	_tcp_port_alloc (tcp_port_alloc,  node.attribute_value("tcp-ports", 0UL)),
 	_udp_port_alloc (udp_port_alloc,  node.attribute_value("udp-ports", 0UL)),
 	_icmp_port_alloc(icmp_port_alloc, node.attribute_value("icmp-ids", 0UL))

repos/os/src/server/nic_router/nat_rule.h

@@ -16,7 +16,6 @@
 
 /* local includes */
 #include <port_allocator.h>
-#include <leaf_rule.h>
 #include <l3_protocol.h>
 #include <avl_tree.h>
 
@@ -25,6 +24,9 @@
 
 namespace Net {
 
+	class Domain;
+	class Domain_tree;
+
 	class Port_allocator;
 	class Nat_rule_base;
 	class Nat_rule;
@@ -32,17 +34,22 @@ namespace Net {
 }
 
 
-class Net::Nat_rule : public Leaf_rule,
-                      public Genode::Avl_node<Nat_rule>
+class Net::Nat_rule : public Genode::Avl_node<Nat_rule>
 {
 	private:
 
+		Domain               &_domain;
 		Port_allocator_guard  _tcp_port_alloc;
 		Port_allocator_guard  _udp_port_alloc;
 		Port_allocator_guard  _icmp_port_alloc;
 
+		static Domain &_find_domain(Domain_tree            &domains,
+		                            Genode::Xml_node const  node);
+
 	public:
 
+		struct Invalid : Genode::Exception { };
+
 		Nat_rule(Domain_tree            &domains,
 		         Port_allocator         &tcp_port_alloc,
 		         Port_allocator         &udp_port_alloc,
@@ -72,6 +79,7 @@ class Net::Nat_rule : public Leaf_rule,
 		 ** Accessors **
 		 ***************/
 
+		Domain               &domain()    const { return _domain; }
 		Port_allocator_guard &tcp_port_alloc()  { return _tcp_port_alloc; }
 		Port_allocator_guard &udp_port_alloc()  { return _udp_port_alloc; }
 		Port_allocator_guard &icmp_port_alloc() { return _icmp_port_alloc; }

repos/os/src/server/nic_router/permit_rule.cc

@@ -22,29 +22,30 @@ using namespace Net;
 using namespace Genode;
 
 
-/*****************
- ** Permit_rule **
- *****************/
-
-Permit_rule::Permit_rule(Domain_tree &domains, Xml_node const node)
-:
-	Leaf_rule(domains, node)
-{ }
-
-
 /*********************
  ** Permit_any_rule **
  *********************/
 
+Domain &Permit_any_rule::_find_domain(Domain_tree    &domains,
+                                      Xml_node const  node)
+{
+	try {
+		return domains.find_by_name(
+			node.attribute_value("domain", Domain_name()));
+	}
+	catch (Domain_tree::No_match) { throw Invalid(); }
+}
+
+
 void Permit_any_rule::print(Output &output) const
 {
-	Genode::print(output, "requests to ", domain());
+	Genode::print(output, "domain ", domain());
 }
 
 
 Permit_any_rule::Permit_any_rule(Domain_tree &domains, Xml_node const node)
 :
-	Permit_rule(domains, node)
+	Permit_rule(_find_domain(domains, node))
 { }
 
 
@@ -52,6 +53,17 @@ Permit_any_rule::Permit_any_rule(Domain_tree &domains, Xml_node const node)
  ** Permit_single_rule **
  ************************/
 
+Domain &Permit_single_rule::_find_domain(Domain_tree    &domains,
+                                         Xml_node const  node)
+{
+	try {
+		return domains.find_by_name(
+			node.attribute_value("domain", Domain_name()));
+	}
+	catch (Domain_tree::No_match) { throw Invalid(); }
+}
+
+
 bool Permit_single_rule::higher(Permit_single_rule *rule)
 {
 	return rule->_port.value > _port.value;
@@ -60,14 +72,14 @@ bool Permit_single_rule::higher(Permit_single_rule *rule)
 
 void Permit_single_rule::print(Output &output) const
 {
-	Genode::print(output, "port ", _port, " requests to ", domain());
+	Genode::print(output, "port ", _port, " domain ", domain());
 }
 
 
 Permit_single_rule::Permit_single_rule(Domain_tree    &domains,
                                        Xml_node const  node)
 :
-	Permit_rule(domains, node),
+	Permit_rule(_find_domain(domains, node)),
 	_port(node.attribute_value("port", Port(0)))
 {
 	if (_port == Port(0) || dynamic_port(_port)) {

repos/os/src/server/nic_router/permit_rule.h

@@ -15,18 +15,23 @@
 #define _PERMIT_RULE_H_
 
 /* local includes */
-#include <leaf_rule.h>
 #include <avl_tree.h>
 
 /* Genode includes */
 #include <util/avl_tree.h>
 #include <net/port.h>
 
-namespace Genode { class Output; }
+namespace Genode {
+
+	class Output;
+	class Xml_node;
+}
 
 namespace Net {
 
 	class Interface;
+	class Domain;
+	class Domain_tree;
 
 	class Permit_rule;
 	class Permit_any_rule;
@@ -35,14 +40,17 @@ namespace Net {
 }
 
 
-struct Net::Permit_rule : private Leaf_rule, public Genode::Interface
+struct Net::Permit_rule : public Genode::Interface
 {
 	friend class Interface;
 
-	Permit_rule(Domain_tree &domains, Genode::Xml_node const node);
+	private:
 
-	using Leaf_rule::domain;
-	using Leaf_rule::Invalid;
+		Domain &_domain;
+
+	public:
+
+		Permit_rule(Domain &domain) : _domain(domain) { };
 
 
 		/*********
@@ -50,11 +58,27 @@ struct Net::Permit_rule : private Leaf_rule, public Genode::Interface
 		 *********/
 
 		virtual void print(Genode::Output &output) const = 0;
+
+
+		/***************
+		 ** Accessors **
+		 ***************/
+
+		Domain &domain() const { return _domain; }
 };
 
 
 struct Net::Permit_any_rule : Permit_rule
 {
+	private:
+
+		static Domain &_find_domain(Domain_tree            &domains,
+		                            Genode::Xml_node const  node);
+
+	public:
+
+		struct Invalid : Genode::Exception { };
+
 		Permit_any_rule(Domain_tree &domains, Genode::Xml_node const node);
 
 
@@ -78,8 +102,13 @@ class Net::Permit_single_rule : public  Permit_rule,
 
 		Port const _port;
 
+		static Domain &_find_domain(Domain_tree            &domains,
+		                            Genode::Xml_node const  node);
+
 	public:
 
+		struct Invalid : Genode::Exception { };
+
 		Permit_single_rule(Domain_tree            &domains,
 		                   Genode::Xml_node const  node);
 

repos/os/src/server/nic_router/rule.h

@@ -1,22 +0,0 @@
-/*
- * \brief  Base of each routing rule
- * \author Martin Stein
- * \date   2016-08-19
- */
-
-/*
- * Copyright (C) 2016-2017 Genode Labs GmbH
- *
- * This file is part of the Genode OS framework, which is distributed
- * under the terms of the GNU Affero General Public License version 3.
- */
-
-#ifndef _RULE_H_
-#define _RULE_H_
-
-/* Genode includes */
-#include <base/exception.h>
-
-namespace Net { struct Rule { struct Invalid : Genode::Exception { }; }; }
-
-#endif /* _RULE_H_ */

repos/os/src/server/nic_router/target.mk

@@ -7,7 +7,7 @@ SRC_CC += component.cc port_allocator.cc forward_rule.cc
 SRC_CC += nat_rule.cc main.cc ipv4_config.cc
 SRC_CC += uplink.cc interface.cc arp_cache.cc configuration.cc
 SRC_CC += domain.cc l3_protocol.cc direct_rule.cc link.cc
-SRC_CC += transport_rule.cc leaf_rule.cc permit_rule.cc
+SRC_CC += transport_rule.cc permit_rule.cc
 SRC_CC += dhcp_client.cc dhcp_server.cc report.cc xml_node.cc
 
 INC_DIR += $(PRG_DIR)

repos/os/src/server/nic_router/transport_rule.cc

@@ -35,7 +35,6 @@ Transport_rule::_read_permit_any_rule(Domain_tree    &domains,
 		                                Permit_any_rule(domains, sub_node));
 	}
 	catch (Xml_node::Nonexistent_sub_node) { }
-	catch (Rule::Invalid) { warning("invalid permit-any rule"); }
 	return Pointer<Permit_any_rule>();
 }
 
@@ -44,7 +43,8 @@ Transport_rule::Transport_rule(Domain_tree    &domains,
                                Xml_node const  node,
                                Allocator      &alloc,
                                Cstring  const &protocol,
-                               Configuration  &config)
+                               Configuration  &config,
+                               Domain   const &domain)
 :
 	Direct_rule(node),
 	_alloc(alloc),
@@ -54,26 +54,27 @@ Transport_rule::Transport_rule(Domain_tree    &domains,
 	try {
 		Permit_any_rule &permit_any_rule = _permit_any_rule();
 		if (config.verbose()) {
-			log("  ", protocol, " rule: ", _dst, " ", permit_any_rule); }
-
+			log("[", domain, "] ", protocol, " permit-any rule: ", permit_any_rule);
+			log("[", domain, "] ", protocol, " rule: dst ", _dst);
+		}
 		return;
 	} catch (Pointer<Permit_any_rule>::Invalid) { }
 
 	/* read specific permit rules */
 	node.for_each_sub_node("permit", [&] (Xml_node const node) {
-		try {
 		Permit_single_rule &rule = *new (alloc)
 			Permit_single_rule(domains, node);
 
 		_permit_single_rules.insert(&rule);
 		if (config.verbose()) {
-				log("  ", protocol, " rule: ", _dst, " ", rule); }
-		}
-		catch (Rule::Invalid) { warning("invalid permit rule"); }
+			log("[", domain, "] ", protocol, " permit rule: ", rule); }
 	});
 	/* drop the transport rule if it has no permitted ports */
 	if (!_permit_single_rules.first()) {
 		throw Invalid(); }
+
+	if (config.verbose()) {
+		log("[", domain, "] ", protocol, " rule: dst ", _dst); }
 }
 
 

repos/os/src/server/nic_router/transport_rule.h

@@ -48,7 +48,8 @@ class Net::Transport_rule : public Direct_rule<Transport_rule>
 		               Genode::Xml_node const  node,
 		               Genode::Allocator      &alloc,
 		               Genode::Cstring  const &protocol,
-		               Configuration          &config);
+		               Configuration          &config,
+		               Domain           const &domain);
 
 		~Transport_rule();