Thanks. Looks good to me. Justin, can you please also have a look?
-- Nadav Har'El [email protected] On Wed, Mar 22, 2017 at 2:57 PM, Waldemar Kozaczuk <[email protected]> wrote: > From: wkozaczuk <[email protected]> > > The patch addressing issue #816 changed dhcp_mbuf::compose_request() > method to > make it unicast DHCPREQUEST message to a DHCP server. This is incorrect > per RCF-2131 > which requires DHCP clients to broadcast messages until IP address is > bound which > is a case during RENEWING phase but not SELECTING one. Now the > dhcp_mbuf::compose_request() > takes extra enum parameter which specifies which phase (per page 34 of > https://www.ietf.org/rfc/rfc2131.txt) > the message is being composed for. > > Fixes #864 > > Signed-off-by: Waldemar Kozaczuk <[email protected]> > --- > core/dhcp.cc | 55 ++++++++++++++++++++++++++++++ > ++++++++++++++++------- > include/osv/dhcp.hh | 10 +++++++++- > 2 files changed, 57 insertions(+), 8 deletions(-) > > diff --git a/core/dhcp.cc b/core/dhcp.cc > index 1b90adb..12f27bb 100644 > --- a/core/dhcp.cc > +++ b/core/dhcp.cc > @@ -81,11 +81,26 @@ namespace dhcp { > > const char * dhcp_options_magic = "\x63\x82\x53\x63"; > > + static std::map<dhcp_message_type,const char*> > dhcp_message_type_name_by_id = { > + {DHCP_MT_DISCOVER, "DHCPDISCOVER"}, > + {DHCP_MT_OFFER, "DHCPOFFER"}, > + {DHCP_MT_REQUEST, "DHCPREQUEST"}, > + {DHCP_MT_DECLINE, "DHCPDECLINE"}, > + {DHCP_MT_ACK, "DHCPACK"}, > + {DHCP_MT_NAK, "DHCPNAK"}, > + {DHCP_MT_RELEASE, "DHCPRELEASE"}, > + {DHCP_MT_INFORM, "DHCPINFORM"}, > + {DHCP_MT_LEASEQUERY, "DHCPLEASEQUERY"}, > + {DHCP_MT_LEASEUNASSIGNED, "DHCPLEASEUNASSIGNED"}, > + {DHCP_MT_LEASEUNKNOWN, "DHCPLEASEUNKNOWN"}, > + {DHCP_MT_LEASEACTIVE, "DHCPLEASEACTIVE"}, > + {DHCP_MT_INVALID, "DHCPINVALID"} > + }; > + > //////////////////////////////////////////////////////////// > /////////////// > > bool dhcp_socket::dhcp_send(dhcp_mbuf& packet) > { > - > struct bsd_sockaddr dst = {}; > struct mbuf *m = packet.get(); > > @@ -205,7 +220,8 @@ namespace dhcp { > void dhcp_mbuf::compose_request(struct ifnet* ifp, > u32 xid, > ip::address_v4 yip, > - ip::address_v4 sip) > + ip::address_v4 sip, > + dhcp_request_packet_type > request_packet_type) > { > size_t dhcp_len = sizeof(struct dhcp_packet); > struct dhcp_packet* pkt = pdhcp(); > @@ -222,7 +238,11 @@ namespace dhcp { > 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); > + if(request_packet_type == DHCP_REQUEST_RENEWING || > request_packet_type == DHCP_REQUEST_REBINDING) { > + // ciaddr should only be set to if RENEWING or REBINDING > + // See pages 21 and 30-31 in https://www.ietf.org/rfc/ > rfc2131.txt > + memcpy(&pkt->ciaddr.s_addr, &yip_n, 4); > + } > > // Options > u8* options_start = reinterpret_cast<u8*>(pkt+1); > @@ -244,7 +264,14 @@ namespace dhcp { > *options++ = DHCP_OPTION_END; > > dhcp_len += options - options_start; > - build_udp_ip_headers(dhcp_len, yip_n, sip_n); > + > + // See page 33 in https://www.ietf.org/rfc/rfc2131.txt > + if(request_packet_type == DHCP_REQUEST_RENEWING) { > + build_udp_ip_headers(dhcp_len, yip_n, sip_n); > + } > + else { > + build_udp_ip_headers(dhcp_len, INADDR_ANY, INADDR_BROADCAST); > + } > } > > void dhcp_mbuf::compose_release(struct ifnet* ifp, > @@ -420,6 +447,9 @@ namespace dhcp { > options += op_len; > } > > + dhcp_i( "Received %s message from DHCP server: %s regarding > offerred IP address: %s", > + dhcp_message_type_name_by_id[_message_type], > _dhcp_server_ip.to_string().c_str(), > + _your_ip.to_string().c_str()); > return true; > } > > @@ -519,6 +549,7 @@ namespace dhcp { > // Save transaction id & send > _xid = dm.get_xid(); > _client_addr = _server_addr = ipv4_zero; > + dhcp_i( "Broadcasting DHCPDISCOVER message with xid: [%d]",_xid); > _sock->dhcp_send(dm); > } > > @@ -533,6 +564,8 @@ namespace dhcp { > > // Save transaction id & send > _xid = dm.get_xid(); > + dhcp_i( "Unicasting DHCPRELEASE message with xid: [%d] from > client: %s to server: %s", > + _xid, _client_addr.to_string().c_str(), > _server_addr.to_string().c_str()); > _sock->dhcp_send(dm); > // IP and routes have to be removed > osv::stop_if(_ifp->if_xname, _client_addr.to_string().c_str()); > @@ -553,9 +586,13 @@ namespace dhcp { > _xid = rand(); > dm.compose_request(_ifp, > _xid, > - _client_addr, _server_addr); > + _client_addr, > + _server_addr, > + 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()); > _sock->dhcp_send(dm); > } > > @@ -600,14 +637,18 @@ namespace dhcp { > dm_req.compose_request(_ifp, > _xid, > dm.get_your_ip(), > - dm.get_dhcp_server_ip()); > + dm.get_dhcp_server_ip(), > + 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()); > _sock->dhcp_send(dm_req); > } > > void dhcp_interface_state::state_request(dhcp_mbuf &dm) > { > if (dm.get_message_type() == DHCP_MT_ACK) { > - dhcp_i("Server acknowledged IP for interface %s", > _ifp->if_xname); > + dhcp_i("Server acknowledged IP %s for interface %s with time > to lease in seconds: %d", > + dm.get_your_ip().to_string().c_str(), _ifp->if_xname, > dm.get_lease_time_sec()); > _state = DHCP_ACKNOWLEDGE; > _client_addr = dm.get_your_ip(); > _server_addr = dm.get_dhcp_server_ip(); > diff --git a/include/osv/dhcp.hh b/include/osv/dhcp.hh > index 3db6df8..6797b7f 100644 > --- a/include/osv/dhcp.hh > +++ b/include/osv/dhcp.hh > @@ -117,6 +117,13 @@ namespace dhcp { > DHCP_REPLY = 2, > }; > > + enum dhcp_request_packet_type { > + DHCP_REQUEST_INIT_REBOOT = 1, > + DHCP_REQUEST_SELECTING = 2, > + DHCP_REQUEST_RENEWING = 3, > + DHCP_REQUEST_REBINDING = 4, > + }; > + > dhcp_mbuf(bool attached = true, struct mbuf* m = nullptr); > ~dhcp_mbuf(); > > @@ -129,7 +136,8 @@ namespace dhcp { > void compose_request(struct ifnet* ifp, > u32 xid, > boost::asio::ip::address_v4 yip, > - boost::asio::ip::address_v4 sip); > + boost::asio::ip::address_v4 sip, > + dhcp_request_packet_type > request_packet_type); > void compose_release(struct ifnet* ifp, > boost::asio::ip::address_v4 yip, > boost::asio::ip::address_v4 sip); > -- > 2.7.4 > > -- > 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.
