In general, I think this is the right direction, but there's something I am not sure I understand: It seems to me that the old code had a loop checking that the new DHPC request worked. The current code, doesn't it just send the updated DHCP request once, and forgets about it? It doesn't wait for it to take effect, and it can even be lost on the network. Don't we need a retry loop here too?
-- Nadav Har'El [email protected] On Fri, Dec 23, 2016 at 12:15 AM, Justin Cinkelj <[email protected]> wrote: > We want to propagate new hostname to DNS server via DHCP server. > Previously, a DHCP release followed by a new DORA cycle was used. > This did update DNS name, but VM was for a short period of time > without a valid IP. > > Now an early lease renew (with the new hostname included) is sent. > Thus VM does not need to stop using assigned IP address. > > Fixes #816 > > Signed-off-by: Justin Cinkelj <[email protected]> > --- > core/dhcp.cc | 33 +++++++++++++++++++++++++++++---- > include/osv/dhcp.hh | 4 +++- > modules/cloud-init/cloud-init.cc | 10 +++++----- > 3 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/core/dhcp.cc b/core/dhcp.cc > index 0436837..b0e9f1e 100644 > --- a/core/dhcp.cc > +++ b/core/dhcp.cc > @@ -72,10 +72,9 @@ void dhcp_release() > net_dhcp_worker.release(); > } > > -void dhcp_restart(bool wait) > +void dhcp_renew() > { > - net_dhcp_worker.release(); > - net_dhcp_worker.start(wait); > + net_dhcp_worker.renew(); > } > > namespace dhcp { > @@ -221,6 +220,9 @@ namespace dhcp { > pkt->secs = 0; > pkt->flags = 0; > memcpy(pkt->chaddr, IF_LLADDR(ifp), ETHER_ADDR_LEN); > + ulong yip_n = htonl(yip.to_ulong()); > + ulong sip_n = htonl(sip.to_ulong()); > + memcpy(&pkt->ciaddr.s_addr, &yip_n, 4); > > // Options > u8* options_start = reinterpret_cast<u8*>(pkt+1); > @@ -242,7 +244,7 @@ namespace dhcp { > *options++ = DHCP_OPTION_END; > > dhcp_len += options - options_start; > - build_udp_ip_headers(dhcp_len, INADDR_ANY, INADDR_BROADCAST); > + build_udp_ip_headers(dhcp_len, yip_n, sip_n); > } > > void dhcp_mbuf::compose_release(struct ifnet* ifp, > @@ -541,6 +543,22 @@ namespace dhcp { > _client_addr = _server_addr = ipv4_zero; > } > > + void dhcp_interface_state::renew() > + { > + // Update state > + _state = DHCP_REQUEST; > + > + // Compose a dhcp request packet > + dhcp_mbuf dm(false); > + _xid = rand(); > + dm.compose_request(_ifp, > + _xid, > + _client_addr, _server_addr); > + > + // Send > + _sock->dhcp_send(dm); > + } > + > void dhcp_interface_state::process_packet(struct mbuf* m) > { > dhcp_mbuf dm(true, m); > @@ -724,6 +742,13 @@ namespace dhcp { > usleep(1000); > } > > + void dhcp_worker::renew() > + { > + for (auto &it: _universe) { > + it.second->renew(); > + } > + } > + > void dhcp_worker::dhcp_worker_fn() > { > while (true) { > diff --git a/include/osv/dhcp.hh b/include/osv/dhcp.hh > index b286727..166df48 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_restart(bool wait); > +void dhcp_renew(); > } > > namespace dhcp { > @@ -226,6 +226,7 @@ namespace dhcp { > > void discover(); > void release(); > + void renew(); > void process_packet(struct mbuf*); > void state_discover(dhcp_mbuf &dm); > void state_request(dhcp_mbuf &dm); > @@ -256,6 +257,7 @@ namespace dhcp { > void start(bool wait); > // Send release packet for all DHCP IPs. > void release(); > + void renew(); > > 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 e3ec8f8..f9b35de 100644 > --- a/modules/cloud-init/cloud-init.cc > +++ b/modules/cloud-init/cloud-init.cc > @@ -17,17 +17,17 @@ > #include <osv/hypervisor.hh> > > // we cannot include osv/dhcp.hh, hence direct declaration. > -extern "C" void dhcp_restart(bool wait); > +extern "C" void dhcp_renew(); > > // Set the hostname to given string. > // If hostname changes, try to propagate the change to DHCP server too. > -void set_hostname_restart_dhcp(std::string hostname) { > +void set_hostname_renew_dhcp(std::string hostname) { > if (hostname.length() > 0) { > char old_hostname[256] = ""; > gethostname(old_hostname, sizeof(old_hostname)); > sethostname(hostname.c_str(), hostname.length()); > if (hostname != old_hostname) { > - dhcp_restart(true); > + dhcp_renew(); > } > } > } > @@ -245,7 +245,7 @@ void hostname_module::handle(const YAML::Node& doc) > { > auto hostname = doc.as<string>(); > debug("cloudinit hostname: %s\n", hostname.c_str()); > - set_hostname_restart_dhcp(hostname); > + set_hostname_renew_dhcp(hostname); > } > > void osvinit::add_module(std::shared_ptr<config_module> module) > @@ -280,7 +280,7 @@ void osvinit::load_from_cloud(bool > ignore_missing_source) > auto& ds = get_data_source(); > > // Set the hostname from given data source, if it exists. > - set_hostname_restart_dhcp(ds.external_hostname()); > + set_hostname_renew_dhcp(ds.external_hostname()); > > // Load user data. > user_data = ds.get_user_data(); > -- > 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 [email protected]. > 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
