[3688] Moved copyDefaultFields to Dhcpv4Exchange class.

And avoid the excessive use of getQuery/getResponse.

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index a1d987c03e82268b314ac362e7596e95e6b4dbe0..27836633aa044854445c68c742e9f689e867b0b4 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -127,6 +127,55 @@ Dhcpv4Exchange::initResponse() {
     }
     if (resp_type > 0) {
         resp_.reset(new Pkt4(resp_type, getQuery()->getTransid()));
+        copyDefaultFields();
+    }
+}
+
+void
+Dhcpv4Exchange::copyDefaultFields() {
+    resp_->setIface(query_->getIface());
+    resp_->setIndex(query_->getIndex());
+
+    resp_->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS()); // explicitly set this to 0
+    // ciaddr is always 0, except for the Renew/Rebind state when it may
+    // be set to the ciaddr sent by the client.
+    resp_->setCiaddr(IOAddress::IPV4_ZERO_ADDRESS());
+    resp_->setHops(query_->getHops());
+
+    // copy MAC address
+    resp_->setHWAddr(query_->getHWAddr());
+
+    // relay address
+    resp_->setGiaddr(query_->getGiaddr());
+
+    // Let's copy client-id to response. See RFC6842.
+    // It is possible to disable RFC6842 to keep backward compatibility
+    bool echo = CfgMgr::instance().echoClientId();
+    OptionPtr client_id = query_->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+    if (client_id && echo) {
+        resp_->addOption(client_id);
+    }
+
+    // If src/dest HW addresses are used by the packet filtering class
+    // we need to copy them as well. There is a need to check that the
+    // address being set is not-NULL because an attempt to set the NULL
+    // HW would result in exception. If these values are not set, the
+    // the default HW addresses (zeroed) should be generated by the
+    // packet filtering class when creating Ethernet header for
+    // outgoing packet.
+    HWAddrPtr src_hw_addr = query_->getLocalHWAddr();
+    if (src_hw_addr) {
+        resp_->setLocalHWAddr(src_hw_addr);
+    }
+    HWAddrPtr dst_hw_addr = query_->getRemoteHWAddr();
+    if (dst_hw_addr) {
+        resp_->setRemoteHWAddr(dst_hw_addr);
+    }
+
+    // If this packet is relayed, we want to copy Relay Agent Info option
+    OptionPtr rai = query_->getOption(DHO_DHCP_AGENT_OPTIONS);
+    if (rai) {
+        resp_->addOption(rai);
     }
 }
 
@@ -603,54 +652,6 @@ Dhcpv4Srv::computeDhcid(const Lease4Ptr& lease) {
 
 }
 
-void
-Dhcpv4Srv::copyDefaultFields(Dhcpv4Exchange& ex) {
-    ex.getResponse()->setIface(ex.getQuery()->getIface());
-    ex.getResponse()->setIndex(ex.getQuery()->getIndex());
-
-    ex.getResponse()->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS()); // explicitly set this to 0
-    // ciaddr is always 0, except for the Renew/Rebind state when it may
-    // be set to the ciaddr sent by the client.
-    ex.getResponse()->setCiaddr(IOAddress::IPV4_ZERO_ADDRESS());
-    ex.getResponse()->setHops(ex.getQuery()->getHops());
-
-    // copy MAC address
-    ex.getResponse()->setHWAddr(ex.getQuery()->getHWAddr());
-
-    // relay address
-    ex.getResponse()->setGiaddr(ex.getQuery()->getGiaddr());
-
-    // Let's copy client-id to response. See RFC6842.
-    // It is possible to disable RFC6842 to keep backward compatibility
-    bool echo = CfgMgr::instance().echoClientId();
-    OptionPtr client_id = ex.getQuery()->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
-    if (client_id && echo) {
-        ex.getResponse()->addOption(client_id);
-    }
-
-    // If src/dest HW addresses are used by the packet filtering class
-    // we need to copy them as well. There is a need to check that the
-    // address being set is not-NULL because an attempt to set the NULL
-    // HW would result in exception. If these values are not set, the
-    // the default HW addresses (zeroed) should be generated by the
-    // packet filtering class when creating Ethernet header for
-    // outgoing packet.
-    HWAddrPtr src_hw_addr = ex.getQuery()->getLocalHWAddr();
-    if (src_hw_addr) {
-        ex.getResponse()->setLocalHWAddr(src_hw_addr);
-    }
-    HWAddrPtr dst_hw_addr = ex.getQuery()->getRemoteHWAddr();
-    if (dst_hw_addr) {
-        ex.getResponse()->setRemoteHWAddr(dst_hw_addr);
-    }
-
-    // If this packet is relayed, we want to copy Relay Agent Info option
-    OptionPtr rai = ex.getQuery()->getOption(DHO_DHCP_AGENT_OPTIONS);
-    if (rai) {
-        ex.getResponse()->addOption(rai);
-    }
-}
-
 void
 Dhcpv4Srv::appendDefaultOptions(Dhcpv4Exchange& ex) {
     // no-op at this time
@@ -680,26 +681,30 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) {
         return;
     }
 
