Browse Source

[3947] Do not send hint prefixes in the Reply to Renew or Rebind.

Marcin Siodelski 9 years ago
parent
commit
5916e577ad
2 changed files with 82 additions and 14 deletions
  1. 9 13
      src/bin/dhcp6/dhcp6_srv.cc
  2. 73 1
      src/bin/dhcp6/tests/renew_unittest.cc

+ 9 - 13
src/bin/dhcp6/dhcp6_srv.cc

@@ -1912,15 +1912,6 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
         // Put the client's prefix into the hints list.
         // Put the client's prefix into the hints list.
         ctx.hints_.push_back(make_pair(prf->getAddress(), prf->getLength()));
         ctx.hints_.push_back(make_pair(prf->getAddress(), prf->getLength()));
     }
     }
-    // We need to remember it as we'll be removing hints from this list as
-    // we extend, cancel or otherwise deal with the leases.
-    bool hints_present = !ctx.hints_.empty();
-
-    /// @todo: The draft-ietf-dhc-dhcpv6-stateful-issues added a new capability
-    /// of the server to to assign new PD leases in both Renew and Rebind.
-    /// There's allow_new_leases_in_renewals_ in the ClientContext6, but we
-    /// currently not use it in PD yet. This should be implemented as part
-    /// of the stateful-issues implementation effort. See ticket #3718.
 
 
     // Call Allocation Engine and attempt to renew leases. Number of things
     // Call Allocation Engine and attempt to renew leases. Number of things
     // may happen. Leases may be extended, revoked (if the lease is no longer
     // may happen. Leases may be extended, revoked (if the lease is no longer
@@ -1954,9 +1945,14 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
     // already, inform the client that he can't have them.
     // already, inform the client that he can't have them.
     for (AllocEngine::HintContainer::const_iterator prefix = ctx.hints_.begin();
     for (AllocEngine::HintContainer::const_iterator prefix = ctx.hints_.begin();
          prefix != ctx.hints_.end(); ++prefix) {
          prefix != ctx.hints_.end(); ++prefix) {
-        OptionPtr prefix_opt(new Option6IAPrefix(D6O_IAPREFIX, prefix->first,
-                                                 prefix->second, 0, 0));
-        ia_rsp->addOption(prefix_opt);
+        // Send the prefix hint with the zero lifetimes only if the prefix
+        // contains non-zero value. A zero value indicates that the hint was
+        // for the prefix length.
+        if (!prefix->first.isV6Zero()) {
+            OptionPtr prefix_opt(new Option6IAPrefix(D6O_IAPREFIX, prefix->first,
+                                                     prefix->second, 0, 0));
+            ia_rsp->addOption(prefix_opt);
+        }
     }
     }
 
 
     // All is left is to insert the status code.
     // All is left is to insert the status code.
@@ -1964,7 +1960,7 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
         // We did not assign anything. If client has sent something, then
         // We did not assign anything. If client has sent something, then
         // the status code is NoBinding, if he sent an empty IA_PD, then it's
         // the status code is NoBinding, if he sent an empty IA_PD, then it's
         // NoAddrsAvailable
         // NoAddrsAvailable
-        if (hints_present && !subnet->getAllocLeasesOnRenew()) {
+        if (!subnet->getAllocLeasesOnRenew()) {
             if (query->getType() == DHCPV6_RENEW) {
             if (query->getType() == DHCPV6_RENEW) {
                 // Insert status code NoBinding to indicate that the lease does not
                 // Insert status code NoBinding to indicate that the lease does not
                 // exist for this client.
                 // exist for this client.

+ 73 - 1
src/bin/dhcp6/tests/renew_unittest.cc

@@ -166,6 +166,78 @@ TEST_F(RenewTest, requestPrefixInRenew) {
     EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
     EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
 }
 }
 
 
+// This test verifies that the client can request a prefix delegation
+// with a hint, while it is renewing an address lease.
+TEST_F(RenewTest, requestPrefixInRenewUseHint) {
+    Dhcp6Client client;
+
+    // Configure client to request IA_NA and IA_PD.
+    client.useNA();
+    client.usePD();
+
+    // Configure the server with NA pools only.
+    ASSERT_NO_THROW(configure(RENEW_CONFIGS[0], *client.getServer()));
+
+    // Perform 4-way exchange.
+    ASSERT_NO_THROW(client.doSARR());
+
+    // Simulate aging of leases.
+    client.fastFwdTime(1000);
+
+    // 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());
+
+    // 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));
+
+    // 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));
+
+    // Specify the hint used for IA_PD.
+    client.useHint(0, 0, 64, "::");
+
+    // Send Renew message to the server, including IA_NA and requesting IA_PD.
+    ASSERT_NO_THROW(client.doRenew());
+
+    // Make sure that the client has acquired NA lease.
+    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(1234));
+
+    leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
+    ASSERT_TRUE(leases_client_pd.empty());
+    ASSERT_EQ(STATUS_NoPrefixAvail, client.getStatusCode(5678));
+
+    // Reconfigure the server to use both NA and PD pools.
+    configure(RENEW_CONFIGS[2], *client.getServer());
+
+    // Specify the hint used for IA_PD.
+    client.useHint(0, 0, 64, "::");
+
+    // Send Renew message to the server, including IA_NA and requesting IA_PD.
+    ASSERT_NO_THROW(client.doRenew());
+
+    // 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));
+
+    // The lease should have been renewed.
+    EXPECT_GE(leases_client_na_renewed[0].cltt_ - leases_client_na[0].cltt_, 1000);
+
+    // 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));
+}
+
 // This test verifies that the client can request the prefix delegation
 // This test verifies that the client can request the prefix delegation
 // while it is renewing an address lease.
 // while it is renewing an address lease.
 TEST_F(RenewTest, requestAddressInRenew) {
 TEST_F(RenewTest, requestAddressInRenew) {
@@ -206,7 +278,7 @@ TEST_F(RenewTest, requestAddressInRenew) {
         client.getLeasesByType(Lease::TYPE_PD);
         client.getLeasesByType(Lease::TYPE_PD);
     ASSERT_EQ(1, leases_client_pd_renewed.size());
     ASSERT_EQ(1, leases_client_pd_renewed.size());
     EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
     EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
-    EXPECT_EQ(1000, leases_client_pd_renewed[0].cltt_ - leases_client_pd[0].cltt_);
+    EXPECT_GE(leases_client_pd_renewed[0].cltt_ - leases_client_pd[0].cltt_, 1000);
 
 
     // The client should now also acquire a NA lease.
     // The client should now also acquire a NA lease.
     leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
     leases_client_na = client.getLeasesByType(Lease::TYPE_NA);