[3232] Addressed review comments.

src/bin/dhcp6/tests/dhcp6_client.cc

@@ -21,6 +21,7 @@
 #include <dhcp6/tests/dhcp6_client.h>
 #include <util/buffer.h>
 #include <boost/pointer_cast.hpp>
+#include <cstdlib>
 #include <time.h>
 
 using namespace isc::test;
@@ -41,63 +42,99 @@ Dhcp6Client::Dhcp6Client() :
     use_relay_(false) {
 }
 
+Dhcp6Client::Dhcp6Client(boost::shared_ptr<NakedDhcpv6Srv>& srv) :
+    relay_link_addr_("3000:1::1"),
+    curr_transid_(0),
+    dest_addr_(ALL_DHCP_RELAY_AGENTS_AND_SERVERS),
+    duid_(generateDUID(DUID::DUID_LLT)),
+    link_local_("fe80::3a60:77ff:fed5:cdef"),
+    srv_(srv),
+    use_na_(false),
+    use_pd_(false),
+    use_relay_(false) {
+}
+
 void
-Dhcp6Client::applyConfiguration(const Pkt6Ptr& reply) {
+Dhcp6Client::applyRcvdConfiguration(const Pkt6Ptr& reply) {
     typedef OptionCollection Opts;
     // Get all options in the reply message and pick IA_NA and IA_PD.
     Opts opts = reply->options_;
     for (Opts::const_iterator opt = opts.begin(); opt != opts.end(); ++opt) {
         Option6IAPtr ia = boost::dynamic_pointer_cast<Option6IA>(opt->second);
-        // If the current one is not IA option, get the next one.
         if (!ia) {
             continue;
         }
-        // The default value of the prefix length is 128 (as for IPv6 address),
-        // as at this point we don't know if we are dealing with the address
-        // of prefix.
-        int prefix_len = 128;
-        // Check if this is the address.
-        Option6IAAddrPtr iaaddr = boost::dynamic_pointer_cast<
-            Option6IAAddr>(ia->getOption(D6O_IAADDR));
-        // If this is not the address it may be a prefix.
-        if (!iaaddr) {
-            iaaddr = boost::dynamic_pointer_cast<
-                Option6IAAddr>(ia->getOption(D6O_IAPREFIX));
-            // If this is a prefix, modify the prefix length accordingly.
-            if (iaaddr) {
-                prefix_len = boost::dynamic_pointer_cast<
-                    Option6IAPrefix>(ia->getOption(D6O_IAPREFIX))->getLength();
+
+        const Opts& ia_opts = ia->getOptions();
+        for (Opts::const_iterator iter_ia_opt = ia_opts.begin();
+             iter_ia_opt != ia_opts.end(); ++iter_ia_opt) {
+            OptionPtr ia_opt = iter_ia_opt->second;
+            LeaseInfo lease_info;
+            switch (ia_opt->getType()) {
+            case D6O_IAADDR:
+                {
+                    Option6IAAddrPtr iaaddr = boost::dynamic_pointer_cast<
+                        Option6IAAddr>(ia->getOption(D6O_IAADDR));
+                    if (!iaaddr) {
+                        // There is no address. This IA option may simply
+                        // contain a status code, so let's just reset the
+                        // lease and keep IAID around.
+                        lease_info.lease_ = Lease6();
+                        lease_info.lease_.type_ = Lease::TYPE_NA;
+                        lease_info.lease_.iaid_ = ia->getIAID();
+                        break;
+                    }
+                    lease_info.lease_ = Lease6(Lease::TYPE_NA,
+                                               iaaddr->getAddress(),
+                                               duid_, ia->getIAID(),
+                                               iaaddr->getPreferred(),
+                                               iaaddr->getValid(),
+                                               ia->getT1(), ia->getT2(), 0);
+                    lease_info.lease_.cltt_ = time(NULL);
+                }
+                break;
+
+            case D6O_IAPREFIX:
+                {
+                    Option6IAPrefixPtr iaprefix = boost::dynamic_pointer_cast<
+                        Option6IAPrefix>(ia->getOption(D6O_IAPREFIX));
+                    if (!iaprefix) {
+                        // There is no prefix. This IA option may simply
+                        // contain a status code, so let's just reset the
+                        // lease and keep IAID around.
+                        lease_info.lease_ = Lease6();
+                        lease_info.lease_.type_ = Lease::TYPE_PD;
+                        lease_info.lease_.iaid_ = ia->getIAID();
+                        break;
+                    }
+                    lease_info.lease_ = Lease6(Lease::TYPE_PD,
+                                               iaprefix->getAddress(), duid_,
+                                               ia->getIAID(),
+                                               iaprefix->getPreferred(),
+                                               iaprefix->getValid(),
+                                               ia->getT1(), ia->getT2(), 0,
+                                               iaprefix->getLength());
+                    lease_info.lease_.cltt_ = time(NULL);
+                }
+                break;
+
+            case D6O_STATUS_CODE:
+                {
+                    // Check if the server has sent status code. If no status
+                    // code, assume the status code to be 0.
+                    OptionCustomPtr status_code = boost::dynamic_pointer_cast<
+                        OptionCustom>(ia->getOption(D6O_STATUS_CODE));
+                    lease_info.status_code_ =
+                        status_code ? status_code->readInteger<uint16_t>(0) : 0;
+                }
+                break;
+
+            default:
+                ; // no-op
             }
-        }
-        /// Set the lease information if we have a prefix or address.
-        LeaseInfo lease_info;
-        if (iaaddr) {
-            Lease6 lease((prefix_len == 128 ? Lease::TYPE_NA : Lease::TYPE_PD),
-                         iaaddr->getAddress(), duid_,
-                         ia->getIAID(), iaaddr->getPreferred(),
-                         iaaddr->getValid(), ia->getT1(), ia->getT2(), 0,
-                         prefix_len);
-            lease.cltt_ = time(NULL);
-
-            lease_info.lease_ = lease;
-        } else {
-            // There is no prefix and no address. This IA option may simply
-            // contain a status code, so let's just reset the lease and keep
-            // IAID around.
-            lease_info.lease_ = Lease6();
-            lease_info.lease_.iaid_ = ia->getIAID();
-        }
 
-        // Check if the server has sent status code. If no status code, assume
-        // the status code to be 0.
-        OptionCustomPtr status_code = boost::dynamic_pointer_cast<
-            OptionCustom>(ia->getOption(D6O_STATUS_CODE));
-        if (status_code) {
-            lease_info.status_code_ = status_code->readInteger<uint16_t>(0);
-        } else {
-            lease_info.status_code_ = 0;
+            applyLease(lease_info);
         }
-        applyLease(lease_info);
     }
 }
 
@@ -111,6 +148,7 @@ Dhcp6Client::applyLease(const LeaseInfo& lease_info) {
         // server hasn't sent the IA option. In this case, there is no
         // lease assignment so we keep what we have.
         if ((existing_lease.iaid_ == lease_info.lease_.iaid_)
+            && (existing_lease.type_ == lease_info.lease_.type_)
             && (lease_info.lease_.addr_ != asiolink::IOAddress("::"))) {
             config_.leases_[i] = lease_info;
             return;
@@ -218,7 +256,7 @@ Dhcp6Client::doRequest() {
 
     // Apply new configuration only if the server has responded.
     if (context_.response_) {
-        applyConfiguration(context_.response_);
+        applyRcvdConfiguration(context_.response_);
     }
 }
 
@@ -231,7 +269,7 @@ Dhcp6Client::doRebind() {
     context_.response_ = receiveOneMsg();
     // Apply configuration only if the server has responded.
     if (context_.response_) {
-        applyConfiguration(context_.response_);
+        applyRcvdConfiguration(context_.response_);
     }
 }
 
@@ -253,7 +291,9 @@ Dhcp6Client::generateDUID(DUID::DUIDType duid_type) const {
                   " generation of DUID LLT");
     }
     duid.push_back(static_cast<uint8_t>(duid_type));
-    duid.insert(duid.end(), 4, 0);
+    for (int i = 0; i < 4; ++i) {
+        duid.push_back(static_cast<uint8_t>(rand() % 255));
+    }
     for (int i = 0; i < 6; ++i) {
         duid.push_back(static_cast<uint8_t>(i));
     }
@@ -319,6 +359,6 @@ Dhcp6Client::sendMsg(const Pkt6Ptr& msg) {
 }
 
 
-}
-}
-}
+} // end of namespace isc::dhcp::test
+} // end of namespace isc::dhcp
+} // end of namespace isc

