Browse Source

[4110a] Addressed comments

Francis Dupont 9 years ago
parent
commit
4a9030245f

+ 4 - 4
src/bin/dhcp4/dhcp4_messages.mes

@@ -205,11 +205,11 @@ A malformed DHCPv4o6 packet was received.
 This debug message is printed when the server is receiving a DHCPv4o6
 from the DHCPv6 server over inter-process communication.
 
-% DHCP4_DHCP4O6_PACKET_SEND %1: trying to send packet %2 (type %3) to %4 on interface %5 encapsulating %6 %7 (type %8)
+% DHCP4_DHCP4O6_PACKET_SEND %1: trying to send packet %2 (type %3) to %4 on interface %5 encapsulating %6: %7 (type %8)
 The arguments specify the client identification information (HW address
-and client identifier), DHCP message name and type, source IPv4
-address and port, destination IPv4 address and port and the
-interface name.
+and client identifier), DHCPv6 message name and type, source IPv6
+address and interface name, DHCPv4 client identification, message
+name and type.
 
 % DHCP4_DHCP4O6_PACKET_SEND_FAIL %1: failed to send DHCPv4o6 packet: %2
 This error is output if the IPv4 DHCP server fails to send an

+ 84 - 70
src/bin/dhcp4/dhcp4_srv.cc

@@ -70,6 +70,34 @@ using namespace isc::log;
 using namespace isc::stats;
 using namespace std;
 
+/// Structure that holds registered hook indexes
+struct Dhcp4Hooks {
+    int hook_index_buffer4_receive_;///< index for "buffer4_receive" hook point
+    int hook_index_pkt4_receive_;   ///< index for "pkt4_receive" hook point
+    int hook_index_subnet4_select_; ///< index for "subnet4_select" hook point
+    int hook_index_lease4_release_; ///< index for "lease4_release" hook point
+    int hook_index_pkt4_send_;      ///< index for "pkt4_send" hook point
+    int hook_index_buffer4_send_;   ///< index for "buffer4_send" hook point
+    int hook_index_lease4_decline_; ///< index for "lease4_decline" hook point
+
+    /// Constructor that registers hook points for DHCPv4 engine
+    Dhcp4Hooks() {
+        hook_index_buffer4_receive_= HooksManager::registerHook("buffer4_receive");
+        hook_index_pkt4_receive_   = HooksManager::registerHook("pkt4_receive");
+        hook_index_subnet4_select_ = HooksManager::registerHook("subnet4_select");
+        hook_index_pkt4_send_      = HooksManager::registerHook("pkt4_send");
+        hook_index_lease4_release_ = HooksManager::registerHook("lease4_release");
+        hook_index_buffer4_send_   = HooksManager::registerHook("buffer4_send");
+        hook_index_lease4_decline_ = HooksManager::registerHook("lease4_decline");
+    }
+};
+
+// Declare a Hooks object. As this is outside any function or method, it
+// will be instantiated (and the constructor run) when the module is loaded.
+// As a result, the hook indexes will be defined before any method in this
+// module is called.
+Dhcp4Hooks Hooks;
+
 namespace isc {
 namespace dhcp {
 
@@ -302,30 +330,13 @@ Dhcpv4Exchange::setHostIdentifiers() {
     }
 }
 
-// Static values so instantiated when the module is loaded.
-// As a result, the hook indexes will be defined before any method in this
-// module is called.
-
-int Dhcpv4Srv::hook_index_buffer4_receive_=
-    HooksManager::registerHook("buffer4_receive");
-int Dhcpv4Srv::hook_index_pkt4_receive_ =
-    HooksManager::registerHook("pkt4_receive");
-int Dhcpv4Srv::hook_index_subnet4_select_ =
-    HooksManager::registerHook("subnet4_select");
-int Dhcpv4Srv::hook_index_pkt4_send_ =
-    HooksManager::registerHook("pkt4_send");
-int Dhcpv4Srv::hook_index_lease4_release_ =
-    HooksManager::registerHook("lease4_release");
-int Dhcpv4Srv::hook_index_buffer4_send_ =
-    HooksManager::registerHook("buffer4_send");
-int Dhcpv4Srv::hook_index_lease4_decline_ =
-    HooksManager::registerHook("lease4_decline");
-
 const std::string Dhcpv4Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
 
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const bool use_bcast,
                      const bool direct_response_desired)
