If I understand correctly, the patch from Waldemar Kozaczuk which I just
applied makes this one unnecessary, and I shouldn't try to apply this.
Right?
--
Nadav Har'El
n...@scylladb.com
On Thu, Mar 30, 2017 at 4:29 PM, Justin Cinkelj <justin.cink...@xlab.si>
wrote:
> The old hostname (DNS name) selection for DHCPREQUEST was problematic.
> It was possible to request value returned by gethostname(), even when
> DHCPOFFER suggested a better (more correct) name. To fix this, a value
> from DHCPOFFER should be preffered over gethostname().
>
> On other side, cloud-init metadata disk can set hostname too. And that
> one should be used as DNS name too. So we should prefer value from metadata
> disk over DHCPOFFER, and DHCPOFFER over gethostname. This is done in
> dhcp_interface_state::get_selected_hostname.
>
> Fixes #866
>
> Signed-off-by: Justin Cinkelj <justin.cink...@xlab.si>
> ---
> core/dhcp.cc | 59 ++++++++++++++++++++++++++++++
> ++--------
> include/osv/dhcp.hh | 12 ++++++--
> modules/cloud-init/cloud-init.cc | 4 +--
> 3 files changed, 60 insertions(+), 15 deletions(-)
>
> diff --git a/core/dhcp.cc b/core/dhcp.cc
> index 5d3ec9a..5a32de3 100644
> --- a/core/dhcp.cc
> +++ b/core/dhcp.cc
> @@ -72,9 +72,9 @@ void dhcp_release()
> net_dhcp_worker.release();
> }
>
> -void dhcp_renew(bool wait)
> +void dhcp_renew(bool wait, std::string preferred_hostname)
> {
> - net_dhcp_worker.renew(wait);
> + net_dhcp_worker.renew(wait, preferred_hostname);
> }
>
> namespace dhcp {
> @@ -221,6 +221,7 @@ namespace dhcp {
> u32 xid,
> ip::address_v4 yip,
> ip::address_v4 sip,
> + std::string request_hostname,
> dhcp_request_packet_type
> request_packet_type)
> {
> size_t dhcp_len = sizeof(struct dhcp_packet);
> @@ -256,9 +257,8 @@ namespace dhcp {
> if(request_packet_type == DHCP_REQUEST_SELECTING) {
> options = add_option(options, DHCP_OPTION_DHCP_SERVER, 4,
> (u8*)&dhcp_server_ip);
> }
> - char hostname[256];
> - if (0 == gethostname(hostname, sizeof(hostname))) {
> - options = add_option(options, DHCP_OPTION_HOSTNAME,
> strlen(hostname), (u8*)hostname);
> + if (request_hostname.size()) {
> + options = add_option(options, DHCP_OPTION_HOSTNAME,
> request_hostname.size(), (u8*)request_hostname.c_str());
> }
> if(request_packet_type == DHCP_REQUEST_SELECTING) {
> options = add_option(options, DHCP_OPTION_REQUESTED_ADDRESS,
> 4, (u8*)&requested_ip);
> @@ -553,6 +553,7 @@ namespace dhcp {
> // Save transaction id & send
> _xid = dm.get_xid();
> _client_addr = _server_addr = ipv4_zero;
> + _dhcpack_hostname = "";
> dhcp_i( "Broadcasting DHCPDISCOVER message with xid: [%d]",_xid);
> _sock->dhcp_send(dm);
> }
> @@ -578,6 +579,7 @@ namespace dhcp {
> osv::set_dns_config({}, {});
> // no reply/ack is expected, after send we just forget all old
> state
> _client_addr = _server_addr = ipv4_zero;
> + _dhcpack_hostname = "";
> }
>
> void dhcp_interface_state::renew()
> @@ -588,15 +590,17 @@ namespace dhcp {
> // Compose a dhcp request packet
> dhcp_mbuf dm(false);
> _xid = rand();
> + auto selected_hostname = get_selected_hostname(_
> dhcpack_hostname);
> dm.compose_request(_ifp,
> _xid,
> _client_addr,
> _server_addr,
> + selected_hostname,
> dhcp_mbuf::DHCP_REQUEST_RENEWING);
>
> // Send
> - dhcp_i( "Unicasting DHCPREQUEST message with xid: [%d] from
> client: %s to server: %s in order to RENEW lease of: %s",
> - _xid, _client_addr.to_string().c_str(),
> _server_addr.to_string().c_str(), _client_addr.to_string().c_str());
> + dhcp_i( "Unicasting DHCPREQUEST message with xid: [%d] from
> client: %s to server: %s in order to RENEW lease of: %s (requested name
> %s)",
> + _xid, _client_addr.to_string().c_str(),
> _server_addr.to_string().c_str(), _client_addr.to_string().c_str(),
> selected_hostname.c_str());
> _sock->dhcp_send(dm);
> }
>
> @@ -638,13 +642,15 @@ namespace dhcp {
> // Send a DHCP Request
> _state = DHCP_REQUEST;
> dhcp_mbuf dm_req(false);
> + auto selected_hostname = get_selected_hostname(dm.get_
> hostname());
> dm_req.compose_request(_ifp,
> _xid,
> dm.get_your_ip(),
> dm.get_dhcp_server_ip(),
> + selected_hostname,
> dhcp_mbuf::DHCP_REQUEST_SELECTING);
> - dhcp_i( "Broadcasting DHCPREQUEST message with xid: [%d] to
> SELECT offered IP: %s",
> - _xid, dm.get_your_ip().to_string().c_str());
> + dhcp_i( "Broadcasting DHCPREQUEST message with xid: [%d] to
> SELECT offered IP: %s (requested name %s)",
> + _xid, dm.get_your_ip().to_string().c_str(),
> selected_hostname.c_str());
> _sock->dhcp_send(dm_req);
> }
>
> @@ -656,6 +662,7 @@ namespace dhcp {
> _state = DHCP_ACKNOWLEDGE;
> _client_addr = dm.get_your_ip();
> _server_addr = dm.get_dhcp_server_ip();
> + _dhcpack_hostname = dm.get_hostname();
>
> // TODO: check that the IP address is not responding with ARP
> // RFC2131 section 3.1.5
> @@ -701,8 +708,10 @@ namespace dhcp {
> });
>
> osv::set_dns_config(dm.get_dns_ips(),
> std::vector<std::string>());
> + // TODO the DHCPACK returned hostname could be different then
> DHCPREQUEST requested one.
> + // What is best to do in such case?
> if (dm.get_hostname().size()) {
> - sethostname(dm.get_hostname().c_str(),
> dm.get_hostname().size());
> + sethostname(dm.get_hostname().c_str(),
> dm.get_hostname().size());
> }
> // TODO: setup lease
> } else if (dm.get_message_type() == DHCP_MT_NAK) {
> @@ -711,12 +720,37 @@ namespace dhcp {
> // configuration process."
> _state = DHCP_INIT;
> _client_addr = _server_addr = ipv4_zero;
> + _dhcpack_hostname = "";
> discover();
> }
> // FIXME: retry on timeout and restart DORA sequence if it
> timeout a
> // couple of time
> }
>
> + void dhcp_interface_state::set_preferred_hostname(std::string
> preferred_hostname) {
> + _preferred_hostname = preferred_hostname;
> + }
> +
> + std::string dhcp_interface_state::get_selected_hostname(std::string
> dhcp_suggested_hostname) {
> +
> + // preferred_hostname is set via cloud-init metadata disk - this
> + // is used if real could-init (the http-based one) is not
> available.
> + // If it is not set, then value from dhcpoffer/dhcpack should be
> used.
> + // If also this one is not set, then we can still try to use
> current local hostname.
> + std::string selected_hostname;
> + char local_hostname[256];
> + if (_preferred_hostname.size()) {
> + selected_hostname = _preferred_hostname;
> + }
> + else if (dhcp_suggested_hostname.size()) {
> + selected_hostname = dhcp_suggested_hostname;
> + }
> + else if (0 == gethostname(local_hostname,
> sizeof(local_hostname))) {
> + selected_hostname = local_hostname;
> + }
> + return selected_hostname;
> + }
> +
> ////////////////////////////////////////////////////////////
> ///////////////
>
> dhcp_worker::dhcp_worker()
> @@ -795,8 +829,11 @@ namespace dhcp {
> usleep(1000);
> }
>
> - void dhcp_worker::renew(bool wait)
> + void dhcp_worker::renew(bool wait, std::string preferred_hostname)
> {
> + for (auto &it: _universe) {
> + it.second->set_preferred_hostname(preferred_hostname);
> + }
> _send_and_wait(wait, &dhcp_interface_state::renew);
> }
>
> diff --git a/include/osv/dhcp.hh b/include/osv/dhcp.hh
> index 6797b7f..3c41cb7 100644
> --- a/include/osv/dhcp.hh
> +++ b/include/osv/dhcp.hh
> @@ -25,7 +25,7 @@
> extern "C" {
> void dhcp_start(bool wait);
> void dhcp_release();
> -void dhcp_renew(bool wait);
> +void dhcp_renew(bool wait, std::string preferred_hostname);
> }
>
> namespace dhcp {
> @@ -137,6 +137,7 @@ namespace dhcp {
> u32 xid,
> boost::asio::ip::address_v4 yip,
> boost::asio::ip::address_v4 sip,
> + std::string request_hostname,
> dhcp_request_packet_type
> request_packet_type);
> void compose_release(struct ifnet* ifp,
> boost::asio::ip::address_v4 yip,
> @@ -240,6 +241,8 @@ namespace dhcp {
> void state_request(dhcp_mbuf &dm);
>
> bool is_acknowledged() { return (_state == DHCP_ACKNOWLEDGE); }
> + void set_preferred_hostname(std::string preferred_hostname);
> + std::string get_selected_hostname(std::string
> dhcp_suggested_hostname);
>
> private:
> state _state;
> @@ -247,6 +250,11 @@ namespace dhcp {
> dhcp_socket* _sock;
> boost::asio::ip::address_v4 _client_addr;
> boost::asio::ip::address_v4 _server_addr;
> + // hostname received in DHCPACK, reuse this value if doing renew
> (and
> + // _preferred_hostname is not set)
> + std::string _dhcpack_hostname;
> + // hostname we want to get registered in DNS
> + std::string _preferred_hostname;
>
> // Transaction id
> u32 _xid;
> @@ -267,7 +275,7 @@ namespace dhcp {
> void start(bool wait);
> // Send release packet for all DHCP IPs.
> void release();
> - void renew(bool wait);
> + void renew(bool wait, std::string preferred_hostname);
>
> void dhcp_worker_fn();
> void queue_packet(struct mbuf* m);
> diff --git a/modules/cloud-init/cloud-init.cc b/modules/cloud-init/cloud-
> init.cc
> index 5e05c82..710eb93 100644
> --- a/modules/cloud-init/cloud-init.cc
> +++ b/modules/cloud-init/cloud-init.cc
> @@ -18,7 +18,7 @@
> #include <osv/hypervisor.hh>
>
> // we cannot include osv/dhcp.hh, hence direct declaration.
> -extern "C" void dhcp_renew(bool wait);
> +extern "C" void dhcp_renew(bool wait, std::string preferred_hostname);
>
> // Set the hostname to given string.
> // If hostname changes, try to propagate the change to DHCP server too.
> @@ -28,7 +28,7 @@ void set_hostname_renew_dhcp(std::string hostname) {
> gethostname(old_hostname, sizeof(old_hostname));
> sethostname(hostname.c_str(), hostname.length());
> if (hostname != old_hostname) {
> - dhcp_renew(true);
> + dhcp_renew(true, hostname);
> }
> }
> }
> --
> 2.9.3
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