src/bin/dhcp6/tests/dhcp6_client.h

@@ -64,6 +64,10 @@ public:
         /// @brief Holds the last status code that server has sent for
         /// the particular lease.
         uint16_t status_code_;
+
+        /// @brief Default constructor for the structure.
+        LeaseInfo() :
+            lease_(), status_code_(0) { }
     };
 
     /// @brief Holds the current client configuration obtained from the
@@ -85,9 +89,34 @@ public:
 
     /// @brief Creates a new client.
     ///
-    /// This constructor initializes the class members to default values.
+    /// This constructor initializes the class members to default values:
+    /// - relay link-addr = 3000:1::1
+    /// - first transaction id = 0
+    /// - dest-addr = All_DHCP_Relay_Agents_and_Servers
+    /// - duid (LLT) = <random 4 bytes>00010203040506
+    /// - link-local-addr = fe80::3a60:77ff:fed5:cdef
+    /// - IA_NA not requested
+    /// - IA_PD not requested
+    /// - not relayed
     Dhcp6Client();
 
+    /// @brief Creates a new client that communicates with a specified server.
+    ///
+    /// This constructor allows passing a pointer to the server object which
+    /// should be used in a test. The server may be preconfigured before passed
+    /// to the constructor. The default configuration used by the client is:
+    /// - relay link-addr = 3000:1::1
+    /// - first transaction id = 0
+    /// - dest-addr = All_DHCP_Relay_Agents_and_Servers
+    /// - duid (LLT) = <random 4 bytes>00010203040506
+    /// - link-local-addr = fe80::3a60:77ff:fed5:cdef
+    /// - IA_NA not requested
+    /// - IA_PD not requested
+    /// - not relayed
+    ///
+    /// @param srv Object representing server under test.
+    Dhcp6Client(boost::shared_ptr<isc::test::NakedDhcpv6Srv>& srv);
+
     /// @brief Performs a 4-way echange between the client and the server.
     ///
     /// If the 4-way exchange is successful, the client should acquire leases
