nic_router: make Ipv4_config a class

The fact that the IPv4 config was a struct with all data members public was a
mere leftover of an early state of the NIC router. Today, the router
implementation style is to avoid structs and public data members wherever
possible.

This commit slightly changes the behavior of the router regarding log output.
The router used to print malformed IPv4 configurations to the log only if
the 'verbose' config flag was set using this style:

! [my_domain] malformed dynamic IP config: interface 10.0.2.1/24 ...

Now, malformed IPv4 configurations are only printed if the
'verbose_domain_state' config flag is set (like with any IP4v configuration
states) using this style:

! [my_domain] dynamic IP config: malformed (interface 10.0.2.1/24 ...)

Fixes #4242
This commit is contained in:
Martin Stein 2021-08-05 10:41:47 +02:00 committed by Christian Helmuth
parent 9e6f7988c2
commit 1111472af7
7 changed files with 112 additions and 99 deletions

View File

@ -72,7 +72,7 @@ void Dhcp_client::discover()
void Dhcp_client::_rerequest(State next_state)
{
_set_state(next_state, _rerequest_timeout(2));
Ipv4_address const client_ip = _domain().ip_config().interface.address;
Ipv4_address const client_ip = _domain().ip_config().interface().address;
_send(Message_type::REQUEST, client_ip, Ipv4_address(), client_ip);
}

View File

@ -196,7 +196,7 @@ bool Dhcp_server::ready() const
if (!_dns_servers.empty()) {
return true;
}
try { return _dns_server_from().ip_config().valid; }
try { return _dns_server_from().ip_config().valid(); }
catch (Pointer<Domain>::Invalid) { }
return true;
}

View File