-    : shutdown_(true), alloc_engine_(), port_(port), use_bcast_(use_bcast) {
+    : shutdown_(true), alloc_engine_(), port_(port),
+      use_bcast_(use_bcast), hook_index_pkt4_receive_(-1),
+      hook_index_subnet4_select_(-1), hook_index_pkt4_send_(-1) {
 
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
@@ -349,6 +360,11 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const bool use_bcast,
         alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 0,
                                             false /* false = IPv4 */));
 
+        // Register hook points
+        hook_index_pkt4_receive_   = Hooks.hook_index_pkt4_receive_;
+        hook_index_subnet4_select_ = Hooks.hook_index_subnet4_select_;
+        hook_index_pkt4_send_      = Hooks.hook_index_pkt4_send_;
+
         /// @todo call loadLibraries() when handling configuration changes
 
     } catch (const std::exception &e) {
@@ -503,7 +519,6 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query) const {
 
 isc::dhcp::Subnet4Ptr
 Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query) const {
-    // Here begin by the same thing than selectSubnet, i.e., the DHCPv4 part
 
     Subnet4Ptr subnet;
 
@@ -512,47 +527,9 @@ Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query) const {
     selector.giaddr_ = query->getGiaddr();
     selector.local_address_ = query->getLocalAddr();
     selector.client_classes_ = query->classes_;
-    // moved selector.remote_address_ as it is IPv6
     selector.iface_name_ = query->getIface();
-
-    // If the link-selection sub-option is present, extract its value.
-    // "The link-selection sub-option is used by any DHCP relay agent
-    // that desires to specify a subnet/link for a DHCP client request
-    // that it is relaying but needs the subnet/link specification to
-    // be different from the IP address the DHCP server should use
-    // when communicating with the relay agent." (RFC 3257)
-    //
-    // Try first Relay Agent Link Selection sub-option
-    OptionPtr rai = query->getOption(DHO_DHCP_AGENT_OPTIONS);
-    if (rai) {
-        OptionCustomPtr rai_custom =
-            boost::dynamic_pointer_cast<OptionCustom>(rai);
-        if (rai_custom) {
-            OptionPtr link_select =
-                rai_custom->getOption(RAI_OPTION_LINK_SELECTION);
-            if (link_select) {
-                OptionBuffer link_select_buf = link_select->getData();
-                if (link_select_buf.size() == sizeof(uint32_t)) {
-                    selector.option_select_ =
-                        IOAddress::fromBytes(AF_INET, &link_select_buf[0]);
-                }
-            }
-        }
-    } else {
-        // Or Subnet Selection option
-        OptionPtr sbnsel = query->getOption(DHO_SUBNET_SELECTION);
-        if (sbnsel) {
-            OptionCustomPtr oc =
-                boost::dynamic_pointer_cast<OptionCustom>(sbnsel);
-            if (oc) {
-                selector.option_select_ = oc->readAddress();
-            }
-        }
-    }
-
     // Mark it as DHCPv4-over-DHCPv6
     selector.dhcp4o6_ = true;
-
     // Now the DHCPv6 part
     selector.remote_address_ = query->getRemoteAddr();
     selector.first_relay_linkaddr_ = IOAddress("::");
@@ -578,8 +555,17 @@ Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query) const {
                                       Pkt6::RELAY_GET_FIRST);
     }
 