@@ -95,6 +124,12 @@ public:
     /// that have been requested (IA_NA, IA_PD).
     ///
     /// The leases acquired are accessible through the @c config_ member.
+    ///
+    /// @throw This function doesn't throw exceptions on its own, but it calls
+    /// functions that are not exception safe, so it may throw exceptions if
+    /// error occurs.
+    ///
+    /// @todo Perform sanity checks on returned messages.
     void doSARR();
 
     /// @brief Send Solicit and receive Advertise.
@@ -102,6 +137,12 @@ public:
     /// This function simulates the first transaction of the 4-way exchange,
     /// i.e. sends a Solicit to the server and receives Advertise. It doesn't
     /// set the lease configuration in the @c config_.
+    ///
+    /// @throw This function doesn't throw exceptions on its own, but it calls
+    /// functions that are not exception safe, so it may throw exceptions if
+    /// error occurs.
+    ///
+    /// @todo Perform sanity checks on returned messages.
     void doSolicit();
 
     /// @brief Sends a Rebind to the server and receives the Reply.
@@ -111,6 +152,12 @@ public:
     /// (either address or prefixes) and places them in the Rebind message.
     /// If the server responds to the Rebind (and extends the lease lifetimes)
     /// the current lease configuration is updated.
+    ///
+    /// @throw This function doesn't throw exceptions on its own, but it calls
+    /// functions that are not exception safe, so it may throw exceptions if
+    /// error occurs.
+    ///
+    /// @todo Perform sanity checks on returned messages.
     void doRebind();
 
     /// @brief Sends Request to the server and receives Reply.