@ -42,20 +42,9 @@ Domain_base::Domain_base(Xml_node const node)
void Domain::_log_ip_config() const
{
Ipv4_config const &ip_config = *_ip_config;
if (_config.verbose()) {
if (!ip_config.valid &&
(ip_config.interface_valid ||
ip_config.gateway_valid ||
!ip_config.dns_servers.empty()))
{
log("[", *this, "] malformed ", _ip_config_dynamic ? "dynamic" :
"static", "IP config: ", ip_config);
}
}
if (_config.verbose_domain_state()) {
log("[", *this, "] ", _ip_config_dynamic ? "dynamic" : "static",
" IP config: ", ip_config);
" IP config: ", *_ip_config);
}
}
@ -66,7 +55,7 @@ void Domain::_prepare_reconstructing_ip_config()
throw Ip_config_static(); }
/* discard old IP config if any */
if (ip_config().valid) {
if (ip_config().valid()) {
/* mark IP config invalid */
_ip_config.construct(_alloc);
@ -94,7 +83,7 @@ void Domain::_finish_reconstructing_ip_config()
_log_ip_config();
/* attach all dependent interfaces to new IP config if it is valid */
if (ip_config().valid) {
if (ip_config().valid()) {
_interfaces.for_each([&] (Interface &interface) {
interface.attach_to_ip_config(*this, ip_config());
});
@ -130,9 +119,9 @@ void Domain::ip_config_from_dhcp_ack(Dhcp_packet &dhcp_ack)
void Domain::try_reuse_ip_config(Domain const &domain)
{
if (ip_config().valid ||
if (ip_config().valid() ||
!_ip_config_dynamic ||
!domain.ip_config().valid ||
!domain.ip_config().valid() ||
!domain._ip_config_dynamic)
{
return;
@ -261,7 +250,7 @@ void Domain::init(Domain_tree &domains)
try {
Dhcp_server &dhcp_server = *new (_alloc)
Dhcp_server(dhcp_server_node, *this, _alloc,
ip_config().interface, domains);
ip_config().interface(), domains);
try {
dhcp_server.
@ -355,8 +344,8 @@ void Domain::deinit()
Ipv4_address const &Domain::next_hop(Ipv4_address const &ip) const
{
if (ip_config().interface.prefix_matches(ip)) { return ip; }
if (ip_config().gateway_valid) { return ip_config().gateway; }
if (ip_config().interface().prefix_matches(ip)) { return ip; }
if (ip_config().gateway_valid()) { return ip_config().gateway(); }
throw No_next_hop();
}
@ -401,8 +390,8 @@ void Domain::report(Xml_generator &xml)
empty = false;
}
if (_config.report().config()) {
xml.attribute("ipv4", String<19>(ip_config().interface));
xml.attribute("gw", String<16>(ip_config().gateway));
xml.attribute("ipv4", String<19>(ip_config().interface()));
xml.attribute("gw", String<16>(ip_config().gateway()));
ip_config().for_each_dns_server([&] (Dns_server const &dns_server) {
xml.node("dns", [&] () {
xml.attribute("ip", String<16>(dns_server.ip()));

View File

@ -107,7 +107,7 @@ class Net::Domain : public Domain_base,
unsigned long _interface_cnt { 0 };
Pointer<Dhcp_server> _dhcp_server { };
Genode::Reconstructible<Ipv4_config> _ip_config;
bool const _ip_config_dynamic { !ip_config().valid };
bool const _ip_config_dynamic { !ip_config().valid() };
List<Domain> _ip_config_dependents { };
Arp_cache _arp_cache { *this };
Arp_waiter_list _foreign_arp_waiters { };

View File

@ -414,7 +414,7 @@ void Interface::attach_to_domain_finish()
/* if domain has yet no IP config, participate in requesting one */
Domain &domain = _domain();
Ipv4_config const &ip_config = domain.ip_config();
if (!ip_config.valid) {
if (!ip_config.valid()) {
_dhcp_client->discover();
return;
}
@ -427,7 +427,7 @@ void Interface::attach_to_ip_config(Domain &domain,
{
/* if others wait for ARP at the domain, participate in requesting it */
domain.foreign_arp_waiters().for_each([&] (Arp_waiter_list_element &le) {
_broadcast_arp_request(ip_config.interface.address,
_broadcast_arp_request(ip_config.interface().address,
le.object()->ip());
});
}
@ -555,7 +555,7 @@ void Interface::_adapt_eth(Ethernet_frame &eth,
Domain &remote_domain)
{
Ipv4_config const &remote_ip_cfg = remote_domain.ip_config();
if (!remote_ip_cfg.valid) {
if (!remote_ip_cfg.valid()) {
throw Drop_packet("target domain has yet no IP config");
}
if (remote_domain.use_arp()) {
@ -564,7 +564,7 @@ void Interface::_adapt_eth(Ethernet_frame &eth,
try { eth.dst(remote_domain.arp_cache().find_by_ip(hop_ip).mac()); }
catch (Arp_cache::No_match) {
remote_domain.interfaces().for_each([&] (Interface &interface) {
interface._broadcast_arp_request(remote_ip_cfg.interface.address,
interface._broadcast_arp_request(remote_ip_cfg.interface().address,
hop_ip);
});
try { new (_alloc) Arp_waiter { *this, remote_domain, hop_ip, pkt }; }
@ -595,7 +595,7 @@ void Interface::_nat_link_and_pass(Ethernet_frame &eth,
log("[", local_domain, "] using NAT rule: ", nat); }
_src_port(prot, prot_base, nat.port_alloc(prot).alloc());
ip.src(remote_domain.ip_config().interface.address);
ip.src(remote_domain.ip_config().interface().address);
remote_port_alloc = nat.port_alloc(prot);
}
catch (Nat_rule_tree::No_match) { }
@ -718,7 +718,7 @@ void Interface::_new_dhcp_allocation(Ethernet_frame &eth,
allocation.ip(),
Dhcp_packet::Message_type::OFFER,
dhcp.xid(),
local_domain.ip_config().interface);
local_domain.ip_config().interface());
}
catch (Out_of_ram) { throw Free_resources_and_retry_handle_eth(); }
catch (Out_of_caps) { throw Free_resources_and_retry_handle_eth(); }
@ -743,7 +743,7 @@ void Interface::_handle_dhcp_request(Ethernet_frame &eth,
_dhcp_allocations.find_by_mac(dhcp.client_mac());
Ipv4_address_prefix const &local_intf =
local_domain.ip_config().interface;
local_domain.ip_config().interface();
switch (msg_type) {
case Dhcp_packet::Message_type::DISCOVER:
@ -915,7 +915,7 @@ void Interface::handle_interface_link_state()
/* if the whole domain is down, discard IP config */
Domain &domain_ = domain();
if (!link_state() && domain_.ip_config().valid) {
if (!link_state() && domain_.ip_config().valid()) {
domain_.interfaces().for_each([&] (Interface &interface) {
if (interface.link_state()) {
throw Keep_ip_config(); }
@ -1051,7 +1051,7 @@ void Interface::_handle_icmp_error(Ethernet_frame &eth,
}
/* 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) {
if (remote_side.dst_ip() == remote_domain.ip_config().interface().address) {
ip.src(remote_side.dst_ip());
}
ip.dst(remote_side.src_ip());
@ -1123,7 +1123,7 @@ void Interface::_handle_ip(Ethernet_frame &eth,
{
/* drop fragmented IPv4 as it isn't supported */
Ipv4_packet &ip = eth.data<Ipv4_packet>(size_guard);
Ipv4_address_prefix const &local_intf = local_domain.ip_config().interface;
Ipv4_address_prefix const &local_intf = local_domain.ip_config().interface();
if (ip.more_fragments() ||
ip.fragment_offset() != 0) {
@ -1350,7 +1350,7 @@ void Interface::_handle_arp_reply(Ethernet_frame &eth,
destroy(waiter.src()._alloc, &waiter);
}
}
Ipv4_address_prefix const &local_intf = local_domain.ip_config().interface;
Ipv4_address_prefix const &local_intf = local_domain.ip_config().interface();
if (local_intf.prefix_matches(arp.dst_ip()) &&
arp.dst_ip() != local_intf.address)
{
@ -1400,7 +1400,7 @@ void Interface::_handle_arp_request(Ethernet_frame &eth,
Domain &local_domain)
{
Ipv4_config const &local_ip_cfg = local_domain.ip_config();
Ipv4_address_prefix const &local_intf = local_ip_cfg.interface;
Ipv4_address_prefix const &local_intf = local_ip_cfg.interface();
if (local_intf.prefix_matches(arp.dst_ip())) {
/* ARP request for an IP local to the domain's subnet */
@ -1429,7 +1429,7 @@ void Interface::_handle_arp_request(Ethernet_frame &eth,
} else {
/* ARP request for an IP foreign to the domain's subnet */
if (local_ip_cfg.gateway_valid) {
if (local_ip_cfg.gateway_valid()) {
/* leave request up to the gateway of the domain */
throw Drop_packet("leave ARP request up to gateway");
@ -1542,7 +1542,7 @@ void Interface::_handle_eth(Ethernet_frame &eth,
Packet_descriptor const &pkt,
Domain &local_domain)
{
if (local_domain.ip_config().valid) {
if (local_domain.ip_config().valid()) {
switch (eth.type()) {
case Ethernet_frame::Type::ARP: _handle_arp(eth, size_guard, local_domain); break;
@ -1801,7 +1801,7 @@ void Interface::_update_link_check_nat(Link &link,
return;
}
try {
if (link.server().dst_ip() != new_srv_dom.ip_config().interface.address) {
if (link.server().dst_ip() != new_srv_dom.ip_config().interface().address) {
_dismiss_link_log(link, "NAT IP");
throw Dismiss_link();
}
@ -2125,7 +2125,7 @@ void Interface::handle_config_3()
return;
}
/* if there was/is no IP config, there is nothing more to update */
if (!new_domain.ip_config().valid) {
if (!new_domain.ip_config().valid()) {
return;
}
/* update state objects */

View File

@ -23,30 +23,30 @@ using namespace Net;
Ipv4_config::Ipv4_config(Allocator &alloc)
:
alloc { alloc },
interface { },
gateway { }
_alloc { alloc },
_interface { },
_gateway { }
{ }
Ipv4_config::Ipv4_config(Xml_node const &domain_node,
Allocator &alloc)
:
alloc { alloc },
interface { domain_node.attribute_value("interface", Ipv4_address_prefix()) },
gateway { domain_node.attribute_value("gateway", Ipv4_address()) }
_alloc { alloc },
_interface { domain_node.attribute_value("interface", Ipv4_address_prefix()) },
_gateway { domain_node.attribute_value("gateway", Ipv4_address()) }
{ }
Ipv4_config::Ipv4_config(Ipv4_config const &ip_config,
Allocator &alloc)
:
alloc { alloc },
interface { ip_config.interface },
gateway { ip_config.gateway }
_alloc { alloc },
_interface { ip_config._interface },
_gateway { ip_config._gateway }
{
ip_config.dns_servers.for_each([&] (Dns_server const &dns_server) {
dns_servers.insert_as_tail(
ip_config._dns_servers.for_each([&] (Dns_server const &dns_server) {
_dns_servers.insert_as_tail(
*new (alloc) Dns_server(dns_server.ip()));
});
}
@ -55,10 +55,10 @@ Ipv4_config::Ipv4_config(Ipv4_config const &ip_config,
Ipv4_config::Ipv4_config(Dhcp_packet &dhcp_ack,
Allocator &alloc)
:
alloc { alloc },
interface { dhcp_ack.yiaddr(),
_alloc { alloc },
_interface { dhcp_ack.yiaddr(),
dhcp_ipv4_option<Dhcp_packet::Subnet_mask>(dhcp_ack) },
gateway { dhcp_ipv4_option<Dhcp_packet::Router_ipv4>(dhcp_ack) }
_gateway { dhcp_ipv4_option<Dhcp_packet::Router_ipv4>(dhcp_ack) }
{
try {
@ -66,7 +66,7 @@ Ipv4_config::Ipv4_config(Dhcp_packet &dhcp_ack,
dhcp_ack.option<Dhcp_packet::Dns_server>() };
dns_server.for_each_address([&] (Ipv4_address const &addr) {
dns_servers.insert_as_tail(*new (alloc) Dns_server(addr));
_dns_servers.insert_as_tail(*new (alloc) Dns_server(addr));
});
}
catch (Dhcp_packet::Option_not_found) { }
@ -75,20 +75,30 @@ Ipv4_config::Ipv4_config(Dhcp_packet &dhcp_ack,
Ipv4_config::~Ipv4_config()
{
dns_servers.destroy_each(alloc);
_dns_servers.destroy_each(_alloc);
}
void Ipv4_config::print(Output &output) const
{
if (valid) {
if (_valid) {
Genode::print(output, "interface ", interface, ", gateway ", gateway,
" P2P ", point_to_point);
Genode::print(output, "interface ", _interface, ", gateway ", _gateway,
", P2P ", _point_to_point);
for_each_dns_server([&] (Dns_server const &dns_server) {
Genode::print(output, ", DNS server ", dns_server.ip()); });
} else if (_interface_valid || _gateway_valid || !_dns_servers.empty()) {
Genode::print(output, "malformed (interface ", _interface,
", gateway ", _gateway, ", P2P ", _point_to_point);
for_each_dns_server([&] (Dns_server const &dns_server) {
Genode::print(output, ", DNS server ", dns_server.ip()); });
Genode::print(output, ")");
} else {
Genode::print(output, "none");

View File

@ -24,21 +24,25 @@
namespace Net { class Ipv4_config; }
struct Net::Ipv4_config
class Net::Ipv4_config
{
Genode::Allocator &alloc;
Ipv4_address_prefix const interface;
bool const interface_valid { interface.valid() };
Ipv4_address const gateway;
bool const gateway_valid { gateway.valid() };
bool const point_to_point { gateway_valid &&
interface_valid &&
interface.prefix == 32 };
Net::List<Dns_server> dns_servers { };
bool const valid { point_to_point ||
(interface_valid &&
(!gateway_valid ||
interface.prefix_matches(gateway))) };
private:
Genode::Allocator &_alloc;
Ipv4_address_prefix const _interface;
bool const _interface_valid { _interface.valid() };
Ipv4_address const _gateway;
bool const _gateway_valid { _gateway.valid() };
bool const _point_to_point { _gateway_valid &&
_interface_valid &&
_interface.prefix == 32 };
Net::List<Dns_server> _dns_servers { };
bool const _valid { _point_to_point ||
(_interface_valid &&
(!_gateway_valid ||
_interface.prefix_matches(_gateway))) };
public:
Ipv4_config(Net::Dhcp_packet &dhcp_ack,
Genode::Allocator &alloc);
@ -55,15 +59,15 @@ struct Net::Ipv4_config
bool operator != (Ipv4_config const &other) const
{
return interface != other.interface ||
gateway != other.gateway ||
!dns_servers.equal_to(other.dns_servers);
return _interface != other._interface ||
_gateway != other._gateway ||
!_dns_servers.equal_to(other._dns_servers);
}
template <typename FUNC>
void for_each_dns_server(FUNC && functor) const
{
dns_servers.for_each([&] (Dns_server const &dns_server) {
_dns_servers.for_each([&] (Dns_server const &dns_server) {
functor(dns_server);
});
}
@ -74,6 +78,16 @@ struct Net::Ipv4_config
*********/
void print(Genode::Output &output) const;
/***************
** Accessors **
***************/
bool valid() const { return _valid; }
Ipv4_address_prefix const &interface() const { return _interface; }
Ipv4_address const &gateway() const { return _gateway; }
bool gateway_valid() const { return _gateway_valid; }
};
#endif /* _IPV4_CONFIG_H_ */