[3947] Addressed review comments.

doc/guide/dhcp6-srv.xml

@@ -2386,8 +2386,8 @@ should include options from the isc option space:
       with the coexistence of multiple stateful options in the messages sent
       between the clients and servers.</para>
 
-      <para>The typical example is when the client (being also a requesting
+      <para>The typical example is when the client, such as a requesting
-      router) requests an allocation of both addresses and prefixes when
+      router, requests an allocation of both addresses and prefixes when
       it performs the 4-way (SARR) exchange with the server. If the
       server is not configured to allocate any prefixes but it can allocate
       some addresses, it will respond with the IA_NA(s) containing allocated

src/bin/dhcp6/tests/dhcp6_client.cc

@@ -287,7 +287,7 @@ Dhcp6Client::conditionallyAppendRequestedIA(const Pkt6Ptr& query,
 
     } else if (address_hint_ && (ia_type == D6O_IA_NA)) {
         requested_ia->addOption(address_hint_);
-    } 
+    }
 
     query->addOption(requested_ia);
 }
@@ -323,8 +323,8 @@ Dhcp6Client::copyIAsFromLeases(const Pkt6Ptr& dest) const {
     for (std::set<uint32_t>::const_iterator iaid = iaids.begin();
          iaid != iaids.end(); ++iaid) {
         std::vector<Lease6> leases = getLeasesByIAID(*iaid);
-        // Only a valid lease should be included. If we have received an
+        // Only a valid lease should be included. Do not copy a
-        // error status code, it should not be copied.
+        // lease which have been marked by the server as invalid.
         if (leases[0].valid_lft_ == 0) {
             continue;
         }
@@ -456,8 +456,8 @@ Dhcp6Client::doInfRequest() {
 
     // IA-PD is also not allowed. So it may be useful in testing, too.
     if (use_pd_) {
-        // Insert IA_PD option with iaid=5678
+        // Insert IA_PD option.
-        Option6IAPtr ia(new Option6IA(D6O_IA_PD, 5678));
+        Option6IAPtr ia(new Option6IA(D6O_IA_PD, pd_iaid_));
         if (prefix_hint_) {
             ia->addOption(prefix_hint_);
         }

src/bin/dhcp6/tests/dhcp6_client.h

@@ -437,6 +437,16 @@ public:
     /// This function configures the client to place IA_NA options in its
     /// Solicit messages to request the IPv6 address assignment.
     ///
+    /// @param iaid IAID to be used in the IA_NA.
+    void useNA(const uint32_t iaid) {
+        useNA(true, iaid);
+    }
+
+    /// @brief Place IA_NA options to request address assignment.
+    ///
+    /// This function configures the client to place IA_NA options in its
+    /// Solicit messages to request the IPv6 address assignment.
+    ///
     /// @param use Parameter which 'true' value indicates that client should
     /// request address assignment.
     /// @param iaid IAID to be used in the IA_NA.
@@ -445,6 +455,16 @@ public:
         na_iaid_ = iaid;
     }
 
+    /// @brief Place IA_PD options to request address assignment.
+    ///
+    /// This function configures the client to place IA_NA options in its
+    /// Solicit messages to request the IPv6 address assignment.
+    ///
+    /// @param iaid IAID to be used in the IA_PD.
+    void usePD(const uint32_t iaid) {
+        usePD(true, iaid);
+    }
+
     /// @brief Place IA_PD options to request prefix assignment.
     ///
     /// This function configures the client to place IA_PD options in its

src/bin/dhcp6/tests/renew_unittest.cc

@@ -108,8 +108,15 @@ public:
     ///
     /// Sets up fake interfaces.
     RenewTest()
-        : Dhcpv6MessageTest() {
+        : Dhcpv6MessageTest(), na_iaid_(1234), pd_iaid_(5678) {
     }
+
+    /// @brief IAID used for IA_NA.
+    uint32_t na_iaid_;
+
+    /// @brief IAID used for IA_PD.
+    uint32_t pd_iaid_;
+
 };
 
 // This test verifies that the client can request the prefix delegation
@@ -118,8 +125,8 @@ TEST_F(RenewTest, requestPrefixInRenew) {
     Dhcp6Client client;
 
     // Configure client to request IA_NA and IA_PD.
-    client.useNA();
+    client.useNA(na_iaid_);
-    client.usePD();
+    client.usePD(pd_iaid_);
 
     // Configure the server with NA pools only.
     ASSERT_NO_THROW(configure(RENEW_CONFIGS[0], *client.getServer()));
@@ -133,17 +140,23 @@ TEST_F(RenewTest, requestPrefixInRenew) {
     // Make sure that the client has acquired NA lease.
     std::vector<Lease6> leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(1, leases_client_na.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 
     // The client should not acquire a PD lease.
     std::vector<Lease6> leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_TRUE(leases_client_pd.empty());
-    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(5678));
+    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(pd_iaid_));
 
     // Send Renew message to the server, including IA_NA and requesting IA_PD.
     ASSERT_NO_THROW(client.doRenew());
     leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_TRUE(leases_client_pd.empty());
-    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(5678));
+    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(pd_iaid_));
+
+    std::vector<Lease6> leases_client_na_renewed =
+        client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(1, leases_client_na_renewed.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 
     // Reconfigure the server to use both NA and PD pools.
     configure(RENEW_CONFIGS[2], *client.getServer());
@@ -152,10 +165,9 @@ TEST_F(RenewTest, requestPrefixInRenew) {
     ASSERT_NO_THROW(client.doRenew());
 
     // Make sure that the client has acquired NA lease.
-    std::vector<Lease6> leases_client_na_renewed =
+    leases_client_na_renewed = client.getLeasesByType(Lease::TYPE_NA);
-        client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(1, leases_client_na_renewed.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(1234));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 
     // The lease should have been renewed.
     EXPECT_EQ(1000, leases_client_na_renewed[0].cltt_ - leases_client_na[0].cltt_);
@@ -163,7 +175,7 @@ TEST_F(RenewTest, requestPrefixInRenew) {
     // The client should now also acquire a PD lease.
     leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_EQ(1, leases_client_pd.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
 }
 
 // This test verifies that the client can request a prefix delegation
@@ -172,8 +184,8 @@ TEST_F(RenewTest, requestPrefixInRenewUseHint) {
     Dhcp6Client client;
 
     // Configure client to request IA_NA and IA_PD.
-    client.useNA();
+    client.useNA(na_iaid_);
-    client.usePD();
+    client.usePD(pd_iaid_);
 
     // Configure the server with NA pools only.
     ASSERT_NO_THROW(configure(RENEW_CONFIGS[0], *client.getServer()));
@@ -191,13 +203,18 @@ TEST_F(RenewTest, requestPrefixInRenewUseHint) {
     // The client should not acquire a PD lease.
     std::vector<Lease6> leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_TRUE(leases_client_pd.empty());
-    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(5678));
+    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(pd_iaid_));
 
     // Send Renew message to the server, including IA_NA and requesting IA_PD.
     ASSERT_NO_THROW(client.doRenew());
     leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_TRUE(leases_client_pd.empty());
-    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(5678));
+    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(pd_iaid_));
+
+    std::vector<Lease6> leases_client_na_renewed =
+        client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(1, leases_client_na_renewed.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 
     // Specify the hint used for IA_PD.
     client.useHint(0, 0, 64, "::");
@@ -206,14 +223,13 @@ TEST_F(RenewTest, requestPrefixInRenewUseHint) {
     ASSERT_NO_THROW(client.doRenew());
 
     // Make sure that the client has acquired NA lease.
-    std::vector<Lease6> leases_client_na_renewed =
+    leases_client_na_renewed = client.getLeasesByType(Lease::TYPE_NA);
-        client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(1, leases_client_na_renewed.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(1234));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 
     leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_TRUE(leases_client_pd.empty());
-    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(5678));
+    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(pd_iaid_));
 
     // Reconfigure the server to use both NA and PD pools.
     configure(RENEW_CONFIGS[2], *client.getServer());
@@ -227,7 +243,7 @@ TEST_F(RenewTest, requestPrefixInRenewUseHint) {
     // Make sure that the client has acquired NA lease.
     leases_client_na_renewed = client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(1, leases_client_na_renewed.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(1234));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 
     // The lease should have been renewed.
     EXPECT_GE(leases_client_na_renewed[0].cltt_ - leases_client_na[0].cltt_, 1000);
@@ -235,7 +251,7 @@ TEST_F(RenewTest, requestPrefixInRenewUseHint) {
     // The client should now also acquire a PD lease.
     leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_EQ(1, leases_client_pd.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
 }
 
 // This test verifies that the client can request the prefix delegation
@@ -244,8 +260,8 @@ TEST_F(RenewTest, requestAddressInRenew) {
     Dhcp6Client client;
 
     // Configure client to request IA_NA and IA_PD.
-    client.useNA();
+    client.useNA(na_iaid_);
-    client.usePD();
+    client.usePD(pd_iaid_);
 
     // Configure the server with PD pools only.
     ASSERT_NO_THROW(configure(RENEW_CONFIGS[1], *client.getServer()));
@@ -259,20 +275,26 @@ TEST_F(RenewTest, requestAddressInRenew) {
     // Make sure that the client has acquired PD lease.
     std::vector<Lease6> leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_EQ(1, leases_client_pd.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
 
     // The client should not acquire a NA lease.
     std::vector<Lease6> leases_client_na =
         client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(0, leases_client_na.size());
-    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
+    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(na_iaid_));
 
     // Send Renew message to the server, including IA_PD and requesting IA_NA.
     // The server should return NoAddrsAvail status code in this case.
     ASSERT_NO_THROW(client.doRenew());
     leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(0, leases_client_na.size());
-    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
+    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(na_iaid_));
+
+    std::vector<Lease6> leases_client_pd_renewed =
+        client.getLeasesByType(Lease::TYPE_PD);
+    ASSERT_EQ(1, leases_client_pd_renewed.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
+    EXPECT_GE(leases_client_pd_renewed[0].cltt_ - leases_client_pd[0].cltt_, 1000);
 
     // Reconfigure the server to use both NA and PD pools.
     configure(RENEW_CONFIGS[2], *client.getServer());
@@ -281,16 +303,15 @@ TEST_F(RenewTest, requestAddressInRenew) {
     ASSERT_NO_THROW(client.doRenew());
 
     // Make sure that the client has renewed PD lease.
-    std::vector<Lease6> leases_client_pd_renewed =
+    leases_client_pd_renewed =  client.getLeasesByType(Lease::TYPE_PD);
-        client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_EQ(1, leases_client_pd_renewed.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
     EXPECT_GE(leases_client_pd_renewed[0].cltt_ - leases_client_pd[0].cltt_, 1000);
 
     // The client should now also acquire a NA lease.
     leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(1, leases_client_na.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(1234));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 }
 
 // This test verifies that the client can request address assignment
@@ -299,8 +320,8 @@ TEST_F(RenewTest, requestAddressInRenewHint) {
     Dhcp6Client client;
 
     // Configure client to request IA_NA and IA_PD.
-    client.useNA();
+    client.useNA(na_iaid_);
-    client.usePD();
+    client.usePD(pd_iaid_);
 
     // Configure the server with PD pools only.
     ASSERT_NO_THROW(configure(RENEW_CONFIGS[1], *client.getServer()));
@@ -314,13 +335,13 @@ TEST_F(RenewTest, requestAddressInRenewHint) {
     // Make sure that the client has acquired PD lease.
     std::vector<Lease6> leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_EQ(1, leases_client_pd.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
 
     // The client should not acquire a NA lease.
     std::vector<Lease6> leases_client_na =
         client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(0, leases_client_na.size());
-    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
+    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(na_iaid_));
 
     client.useHint(0, 0, "2001:db8:1::100");
 
@@ -332,7 +353,13 @@ TEST_F(RenewTest, requestAddressInRenewHint) {
     ASSERT_EQ(1, leases_client_na.size());
     EXPECT_EQ(0, leases_client_na[0].preferred_lft_);
     EXPECT_EQ(0, leases_client_na[0].valid_lft_);
-    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
+    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(na_iaid_));
+
+    std::vector<Lease6> leases_client_pd_renewed =
+        client.getLeasesByType(Lease::TYPE_PD);
+    ASSERT_EQ(1, leases_client_pd_renewed.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
+    EXPECT_GE(leases_client_pd_renewed[0].cltt_ - leases_client_pd[0].cltt_, 1000);
 
     // Reconfigure the server to use both NA and PD pools.
     configure(RENEW_CONFIGS[2], *client.getServer());
@@ -341,16 +368,15 @@ TEST_F(RenewTest, requestAddressInRenewHint) {
     ASSERT_NO_THROW(client.doRenew());
 
     // Make sure that the client has renewed PD lease.
-    std::vector<Lease6> leases_client_pd_renewed =
+    leases_client_pd_renewed = client.getLeasesByType(Lease::TYPE_PD);
-        client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_EQ(1, leases_client_pd_renewed.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(pd_iaid_));
     EXPECT_GE(leases_client_pd_renewed[0].cltt_ - leases_client_pd[0].cltt_, 1000);
 
     // The client should now also acquire a NA lease.
     leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
     ASSERT_EQ(1, leases_client_na.size());
-    EXPECT_EQ(STATUS_Success, client.getStatusCode(1234));
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(na_iaid_));
 }