+    Pkt4Ptr query = ex.getQuery();
+
     // try to get the 'Parameter Request List' option which holds the
     // codes of requested options.
     OptionUint8ArrayPtr option_prl = boost::dynamic_pointer_cast<
-        OptionUint8Array>(ex.getQuery()->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
+        OptionUint8Array>(query->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
     // If there is no PRL option in the message from the client then
     // there is nothing to do.
     if (!option_prl) {
         return;
     }
 
+    Pkt4Ptr resp = ex.getResponse();
+
     // Get the codes of requested options.
     const std::vector<uint8_t>& requested_opts = option_prl->getValues();
     // For each requested option code get the instance of the option
     // to be returned to the client.
     for (std::vector<uint8_t>::const_iterator opt = requested_opts.begin();
          opt != requested_opts.end(); ++opt) {
-        if (!ex.getResponse()->getOption(*opt)) {
+        if (!resp->getOption(*opt)) {
             OptionDescriptor desc = subnet->getCfgOption()->get("dhcp4", *opt);
             if (desc.option_) {
-                ex.getResponse()->addOption(desc.option_);
+                resp->addOption(desc.option_);
             }
         }
     }
@@ -780,16 +785,18 @@ Dhcpv4Srv::appendBasicOptions(Dhcpv4Exchange& ex) {
         return;
     }
 
+    Pkt4Ptr resp = ex.getResponse();
+
     // Try to find all 'required' options in the outgoing
     // message. Those that are not present will be added.
     for (int i = 0; i < required_options_size; ++i) {
-        OptionPtr opt = ex.getResponse()->getOption(required_options[i]);
+        OptionPtr opt = resp->getOption(required_options[i]);
         if (!opt) {
             // Check whether option has been configured.
             OptionDescriptor desc = subnet->getCfgOption()->
                 get("dhcp4", required_options[i]);
             if (desc.option_) {
-                ex.getResponse()->addOption(desc.option_);
+                resp->addOption(desc.option_);
             }
         }
     }
@@ -1453,7 +1460,6 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 
     Dhcpv4Exchange ex(alloc_engine_, discover, selectSubnet(discover));
 
-    copyDefaultFields(ex);
     appendDefaultOptions(ex);
 
     // If DHCPDISCOVER message contains the FQDN or Hostname option, server
@@ -1507,7 +1513,6 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 
     Dhcpv4Exchange ex(alloc_engine_, request, selectSubnet(request));
 
-    copyDefaultFields(ex);
     appendDefaultOptions(ex);
 
     // If DHCPREQUEST message contains the FQDN or Hostname option, server
@@ -1677,7 +1682,6 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
 
     Pkt4Ptr ack = ex.getResponse();
 
-    copyDefaultFields(ex);
     appendRequestedOptions(ex);
     appendRequestedVendorOptions(ex);
     appendBasicOptions(ex);

diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 78cc83b12b6d666943e4ce380b0f529d71acdfae..e14f3b8724dce9ec576e0f864dbf5d11dd71ee2f 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -110,6 +110,18 @@ public:
     }
 
 private:
+
+    /// @brief Copies default parameters from client's to server's message
+    ///
+    /// Some fields are copied from client's message into server's response,
+    /// e.g. client HW address, number of hops, transaction-id etc.
+    ///
+    /// @warning This message is called internally by @c initResponse and
+    /// thus it doesn't check if the resp_ value has been initialized. The
+    /// calling method is responsible for making sure that @c resp_ is
+    /// not NULL.
+    void copyDefaultFields();
+
     /// @brief Pointer to the allocation engine used by the server.
     AllocEnginePtr alloc_engine_;
     /// @brief Pointer to the DHCPv4 message sent by the client.
@@ -403,15 +415,6 @@ protected:
     /// @param inform message received from client
     Pkt4Ptr processInform(Pkt4Ptr& inform);
 
-    /// @brief Copies default parameters from client's to server's message
-    ///
-    /// Some fields are copied from client's message into server's response,
-    /// e.g. client HW address, number of hops, transaction-id etc.
-    ///
-    /// @param ex The exchange holding both the client's message and the
-    /// server's response.
-    void copyDefaultFields(Dhcpv4Exchange& ex);
-
     /// @brief Appends options requested by client.
     ///
     /// This method assigns options that were requested by client