[5132] Changes after review (comment #9):

 - new tests added
 - needed => required
 - spaced added where needed
 - CfgHosts::toElement6 updated
 -

src/bin/dhcp4/dhcp4_hooks.dox

@@ -118,7 +118,7 @@ to the end of this list.
  - @b Description: this callout is executed only if flexible identifiers are
    enabled, i.e. host-reservation-identifiers contain 'flex-id' value. This
    callout enables external library to provide values for flexible identifiers.
-   To be able to use this feature, flex_id hook library is needed.
+   To be able to use this feature, flex_id hook library is required.
 
  - <b>Next step status</b>: If a callout installed on the "host4_identifier" hook
    point sets the next step status to value other than NEXT_STEP_CONTINUE, the

src/bin/dhcp4/tests/hooks_unittest.cc

@@ -36,13 +36,13 @@ TEST_F(Dhcpv4SrvTest, Hooks) {
     NakedDhcpv4Srv srv(0);
 
     // check if appropriate hooks are registered
-    int hook_index_buffer4_receive = -1;
-    int hook_index_pkt4_receive    = -1;
-    int hook_index_select_subnet   = -1;
-    int hook_index_lease4_release  = -1;
-    int hook_index_pkt4_send       = -1;
-    int hook_index_buffer4_send    = -1;
-    int hook_index_host4_identifier= -1;
+    int hook_index_buffer4_receive  = -1;
+    int hook_index_pkt4_receive     = -1;
+    int hook_index_select_subnet    = -1;
+    int hook_index_lease4_release   = -1;
+    int hook_index_pkt4_send        = -1;
+    int hook_index_buffer4_send     = -1;
+    int hook_index_host4_identifier = -1;
 
     // check if appropriate indexes are set
     EXPECT_NO_THROW(hook_index_buffer4_receive = ServerHooks::getServerHooks()
@@ -555,16 +555,54 @@ public:
     host4_identifier_foo_callout(CalloutHandle& handle) {
         callback_name_ = string("host4_identifier");
 
-        std::vector<uint8_t> id(3);
-        id[0] = 0x66; // f
-        id[1] = 0x6f; // o
-        id[2] = 0x6f; // o
+        // Make sure the query4 parameter is passed.
+        handle.getArgument("query4", callback_qry_pkt4_);
+
+        // Make sure id_type parameter is passed.
+        Host::IdentifierType type = Host::IDENT_FLEX;
+        handle.getArgument("id_type", type);
+
+        // Make sure id_value parameter is passed.
+        std::vector<uint8_t> id_test;
+        handle.getArgument("id_value", id_test);
+
+        std::vector<uint8_t> id = { 0x66, 0x6f, 0x6f }; // foo
         handle.setArgument("id_value", id);
         handle.setArgument("id_type", Host::IDENT_FLEX);
 
         return (0);
     }
 
+    /// @brief Test host4_identifier callout by setting identifier to hwaddr
+    ///
+    /// This callout always returns fixed HWADDR: 00:01:02:03:04:05
+    ///
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    host4_identifier_hwaddr_callout(CalloutHandle& handle) {
+        callback_name_ = string("host4_identifier");
+
+        // Make sure the query4 parameter is passed.
+        handle.getArgument("query4", callback_qry_pkt4_);
+
+        // Make sure id_type parameter is passed.
+        Host::IdentifierType type = Host::IDENT_FLEX;
+        handle.getArgument("id_type", type);
+
+        // Make sure id_value parameter is passed.
+        std::vector<uint8_t> id_test;
+        handle.getArgument("id_value", id_test);
+
+        // Ok, now set the identifier to 00:01:02:03:04:05
+        std::vector<uint8_t> id = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+        handle.setArgument("id_value", id);
+        handle.setArgument("id_type", Host::IDENT_HWADDR);
+
+        return (0);
+    }
+
+
     /// resets buffers used to store data received by callouts
     void resetCalloutBuffers() {
         callback_name_ = string("");
@@ -1793,6 +1831,75 @@ TEST_F(HooksDhcpv4SrvTest, host4_identifier) {
     EXPECT_EQ("192.0.2.201", adv->getYiaddr().toText());
 }
 
+// Checks if callout installed on host4_identifier can generate identifier of
+// other type. This particular callout always returns hwaddr.
+TEST_F(HooksDhcpv4SrvTest, host4_identifier_hwaddr) {
+    IfaceMgrTestConfig test_config(true);
+    IfaceMgr::instance().openSockets4();
+
+    // Configure a subnet with host reservation. The reservation is based on
+    // flexible identifier value of 'foo'. That's exactly what the
+    // host4_identifier_foo_callout sets.
+    string config = "{ \"interfaces-config\": {"
+        "    \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"host-reservation-identifiers\": [ \"flex-id\" ], "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"192.0.2.0/25\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"interface\": \"eth0\", "
+        "    \"reservations\": ["
+        "        {"
+        "            \"hw-address\": \"00:01:02:03:04:05\","
+        "            \"ip-address\": \"192.0.2.201\""
+        "        }"
+        "    ]"
+        "} ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ConstElementPtr json;
+    EXPECT_NO_THROW(json = parseDHCP4(config));
+    ASSERT_TRUE(json);
+    ConstElementPtr status;
+
+    // Configure the server and make sure the config is accepted
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    comment_ = parseAnswer(rcode_, status);
+    ASSERT_EQ(0, rcode_);
+
+    CfgMgr::instance().commit();
+
+    // Install host4_identifier_hwaddr_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "host4_identifier", host4_identifier_hwaddr_callout));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = generateSimpleDiscover();
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // check that the server did send a response
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+
+    // Make sure that we received a response
+    Pkt4Ptr adv = srv_->fake_sent_.front();
+    ASSERT_TRUE(adv);
+
+    // Make sure the address offered is the one that was reserved.
+    EXPECT_EQ("192.0.2.201", adv->getYiaddr().toText());
+}
+
+
 // Verifies that libraries are unloaded by server destruction
 // The callout libraries write their library index number to a marker
 // file upon load and unload, making it simple to test whether or not

src/bin/dhcp6/tests/hooks_unittest.cc

@@ -53,17 +53,16 @@ TEST_F(Dhcpv6SrvTest, Hooks) {
     NakedDhcpv6Srv srv(0);
 
     // check if appropriate hooks are registered
-    int hook_index_buffer6_receive = -1;
-    int hook_index_buffer6_send    = -1;
-    int hook_index_lease6_renew    = -1;
-    int hook_index_lease6_release  = -1;
-    int hook_index_lease6_rebind   = -1;
-    int hook_index_lease6_decline  = -1;
-    int hook_index_pkt6_received   = -1;
-    int hook_index_select_subnet   = -1;
-    int hook_index_pkt6_send       = -1;
-    int hook_index_host6_identifier= -1;
-
+    int hook_index_buffer6_receive  = -1;
+    int hook_index_buffer6_send     = -1;
+    int hook_index_lease6_renew     = -1;
+    int hook_index_lease6_release   = -1;
+    int hook_index_lease6_rebind    = -1;
+    int hook_index_lease6_decline   = -1;
+    int hook_index_pkt6_received    = -1;
+    int hook_index_select_subnet    = -1;
+    int hook_index_pkt6_send        = -1;
+    int hook_index_host6_identifier = -1;
 
     // check if appropriate indexes are set
     EXPECT_NO_THROW(hook_index_buffer6_receive = ServerHooks::getServerHooks()
@@ -666,19 +665,46 @@ public:
         handle.getArgument("id_type", type);
 
         // Make sure id_value parameter is passed.
-        std::vector<uint8_t> id;
-        handle.getArgument("id_value", id);
+        std::vector<uint8_t> id_test;
+        handle.getArgument("id_value", id_test);
 
-        id.resize(3);
-        id[0] = 0x66; // f
-        id[1] = 0x6f; // o
-        id[2] = 0x6f; // o
+        // Ok, now set the identifier.
+        std::vector<uint8_t> id = { 0x66, 0x6f, 0x6f }; // foo
         handle.setArgument("id_value", id);
         handle.setArgument("id_type", Host::IDENT_FLEX);
 
         return (0);
     }
 
+    /// @brief Test host4_identifier callout by setting identifier to hwaddr
+    ///
+    /// This callout always returns fixed HWADDR: 00:01:02:03:04:05
+    ///
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    host6_identifier_hwaddr_callout(CalloutHandle& handle) {
+        callback_name_ = string("host6_identifier");
+
+        // Make sure the query6 parameter is passed.
+        handle.getArgument("query6", callback_qry_pkt6_);
+
+        // Make sure id_type parameter is passed.
+        Host::IdentifierType type = Host::IDENT_FLEX;
+        handle.getArgument("id_type", type);
+
+        // Make sure id_value parameter is passed.
+        std::vector<uint8_t> id_test;
+        handle.getArgument("id_value", id_test);
+
+        // Ok, now set the identifier to 00:01:02:03:04:05
+        std::vector<uint8_t> id = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+        handle.setArgument("id_value", id);
+        handle.setArgument("id_type", Host::IDENT_HWADDR);
+
+        return (0);
+    }
+
 
     /// Resets buffers used to store data received by callouts
     void resetCalloutBuffers() {
@@ -2264,7 +2290,6 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineDrop) {
     EXPECT_EQ(Lease::STATE_DEFAULT, from_mgr->state_);
 }
 
-
 // Checks if callout installed on host6_identifier can generate an
 // identifier and whether that identifier is actually used.
 TEST_F(HooksDhcpv6SrvTest, host6Identifier) {
@@ -2338,6 +2363,79 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier) {
     ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText());
 }
 
+// Checks if callout installed on host6_identifier can generate an identifier
+// other type. This particular callout always returns hwaddr.
+TEST_F(HooksDhcpv6SrvTest, host6Identifier_hwaddr) {
+
+    // Configure 2 subnets, both directly reachable over local interface
+    // (let's not complicate the matter with relays)
+    string config = "{ \"interfaces-config\": {\n"
+        "  \"interfaces\": [ \"*\" ]\n"
+        "},\n"
+        "\"preferred-lifetime\": 3000,\n"
+        "\"rebind-timer\": 2000,\n"
+        "\"renew-timer\": 1000,\n"
+        "\"host-reservation-identifiers\": [ \"flex-id\" ],\n"
+        "\"subnet6\": [ {\n"
+        "    \"pools\": [ { \"pool\": \"2001:db8::/64\" } ],\n"
+        "    \"subnet\": \"2001:db8::/48\", \n"
+        "    \"interface\": \"" + valid_iface_ + "\",\n"
+        "    \"reservations\": [\n"
+        "        {\n"
+        "            \"hw-address\": \"00:01:02:03:04:05\",\n"
+        "            \"ip-addresses\": [ \"2001:db8::f00\" ]\n"
+        "        }\n"
+        "    ]\n"
+        " } ]\n,"
+        "\"valid-lifetime\": 4000 }";
+
+    ConstElementPtr json;
+    EXPECT_NO_THROW(json = parseDHCP6(config));
+    ConstElementPtr status;
+
+    // Configure the server and make sure the config is accepted
+    EXPECT_NO_THROW(status = configureDhcp6Server(*srv_, json));
+    ASSERT_TRUE(status);
+    comment_ = isc::config::parseAnswer(rcode_, status);
+    ASSERT_EQ(0, rcode_);
+
+    CfgMgr::instance().commit();
+
+    // Install host6_identifier_foo_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "host6_identifier", host6_identifier_hwaddr_callout));
+
+    // Prepare solicit packet. Server should select first subnet for it
+    Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
+    sol->setRemoteAddr(IOAddress("fe80::abcd"));
+    sol->setIface(valid_iface_);
+    sol->addOption(generateIA(D6O_IA_NA, 234, 1500, 3000));
+    OptionPtr clientid = generateClientId();
+    sol->addOption(clientid);
+
+    // Pass it to the server and get an advertise
+    Pkt6Ptr adv = srv_->processSolicit(sol);
+
+    // Check if we get response at all
+    ASSERT_TRUE(adv);
+
+    // Check that the callback called is indeed the one we installed
+    EXPECT_EQ("host6_identifier", callback_name_);
+
+    // Check that pkt6 argument passing was successful and returned proper value
+    EXPECT_TRUE(callback_qry_pkt6_.get() == sol.get());
+
+    // Now check if we got the reserved address
+    OptionPtr tmp = adv->getOption(D6O_IA_NA);
+    ASSERT_TRUE(tmp);
+
+    // Check that IA_NA was returned and that there's an address included
+    boost::shared_ptr<Option6IAAddr> addr_opt = checkIA_NA(adv, 234, 1000, 2000);
+
+    ASSERT_TRUE(addr_opt);
+    ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText());
+}
+
 
 // Verifies that libraries are unloaded by server destruction
 // The callout libraries write their library index number to a marker

src/lib/dhcpsrv/cfg_hosts.cc

@@ -775,6 +775,10 @@ CfgHosts::toElement6() const {
             isc_throw(ToElementError, "unexpected circuit-id DUID type");
         } else if (id_type == Host::IDENT_CLIENT_ID) {
             isc_throw(ToElementError, "unexpected client-id DUID type");
+        } else if (id_type == Host::IDENT_FLEX) {
+            const std::vector<uint8_t>& bin = (*host)->getIdentifier();
+            std::string flex = util::encode::encodeHex(bin);
+            map->set("flex-id", Element::create(flex));
         } else {
             isc_throw(ToElementError, "invalid DUID type: " << id_type);
         }