@@ -120,6 +167,12 @@ public:
     /// from the current context (server's Advertise) to request acquisition
     /// of offered IAs. If the server responds to the Request (leases are
     /// acquired) the client's lease configuration is updated.
+    ///
+    /// @throw This function doesn't throw exceptions on its own, but it calls
+    /// functions that are not exception safe, so it may throw exceptions if
+    /// error occurs.
+    ///
+    /// @todo Perform sanity checks on returned messages.
     void doRequest();
 
     /// @brief Simulates aging of leases by the specified number of seconds.
@@ -141,6 +194,10 @@ public:
 
     /// @brief Returns lease at specified index.
     ///
+    /// @warning This method doesn't check if the specified index is out of
+    /// range. The caller is responsible for using a correct offset by
+    /// invoking the @c getLeaseNum function.
+    ///
     /// @param at Index of the lease held by the client.
     /// @return A lease at the specified index.
     Lease6 getLease(const size_t at) const {
@@ -149,6 +206,10 @@ public:
 
     /// @brief Returns status code set by the server for the lease.
     ///
+    /// @warning This method doesn't check if the specified index is out of
+    /// range. The caller is responsible for using a correct offset by
+    /// invoking the @c getLeaseNum function.
+    ///
     /// @param at Index of the lease held by the client.
     /// @return A status code for the lease at the specified index.
     uint16_t getStatusCode(const size_t at) const {
@@ -215,8 +276,11 @@ public:
     ///
     /// @param use Parameter which 'true' value indicates that client should
     /// simulate sending messages via relay.
-    void useRelay(const bool use = true) {
+    /// @param link_addr Relay link-addr.
+    void useRelay(const bool use = true,
+                  const asiolink::IOAddress& link_addr = asiolink::IOAddress("3000:1::1")) {
         use_relay_ = use;
+        relay_link_addr_ = link_addr;
     }
 
     /// @brief Lease configuration obtained by the client.
@@ -231,14 +295,20 @@ private:
     ///
     /// This method is called when the client obtains a new configuration
     /// from the server in the Reply message. This function adds new leases
-    /// or replaces existing ones.
+    /// or replaces existing ones, on the client's side. Client uses these
+    /// leases in any later communication with the server when doing Renew
+    /// or Rebind.
     ///
     /// @param reply Server response.
-    void applyConfiguration(const Pkt6Ptr& reply);
+    ///
+    /// @todo Currently this function supports one IAAddr or IAPrefix option
+    /// within IA. We will need to extend it to support multiple options
+    /// within a single IA once server supports that.
+    void applyRcvdConfiguration(const Pkt6Ptr& reply);
 
     /// @brief Applies configuration for the single lease.
     ///
-    /// This method is called by the @c Dhcp6Client::applyConfiguration for
+    /// This method is called by the @c Dhcp6Client::applyRcvdConfiguration for
     /// each individual lease.
     ///
     /// @param lease_info Structure holding new lease information.
@@ -252,6 +322,8 @@ private:
     ///
     /// @param source Message from which IA options will be copied.
     /// @param dest Message to which IA options will be copied.
+    ///
+    /// @todo Add support for IA_TA.
     void copyIAs(const Pkt6Ptr& source, const Pkt6Ptr& dest);
 
     /// @brief Creates IA options from existing configuration.
@@ -283,6 +355,10 @@ private:
 
     /// @brief Simulates sending a message to the server.
     ///
+    /// This function instantly triggers processing of the message by the
+    /// server. The server's response can be gathered by invoking the
+    /// @c receiveOneMsg function.
+    ///
     /// @param msg Message to be sent.
     void sendMsg(const Pkt6Ptr& msg);
 

src/bin/dhcp6/tests/rebind_unittest.cc

@@ -29,7 +29,40 @@ using namespace isc::test;
 namespace {
 
 /// @brief Set of JSON configurations used throughout the Rebind tests.
-const std::string REBIND_CONFIGS[] = {
+///
+/// - Configuration 0:
+///   - only addresses (no prefixes)
+///   - 2 subnets with 2001:db8:1::/64 and 2001:db8:2::64
+///   - 1 subnet for eth0 and 1 subnet for eth1
+///
+/// - Configuration 1:
+///   - similar to Configuration 0 but different subnets
+///   - pools configured: 2001:db8:3::/64 and 2001:db8:4::/64
+///
+/// - Configuration 2:
+///   - similar to Configuration 0 and Configuration 1
+///   - pools configured: 3000:1::/64 and 3000:2::/64
+///   - this specific configuration is used by tests using relays
+///
+/// - Configuration 3:
+///   - similar to Configuration 2 but with different subnets
+///   - pools configured: 3000:3::/64 and 3000:4::/64
+///   - this specific configuration is used by tests using relays
+///
+/// - Configuration 5:
+///   - only prefixes (no addresses)
+///   - 2 subnets: 2001:db8:1::/40 and 2001:db8:2::/40
+///   - 2 prefix pools: 2001:db8:1::/72 and 2001:db8:2::/72
+///   - 1 subnet for eth0 and 1 subnet for eth1
+///   - this specific configuration is used by tests which don't use relays
+///
+/// - Configuration 6:
+///   - similar to Configuration 5 but with different subnets
+///   - 2 subnets: 2001:db8:3::/40 and 2001:db8:4::/40
+///   - 2 prefix pools: 2001:db8:3::/72 and 2001:db8:4::/72
+///   - delegated length /80
+///   - this specific configuration is used by tests which don't use relays
+const char* REBIND_CONFIGS[] = {
 // Configuration 0
     "{ \"interfaces\": [ \"all\" ],"
         "\"preferred-lifetime\": 3000,"
@@ -107,25 +140,6 @@ const std::string REBIND_CONFIGS[] = {
         "\"valid-lifetime\": 4000 }",
 
 // Configuration 4
-    "{ \"interfaces\": [ \"all\" ],"
-        "\"preferred-lifetime\": 3000,"
-        "\"rebind-timer\": 2000, "
-        "\"renew-timer\": 1000, "
-        "\"subnet6\": [ { "
-        "    \"pool\": [ \"3000:3::/64\" ],"
-        "    \"subnet\": \"3000:3::/48\", "
-        "    \"interface-id\": \"\","
-        "    \"interface\": \"eth1\""
-        " },"
-        " {"
-        "    \"pool\": [ \"3000:4::/64\" ],"
-        "    \"subnet\": \"3000:4::/48\", "
-        "    \"interface-id\": \"\","
-        "    \"interface\": \"eth0\""
-        " } ],"
-        "\"valid-lifetime\": 4000 }",
-
-// Configuration 5
     "{ \"interfaces\": [ \"all\" ],"
         "\"preferred-lifetime\": 3000,"
         "\"rebind-timer\": 2000, "
@@ -152,7 +166,7 @@ const std::string REBIND_CONFIGS[] = {
         " } ],"
         "\"valid-lifetime\": 4000 }",
 
-// Configuration 6
+// Configuration 5
     "{ \"interfaces\": [ \"all\" ],"
         "\"preferred-lifetime\": 3000,"
         "\"rebind-timer\": 2000, "
@@ -218,6 +232,8 @@ RebindTest::configure(const std::string& config, NakedDhcpv6Srv& srv) {
 void
 RebindTest::requestLease(const int config_index, const int subnets_num,
                          Dhcp6Client& client) {
+    // Check that the index is in the configuration table.
+    ASSERT_LT(config_index, sizeof(REBIND_CONFIGS)/sizeof(REBIND_CONFIGS[0]));
     // Configure the server.
     configure(REBIND_CONFIGS[config_index], *client.getServer());
     // Make sure we ended-up having expected number of subnets configured.
@@ -241,6 +257,8 @@ RebindTest::requestLease(const int config_index, const int subnets_num,
     EXPECT_EQ(STATUS_Success, client.getStatusCode(0));
 }
 
+// Test that directly connected client's Rebind message is processed and Reply
+// message is sent back.
 TEST_F(RebindTest, directClient) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -266,6 +284,8 @@ TEST_F(RebindTest, directClient) {
     EXPECT_TRUE(lease_server2);
 }
 
+// Test that server doesn't extend the lease when the configuration has changed
+// such that the existing subnet is replaced with a different subnet.
 TEST_F(RebindTest, directClientChangingSubnet) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -298,6 +318,8 @@ TEST_F(RebindTest, directClientChangingSubnet) {
 
 }
 
+// Check that the server doesn't extend the lease for the client when the
+// client sends IAID which doesn't belong to the lease that client has.
 TEST_F(RebindTest, directClientChangingIAID) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -323,6 +345,8 @@ TEST_F(RebindTest, directClientChangingIAID) {
 
 }
 
+// Check that server sends NoBinding when the lease has been lost from
+// the database and client is trying to Rebind it.
 TEST_F(RebindTest, directClientLostLease) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -341,6 +365,9 @@ TEST_F(RebindTest, directClientLostLease) {
     EXPECT_EQ(STATUS_NoBinding, client.getStatusCode(0));
 }
 
+/// @todo Extend tests for direct client changing address.
+
+// Check that the client can Rebind existing lease through a relay.
 TEST_F(RebindTest, relayedClient) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -371,6 +398,9 @@ TEST_F(RebindTest, relayedClient) {
     EXPECT_TRUE(lease_server2);
 }
 
+// Check that the lease is not extended for the relayed client when the
+// configuration has changed such that the subnet that client is using
+// doesn't exist anymore.
 TEST_F(RebindTest, relayedClientChangingSubnet) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -409,6 +439,8 @@ TEST_F(RebindTest, relayedClientChangingSubnet) {
 
 }
 
+// Check that the lease is not extended for the relayed client when the IAID in
+// the Rebind message doesn't match the one recorded for the client.
 TEST_F(RebindTest, relayedClientChangingIAID) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -438,6 +470,8 @@ TEST_F(RebindTest, relayedClientChangingIAID) {
 
 }
 
+// Check that the relayed client receives NoBinding when the lease that he
+// is Rebinding has been lost from the database.
 TEST_F(RebindTest, relayedClientLostLease) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -460,6 +494,8 @@ TEST_F(RebindTest, relayedClientLostLease) {
     EXPECT_EQ(STATUS_NoBinding, client.getStatusCode(0));
 }
 
+// Check that relayed client receives the IA with lifetimes of 0, when
+// client is tgrying to Rebind using an address it doesn't have.
 TEST_F(RebindTest, relayedClientChangingAddress) {
     Dhcp6Client client;
     // Configure client to request IA_NA.
@@ -497,13 +533,13 @@ TEST_F(RebindTest, relayedClientChangingAddress) {
     EXPECT_NE(0, lease_server->preferred_lft_);
 }
 
-
+// Check that the server extends the lease for the client having a prefix.
 TEST_F(RebindTest, directClientPD) {
     Dhcp6Client client;
     // Configure client to request IA_PD.
     client.usePD();
     // Make 4-way exchange to get the lease.
-    ASSERT_NO_FATAL_FAILURE(requestLease(5, 2, client));
+    ASSERT_NO_FATAL_FAILURE(requestLease(4, 2, client));
     // Keep the client's lease for future reference.
     Lease6 lease_client = client.getLease(0);
     // Send Rebind message to the server.
@@ -523,18 +559,21 @@ TEST_F(RebindTest, directClientPD) {
     EXPECT_TRUE(lease_server2);
 }
 
+// Check that the prefix lifetime is not extended for the client in case
+// the configuration has been changed such, that the subnet he is using
+// doesn't exist anymore.
 TEST_F(RebindTest, directClientPDChangingSubnet) {
     Dhcp6Client client;
     // Configure client to request IA_PD.
     client.usePD();
     // Make 4-way exchange to get the lease.
-    ASSERT_NO_FATAL_FAILURE(requestLease(5, 2, client));
+    ASSERT_NO_FATAL_FAILURE(requestLease(4, 2, client));
     // Keep the client's lease for future reference.
     Lease6 lease_client = client.getLease(0);
     // Reconfigure the server so as the new subnet is served on the
     // client's interface. Note that there will also be a new subnet
     // id assigned to the subnet on this interface.
-    configure(REBIND_CONFIGS[6], *client.getServer());
+    configure(REBIND_CONFIGS[5], *client.getServer());
     // Try to rebind, using the address that the client had acquired using
     // previous server configuration.
     ASSERT_NO_THROW(client.doRebind());
@@ -557,12 +596,15 @@ TEST_F(RebindTest, directClientPDChangingSubnet) {
     EXPECT_TRUE(lease_server2);
 }
 
+// Check that the prefix lifetime is not extended for the client when the
+// IAID used in the Rebind is not matching the one recorded by the server
+// for the particular client.
 TEST_F(RebindTest, directClientPDChangingIAID) {
     Dhcp6Client client;
     // Configure client to request IA_PD.
     client.usePD();
     // Make 4-way exchange to get the lease.
-    ASSERT_NO_FATAL_FAILURE(requestLease(5, 2, client));
+    ASSERT_NO_FATAL_FAILURE(requestLease(4, 2, client));
     // Keep the client's lease for future reference.
     Lease6 lease_client = client.getLease(0);
     // Modify the IAID of the lease record that client stores. By adding
@@ -585,17 +627,21 @@ TEST_F(RebindTest, directClientPDChangingIAID) {
     EXPECT_TRUE(lease_server);
 }
 
+// Check that the prefix lifetime is not extended for the client when the
+// prefix used in Rebind message doesn't match the one that client has.
 TEST_F(RebindTest, directClientPDChangingPrefix) {
     Dhcp6Client client;
     // Configure client to request IA_PD.
     client.usePD();
     // Make 4-way exchange to get the lease.
-    ASSERT_NO_FATAL_FAILURE(requestLease(5, 2, client));
+    ASSERT_NO_FATAL_FAILURE(requestLease(4, 2, client));
     // Keep the client's lease for future reference.
     Lease6 lease_client = client.getLease(0);
     // Modify the Prefix of the lease record that client stores. The server
     // should check that the prefix is invalid (hasn't been allocated for
     // the particular IAID).
+    ASSERT_NE(client.config_.leases_[0].lease_.addr_,
+              IOAddress("2001:db8:1:10::"));
     client.config_.leases_[0].lease_.addr_ = IOAddress("2001:db8:1:10::");
     // Try to Rebind. The client will use correct IAID but will specify a
     // wrong prefix. The server will discover that the client has a binding
@@ -625,6 +671,7 @@ TEST_F(RebindTest, directClientPDChangingPrefix) {
 }
 
 /// @todo Extend PD tests for relayed messages.
+/// @todo Extend PD tests to cover same prefix buyt different length.
 
 // This test checks that the Rebind message is discarded by the server if it
 // has been sent to unicast address (RFC3315, section 15).
@@ -636,7 +683,7 @@ TEST_F(RebindTest, unicast) {
     ASSERT_NO_FATAL_FAILURE(requestLease(0, 2, client));
     // Keep the client's lease for future reference.
     Lease6 lease_client = client.getLease(0);
-    // Set the unicast destionation address for the Rebind message.
+    // Set the unicast destination address for the Rebind message.
     // The Rebind should be discarded when sent to unicast address,
     // according to section 15 of RFC3315.
     client.setDestAddress(IOAddress("2001:db8:1::1"));
@@ -655,5 +702,38 @@ TEST_F(RebindTest, unicast) {
     EXPECT_FALSE(client.getContext().response_);
 }