+    // If the Subnet Selection option is present, extract its value.
+    OptionPtr sbnsel = query->getOption(DHO_SUBNET_SELECTION);
+    if (sbnsel) {
+        OptionCustomPtr oc = boost::dynamic_pointer_cast<OptionCustom>(sbnsel);
+        if (oc) {
+            selector.option_select_ = oc->readAddress();
+        }
+    }
+
     CfgMgr& cfgmgr = CfgMgr::instance();
-    subnet = cfgmgr.getCurrentCfg()->getCfgSubnets4()->selectSubnet(selector);
+    subnet = cfgmgr.getCurrentCfg()->getCfgSubnets4()->selectSubnet4o6(selector);
 
     // Let's execute all callouts registered for subnet4_select
     if (HooksManager::calloutsPresent(hook_index_subnet4_select_)) {
@@ -746,7 +732,7 @@ Dhcpv4Srv::run_one() {
         // Option objects modification does not make sense anymore. Hooks
         // can only manipulate wire buffer at this stage.
         // Let's execute all callouts registered for buffer4_send
-        if (HooksManager::calloutsPresent(hook_index_buffer4_send_)) {
+        if (HooksManager::calloutsPresent(Hooks.hook_index_buffer4_send_)) {
             CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
             // Delete previously set arguments
@@ -756,7 +742,7 @@ Dhcpv4Srv::run_one() {
             callout_handle->setArgument("response4", rsp);
 
             // Call callouts
-            HooksManager::callCallouts(hook_index_buffer4_send_,
+            HooksManager::callCallouts(Hooks.hook_index_buffer4_send_,
                                        *callout_handle);
 
             // Callouts decided to skip the next processing step. The next
@@ -815,7 +801,7 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) {
 
     // The packet has just been received so contains the uninterpreted wire
     // data; execute callouts registered for buffer4_receive.
-    if (HooksManager::calloutsPresent(hook_index_buffer4_receive_)) {
+    if (HooksManager::calloutsPresent(Hooks.hook_index_buffer4_receive_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
         // Delete previously set arguments
@@ -825,7 +811,7 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) {
         callout_handle->setArgument("query4", query);
 
         // Call callouts
-        HooksManager::callCallouts(hook_index_buffer4_receive_,
+        HooksManager::callCallouts(Hooks.hook_index_buffer4_receive_,
                                    *callout_handle);
 
         // Callouts decided to skip the next processing step. The next
@@ -905,7 +891,7 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) {
         .arg(query->toText());
 
     // Let's execute all callouts registered for pkt4_receive
-    if (HooksManager::calloutsPresent(hook_index_pkt4_receive_)) {
+    if (HooksManager::calloutsPresent(Hooks.hook_index_pkt4_receive_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
         // Delete previously set arguments
@@ -915,7 +901,7 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) {
         callout_handle->setArgument("query4", query);
 
         // Call callouts
-        HooksManager::callCallouts(hook_index_pkt4_receive_,
+        HooksManager::callCallouts(Hooks.hook_index_pkt4_receive_,
                                    *callout_handle);
 
         // Callouts decided to skip the next processing step. The next
@@ -2075,7 +2061,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
         bool skip = false;
 
         // Execute all callouts registered for lease4_release
-        if (HooksManager::calloutsPresent(hook_index_lease4_release_)) {
+        if (HooksManager::calloutsPresent(Hooks.hook_index_lease4_release_)) {
             CalloutHandlePtr callout_handle = getCalloutHandle(release);
 
             // Delete all previous arguments
@@ -2088,7 +2074,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
             callout_handle->setArgument("lease4", lease);
 
             // Call all installed callouts
-            HooksManager::callCallouts(hook_index_lease4_release_,
+            HooksManager::callCallouts(Hooks.hook_index_lease4_release_,
                                        *callout_handle);
 
             // Callouts decided to skip the next processing step. The next
@@ -2218,7 +2204,7 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline) {
     // Let's check if there are hooks installed for decline4 hook point.
     // If they are, let's pass the lease and client's packet. If the hook
     // sets status to drop, we reject this Decline.
-    if (HooksManager::calloutsPresent(hook_index_lease4_decline_)) {
+    if (HooksManager::calloutsPresent(Hooks.hook_index_lease4_decline_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(decline);
 
         // Delete previously set arguments
@@ -2229,7 +2215,7 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline) {
         callout_handle->setArgument("query4", decline);
 
         // Call callouts
-        HooksManager::callCallouts(hook_index_lease4_decline_,
+        HooksManager::callCallouts(Hooks.hook_index_lease4_decline_,
                                    *callout_handle);
 
         // Check if callouts decided to drop the packet. If any of them did,
@@ -2788,5 +2774,33 @@ void Dhcpv4Srv::processStatsSent(const Pkt4Ptr& response) {
                                               static_cast<int64_t>(1));
 }
 
+int Dhcpv4Srv::getHookIndexBuffer4Receive() {
+    return (Hooks.hook_index_buffer4_receive_);
+}
+
+int Dhcpv4Srv::getHookIndexPkt4Receive() {
+    return (Hooks.hook_index_pkt4_receive_);
+}
+
+int Dhcpv4Srv::getHookIndexSubnet4Select() {
+    return (Hooks.hook_index_subnet4_select_);
+}
+
+int Dhcpv4Srv::getHookIndexLease4Release() {
+    return (Hooks.hook_index_lease4_release_);
+}
+
+int Dhcpv4Srv::getHookIndexPkt4Send() {
+    return (Hooks.hook_index_pkt4_send_);
+}
+
+int Dhcpv4Srv::getHookIndexBuffer4Send() {
+    return (Hooks.hook_index_buffer4_send_);
+}
+
+int Dhcpv4Srv::getHookIndexLease4Decline() {
+    return (Hooks.hook_index_lease4_decline_);
+}
+
 }   // namespace dhcp
 }   // namespace isc

+ 27 - 19
src/bin/dhcp4/dhcp4_srv.h

@@ -9,8 +9,6 @@
 
 #include <dhcp/dhcp4.h>
 #include <dhcp/pkt4.h>
-#include <dhcp/pkt4o6.h>
-#include <dhcp/pkt6.h>
 #include <dhcp/option.h>
 #include <dhcp/option_string.h>
 #include <dhcp/option4_client_fqdn.h>
@@ -793,8 +791,13 @@ private:
     uint16_t port_;  ///< UDP port number on which server listens.
     bool use_bcast_; ///< Should broadcast be enabled on sockets (if true).
 
+    /// Indexes for registered hook points
+    int hook_index_pkt4_receive_;
+    int hook_index_subnet4_select_;
+    int hook_index_pkt4_send_;
+
 public:
-    /// Class methods and variables for DHCPv4-over-DHCPv6 handler
+    /// Class methods for DHCPv4-over-DHCPv6 handler
 
     /// @brief Updates statistics for received packets
     /// @param query packet received
@@ -804,28 +807,33 @@ public:
     /// @param response packet transmitted
     static void processStatsSent(const Pkt4Ptr& response);
 
-    /// Indexes for registered hook points
-
-    /// @brief index for "buffer4_receive" hook point
-    static int hook_index_buffer4_receive_;
+    /// @brief Returns the index for "buffer4_receive" hook point
+    /// @return the index for "buffer4_receive" hook point
+    static int getHookIndexBuffer4Receive();
 
-    /// @brief index for "pkt4_receive" hook point
-    static int hook_index_pkt4_receive_;
+    /// @brief Returns the index for "pkt4_receive" hook point
+    /// @return the index for "pkt4_receive" hook point
+    static int getHookIndexPkt4Receive();
 
-    /// @brief index for "subnet4_select" hook point
-    static int hook_index_subnet4_select_;
+    /// @brief Returns the index for "subnet4_select" hook point
+    /// @return the index for "subnet4_select" hook point
+    static int getHookIndexSubnet4Select();
 
-    /// @brief index for "lease4_release" hook point
-    static int hook_index_lease4_release_;
+    /// @brief Returns the index for "lease4_release" hook point
+    /// @return the index for "lease4_release" hook point
+    static int getHookIndexLease4Release();
 
-    /// @brief index for "pkt4_send" hook point
-    static int hook_index_pkt4_send_;
+    /// @brief Returns the index for "pkt4_send" hook point
+    /// @return the index for "pkt4_send" hook point
+    static int getHookIndexPkt4Send();
 
-    /// @brief index for "buffer4_send" hook point
-    static int hook_index_buffer4_send_;
+    /// @brief Returns the index for "buffer4_send" hook point
+    /// @return the index for "buffer4_send" hook point
+    static int getHookIndexBuffer4Send();
 
-    /// @brief index for "lease4_decline" hook point
-    static int hook_index_lease4_decline_;
+    /// @brief Returns the index for "lease4_decline" hook point
+    /// @return the index for "lease4_decline" hook point
+    static int getHookIndexLease4Decline();
 };
 
 }; // namespace isc::dhcp

+ 2 - 2
src/bin/dhcp4/dhcp4to6_ipc.cc

@@ -115,7 +115,7 @@ void Dhcp4to6Ipc::handler() {
         // Option objects modification does not make sense anymore. Hooks
         // can only manipulate wire buffer at this stage.
         // Let's execute all callouts registered for buffer4_send
-        if (HooksManager::calloutsPresent(Dhcpv4Srv::hook_index_buffer4_send_)) {
+      if (HooksManager::calloutsPresent(Dhcpv4Srv::getHookIndexBuffer4Send())) {
             CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
             // Delete previously set arguments
@@ -125,7 +125,7 @@ void Dhcp4to6Ipc::handler() {
             callout_handle->setArgument("response4", rsp);
 
             // Call callouts
-            HooksManager::callCallouts(Dhcpv4Srv::hook_index_buffer4_send_,
+            HooksManager::callCallouts(Dhcpv4Srv::getHookIndexBuffer4Send(),
                                        *callout_handle);
 
             // Callouts decided to skip the next processing step. The next

+ 25 - 22
src/bin/dhcp4/tests/dhcp4to6_ipc_unittest.cc

@@ -15,6 +15,7 @@
 #include <dhcp4/tests/dhcp4_test_utils.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/testutils/dhcp4o6_test_ipc.h>
+#include <stats/stats_mgr.h>
 #include <hooks/callout_handle.h>
 #include <hooks/hooks_manager.h>
 
@@ -25,6 +26,7 @@ using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
+using namespace isc::stats;
 using namespace isc::hooks;
 using namespace isc::util;
 
@@ -56,6 +58,12 @@ public:
         // Install buffer4_send_callout
         EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().
                         registerCallout("buffer4_send", buffer4_send_callout));
+        // Verify we have a controlled server
+        ControlledDhcpv4Srv* srv = NULL;
+        EXPECT_NO_THROW(srv = ControlledDhcpv4Srv::getInstance());
+        EXPECT_TRUE(srv);
+        // Let's wipe all existing statistics.
+        StatsMgr::instance().removeAll();
     }
 
     /// @brief Configure DHCP4o6 port.
@@ -105,7 +113,7 @@ public:
     ///
     /// Dhcp4to6Ipc::handler() uses the instance of the controlled server
     /// so it has to be build. This reference does this.
-    ControlledDhcpv4Srv srv;
+    ControlledDhcpv4Srv srv_;
 
 private:
 
@@ -149,11 +157,6 @@ TEST_F(Dhcp4to6IpcTest, invalidPortError) {
 // This test verifies that the DHCPv4 endpoint of the DHCPv4o6 IPC can
 // receive messages.
 TEST_F(Dhcp4to6IpcTest, receive) {
-    // Verify we have a controlled server
-    ControlledDhcpv4Srv* srv = NULL;
-    ASSERT_NO_THROW(srv = ControlledDhcpv4Srv::getInstance());
-    ASSERT_TRUE(srv);
-
     // Create instance of the IPC endpoint under test.
     Dhcp4to6Ipc& ipc = Dhcp4to6Ipc::instance();
     // Create instance of the IPC endpoint being used as a source of messages.
@@ -194,11 +197,6 @@ TEST_F(Dhcp4to6IpcTest, receive) {
 // This test verifies that message with multiple DHCPv4 query options
 // is rejected.
 TEST_F(Dhcp4to6IpcTest, receiveMultipleQueries) {
-    // Verify we have a controlled server
-    ControlledDhcpv4Srv* srv = NULL;
-    ASSERT_NO_THROW(srv = ControlledDhcpv4Srv::getInstance());
-    ASSERT_TRUE(srv);
-
     // Create instance of the IPC endpoint under test.
     Dhcp4to6Ipc& ipc = Dhcp4to6Ipc::instance();
     // Create instance of the IPC endpoint being used as a source of messages.
@@ -231,11 +229,6 @@ TEST_F(Dhcp4to6IpcTest, receiveMultipleQueries) {
 
 // This test verifies that message with no DHCPv4 query options is rejected.
 TEST_F(Dhcp4to6IpcTest, receiveNoQueries) {
-    // Verify we have a controlled server
-    ControlledDhcpv4Srv* srv = NULL;
-    ASSERT_NO_THROW(srv = ControlledDhcpv4Srv::getInstance());
-    ASSERT_TRUE(srv);
-
     // Create instance of the IPC endpoint under test.
     Dhcp4to6Ipc& ipc = Dhcp4to6Ipc::instance();
     // Create instance of the IPC endpoint being used as a source of messages.
@@ -266,11 +259,6 @@ TEST_F(Dhcp4to6IpcTest, receiveNoQueries) {
 // This test verifies that the DHCPv4 endpoint of the DHCPv4o6 IPC can
 // process messages.
 TEST_F(Dhcp4to6IpcTest, process) {
-    // Verify we have a controlled server
-    ControlledDhcpv4Srv* srv = NULL;
-    ASSERT_NO_THROW(srv = ControlledDhcpv4Srv::getInstance());
-    ASSERT_TRUE(srv);
-
     // Create instance of the IPC endpoint under test.
     Dhcp4to6Ipc& ipc = Dhcp4to6Ipc::instance();
     // Create instance of the IPC endpoint being used as a source of messages.
@@ -280,6 +268,13 @@ TEST_F(Dhcp4to6IpcTest, process) {
     ASSERT_NO_THROW(ipc.open());
     ASSERT_NO_THROW(src_ipc.open());
 
+    // Get statistics
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_snd = mgr.getObservation("pkt4-sent");
+    ObservationPtr pkt4_ack = mgr.getObservation("pkt4-ack-sent");
+    EXPECT_FALSE(pkt4_snd);
+    EXPECT_FALSE(pkt4_ack);
+
     // Create an information request message
     Pkt4Ptr infreq(new Pkt4(DHCPINFORM, 1234));
     infreq->setHWAddr(generateHWAddr(6));
@@ -331,7 +326,7 @@ TEST_F(Dhcp4to6IpcTest, process) {
     EXPECT_EQ("eth0", pkt6_sent->getIface());
     EXPECT_EQ("2001:db8:1::123", pkt6_sent->getRemoteAddr().toText());
 
-    // Check the 4o6 part
+    // Verify the 4o6 part
     OptionCollection sent_msgs = pkt6_sent->getOptions(D6O_DHCPV4_MSG);
     ASSERT_EQ(1, sent_msgs.size());
     OptionPtr sent_msg = sent_msgs.begin()->second;
@@ -342,6 +337,14 @@ TEST_F(Dhcp4to6IpcTest, process) {
     ASSERT_NO_THROW(pkt4_opt->unpack());
     EXPECT_EQ(DHCPACK, pkt4_sent->getType());
     EXPECT_EQ(pkt4_sent->len(), pkt4_opt->len());
+
+    // Verify statistics
+    pkt4_snd = mgr.getObservation("pkt4-sent");
+    pkt4_ack = mgr.getObservation("pkt4-ack-sent");
+    ASSERT_TRUE(pkt4_snd);
+    ASSERT_TRUE(pkt4_ack);
+    EXPECT_EQ(1, pkt4_snd->getInteger().first);
+    EXPECT_EQ(1, pkt4_ack->getInteger().first);
 }
 
 } // end of anonymous namespace

+ 0 - 4
src/lib/dhcpsrv/cfg_subnets4.cc

@@ -76,10 +76,6 @@ CfgSubnets4::selectSubnet4o6(const SubnetSelector& selector) const {
 Subnet4Ptr
 CfgSubnets4::selectSubnet(const SubnetSelector& selector) const {
 
-    if (selector.dhcp4o6_) {
-        return selectSubnet4o6(selector);
-    }
-
     // First use RAI link select sub-option or subnet select option
     if (!selector.option_select_.isV4Zero()) {
         return (selectSubnet(selector.option_select_,

+ 4 - 2
src/lib/dhcpsrv/dhcp4o6_ipc.cc

@@ -166,7 +166,8 @@ Pkt6Ptr Dhcp4o6IpcBase::receive() {
         LOG_WARN(dhcpsrv_logger, DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET)
             .arg("no interface suboption");
         isc_throw(Dhcp4o6IpcError,
-                  "malformed packet (no interface suboption)");
+                  "malformed packet (interface suboption missing "
+                  "or has incorrect type)");
     }
 
     // Check if this interface is present in the system.
@@ -186,7 +187,8 @@ Pkt6Ptr Dhcp4o6IpcBase::receive() {
         LOG_WARN(dhcpsrv_logger, DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET)
             .arg("no source address suboption");
         isc_throw(Dhcp4o6IpcError,
-                  "malformed packet (o source address suboption)");
+                  "malformed packet (source address suboption missing "
+                  "or has incorrect type)");
     }
 
     // Update the packet.

+ 5 - 5
src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc

@@ -375,7 +375,7 @@ TEST(CfgSubnets4Test, 4o6subnetMatchByAddress) {
     selector.dhcp4o6_ = true;
     selector.remote_address_ = IOAddress("2001:db8:1::dead:beef");
 
-    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet4o6(selector));
 }
 
 // This test checks if the IPv4 subnet can be selected based on the value of
@@ -405,11 +405,11 @@ TEST(CfgSubnets4Test, 4o6subnetMatchByInterfaceId) {
     selector.dhcp4o6_ = true;
     selector.interface_id_ = interfaceId2;
     // We have mismatched interface-id options (data1 vs data2). Should not match.
-    EXPECT_FALSE(cfg.selectSubnet(selector));
+    EXPECT_FALSE(cfg.selectSubnet4o6(selector));
 
     // This time we have correct interface-id. Should match.
     selector.interface_id_ = interfaceId1;
-    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet4o6(selector));
 }
 
 // This test checks if the IPv4 subnet can be selected based on the value of
@@ -431,11 +431,11 @@ TEST(CfgSubnets4Test, 4o6subnetMatchByInterfaceName) {
     selector.dhcp4o6_ = true;
     selector.iface_name_ = "eth5";
     // We have mismatched interface names. Should not match.
-    EXPECT_FALSE(cfg.selectSubnet(selector));
+    EXPECT_FALSE(cfg.selectSubnet4o6(selector));
 
     // This time we have correct names. Should match.
     selector.iface_name_ = "eth7";
-    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
+    EXPECT_EQ(subnet2, cfg.selectSubnet4o6(selector));
 }