From 9a94fbb1ec7f88c42de8a09bd9a001997c8f6e01 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Thu, 20 Jul 2023 17:05:42 +0200 Subject: [PATCH] ping: align dhcp client more with nic_router again The DHCP client implementations of Ping originally is a copy of the NIC router implementation adapted for Ping. The two versions diverged further over the years. This issue should be solved by should merging them into a centralized implementation. However, this commit treats only a recent issue with the nic_uplink.run test on pbxa9 qemu but does this by re-aligning the two implementations partially. The final merge should be done in a separate commit. Ref #4966 --- repos/os/src/app/ping/dhcp_client.cc | 41 +++++++++++++++++----------- repos/os/src/app/ping/dhcp_client.h | 5 +++- repos/os/src/app/ping/main.cc | 2 ++ repos/os/src/app/ping/nic.h | 2 ++ 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/repos/os/src/app/ping/dhcp_client.cc b/repos/os/src/app/ping/dhcp_client.cc index b32d603689..c451992f25 100644 --- a/repos/os/src/app/ping/dhcp_client.cc +++ b/repos/os/src/app/ping/dhcp_client.cc @@ -19,8 +19,6 @@ /* Genode includes */ #include -enum { PKT_SIZE = 1024 }; - struct Send_buffer_too_small : Genode::Exception { }; struct Bad_send_dhcp_args : Genode::Exception { }; @@ -41,6 +39,7 @@ void append_param_req_list(Dhcp_options &dhcp_opts) data.append_param_req(); data.append_param_req(); data.append_param_req(); + data.append_param_req(); data.append_param_req(); data.append_param_req(); }); @@ -65,17 +64,20 @@ Dhcp_client::Dhcp_client(Timer::Connection &timer, void Dhcp_client::_discover() { + enum { DISCOVER_PKT_SIZE = 309 }; _set_state(State::SELECT, _discover_timeout); _send(Message_type::DISCOVER, Ipv4_address(), Ipv4_address(), - Ipv4_address()); + Ipv4_address(), DISCOVER_PKT_SIZE); } void Dhcp_client::_rerequest(State next_state) { + enum { REREQUEST_PKT_SIZE = 309 }; _set_state(next_state, _rerequest_timeout(2)); Ipv4_address const client_ip = _handler.ip_config().interface.address; - _send(Message_type::REQUEST, client_ip, Ipv4_address(), client_ip); + _send(Message_type::REQUEST, client_ip, Ipv4_address(), client_ip, + REREQUEST_PKT_SIZE); } @@ -103,9 +105,10 @@ Microseconds Dhcp_client::_rerequest_timeout(unsigned lease_time_div_log2) void Dhcp_client::_handle_timeout(Duration) { switch (_state) { - case State::BOUND: _rerequest(State::RENEW); break; - case State::RENEW: _rerequest(State::REBIND); break; - default: _discover(); + case State::BOUND: _rerequest(State::RENEW); break; + case State::RENEW: _rerequest(State::REBIND); break; + case State::REBIND: _handler.discard_ip_config(); [[fallthrough]]; + default: _discover(); } } @@ -150,9 +153,10 @@ void Dhcp_client::_handle_dhcp_reply(Dhcp_packet &dhcp) throw Drop_packet_inform("DHCP client expects an offer"); } _set_state(State::REQUEST, _request_timeout); + enum { REQUEST_PKT_SIZE = 321 }; _send(Message_type::REQUEST, Ipv4_address(), dhcp.option().value(), - dhcp.yiaddr()); + dhcp.yiaddr(), REQUEST_PKT_SIZE); break; case State::REQUEST: @@ -194,14 +198,16 @@ void Dhcp_client::_handle_dhcp_reply(Dhcp_packet &dhcp) void Dhcp_client::_send(Message_type msg_type, Ipv4_address client_ip, Ipv4_address server_ip, - Ipv4_address requested_ip) + Ipv4_address requested_ip, + size_t pkt_size) { - _nic.send(PKT_SIZE, [&] (void *pkt_base, Size_guard &size_guard) { + Mac_address client_mac = _nic.mac(); + _nic.send(pkt_size, [&] (void *pkt_base, Size_guard &size_guard) { /* create ETH header of the request */ Ethernet_frame ð = Ethernet_frame::construct_at(pkt_base, size_guard); eth.dst(Mac_address(0xff)); - eth.src(_nic.mac()); + eth.src(client_mac); eth.type(Ethernet_frame::Type::IPV4); /* create IP header of the request */ @@ -228,23 +234,24 @@ void Dhcp_client::_send(Message_type msg_type, dhcp.htype(Dhcp_packet::Htype::ETH); dhcp.hlen(sizeof(Mac_address)); dhcp.ciaddr(client_ip); - dhcp.client_mac(_nic.mac()); + dhcp.client_mac(client_mac); dhcp.default_magic_cookie(); /* append DHCP option fields to the request */ + enum { MAX_PKT_SIZE = 1024 }; Dhcp_options dhcp_opts(dhcp, size_guard); dhcp_opts.append_option(msg_type); switch (msg_type) { case Message_type::DISCOVER: append_param_req_list(dhcp_opts); - dhcp_opts.append_option(_nic.mac()); - dhcp_opts.append_option(PKT_SIZE - dhcp_off); + dhcp_opts.append_option(client_mac); + dhcp_opts.append_option(MAX_PKT_SIZE - dhcp_off); break; case Message_type::REQUEST: append_param_req_list(dhcp_opts); - dhcp_opts.append_option(_nic.mac()); - dhcp_opts.append_option(PKT_SIZE - dhcp_off); + dhcp_opts.append_option(client_mac); + dhcp_opts.append_option(MAX_PKT_SIZE - dhcp_off); if (_state == State::REQUEST) { dhcp_opts.append_option(requested_ip); dhcp_opts.append_option(server_ip); @@ -262,4 +269,6 @@ void Dhcp_client::_send(Message_type msg_type, ip.total_length(size_guard.head_size() - ip_off); ip.update_checksum(); }); + + _nic.wakeup_source(); } diff --git a/repos/os/src/app/ping/dhcp_client.h b/repos/os/src/app/ping/dhcp_client.h index 4df8f6cedb..d1d1f083c4 100644 --- a/repos/os/src/app/ping/dhcp_client.h +++ b/repos/os/src/app/ping/dhcp_client.h @@ -44,6 +44,8 @@ class Net::Dhcp_client_handler { public: + virtual void discard_ip_config() = 0; + virtual void ip_config(Ipv4_config const &ip_config) = 0; virtual Ipv4_config const &ip_config() const = 0; @@ -85,7 +87,8 @@ class Net::Dhcp_client void _send(Dhcp_packet::Message_type msg_type, Ipv4_address client_ip, Ipv4_address server_ip, - Ipv4_address requested_ip); + Ipv4_address requested_ip, + Genode::size_t pkt_size); void _discover(); diff --git a/repos/os/src/app/ping/main.cc b/repos/os/src/app/ping/main.cc index 48b323611a..ad2c411ded 100644 --- a/repos/os/src/app/ping/main.cc +++ b/repos/os/src/app/ping/main.cc @@ -125,6 +125,8 @@ class Main : public Nic_handler, ** Dhcp_client_handler ** *************************/ + void discard_ip_config() override { ip_config(Ipv4_config { }); } + void ip_config(Ipv4_config const &ip_config) override; Ipv4_config const &ip_config() const override { return *_ip_config; } diff --git a/repos/os/src/app/ping/nic.h b/repos/os/src/app/ping/nic.h index f29d337c2c..974a0f9935 100644 --- a/repos/os/src/app/ping/nic.h +++ b/repos/os/src/app/ping/nic.h @@ -117,6 +117,8 @@ class Net::Nic Genode::warning("failed to allocate packet"); } } + void wakeup_source() { _source().wakeup(); } + /*************** ** Accessors **