Commit 8f1fb501 authored by Thomas Markwalder's avatar Thomas Markwalder

[#597,!323] Use round() when calculating T1/T2 to ensure consistent results

src/bin/dhcp4/dhcp4_srv.cc
    Dhcpv4Srv::setTeeTimes() - added use of round()

src/bin/dhcp6/dhcp6_srv.cc
    Dhcpv6Srv::setTeeTimes() - added use of round()

src/bin/dhcp6/tests/dhcp6_test_utils.cc
    Dhcpv6SrvTest::checkIA_NA() - replaced used of
    EXPECTED_EQ with logic to add a failure so callers
    can ASSERT_NO_FATAL_FAILURE and pinpoint the invocation.

src/bin/dhcp6/tests/hooks_unittest.cc
    Wrapped calls to checkIA_NA() with ASSERT_NO_FATAL_FAILURE
    Updated expected T1 values to adjust for use of round().
parent 05b4f388
......@@ -2269,7 +2269,7 @@ Dhcpv4Srv::setTeeTimes(const Lease4Ptr& lease, const Subnet4Ptr& subnet, Pkt4Ptr
t2_time = subnet->getT2();
} else if (subnet->getCalculateTeeTimes()) {
// Calculating tee times is enabled, so calculated it.
t2_time = static_cast<uint32_t>(subnet->getT2Percent() * (lease->valid_lft_));
t2_time = static_cast<uint32_t>(round(subnet->getT2Percent() * (lease->valid_lft_)));
}
// Send the T2 candidate value only if it's sane: to be sane it must be less than
......@@ -2288,7 +2288,7 @@ Dhcpv4Srv::setTeeTimes(const Lease4Ptr& lease, const Subnet4Ptr& subnet, Pkt4Ptr
t1_time = subnet->getT1();
} else if (subnet->getCalculateTeeTimes()) {
// Calculating tee times is enabled, so calculate it.
t1_time = static_cast<uint32_t>(subnet->getT1Percent() * (lease->valid_lft_));
t1_time = static_cast<uint32_t>(round(subnet->getT1Percent() * (lease->valid_lft_)));
}
// Send T1 if it's sane: If we sent T2, T1 must be less than that. If not it must be
......
......@@ -3908,7 +3908,7 @@ Dhcpv6Srv::setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6
t2_time = subnet->getT2();
} else if (subnet->getCalculateTeeTimes()) {
// Calculating tee times is enabled, so calculate it.
t2_time = static_cast<uint32_t>(subnet->getT2Percent() * preferred_lft);
t2_time = static_cast<uint32_t>(round(subnet->getT2Percent() * preferred_lft));
}
// We allow T2 to be any value.
......@@ -3922,7 +3922,7 @@ Dhcpv6Srv::setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6
t1_time = subnet->getT1();
} else if (subnet->getCalculateTeeTimes()) {
// Calculating tee times is enabled, so calculate it.
t1_time = static_cast<uint32_t>(subnet->getT1Percent() * preferred_lft);
t1_time = static_cast<uint32_t>(round(subnet->getT1Percent() * preferred_lft));
}
// T1 is sane if it is less than or equal to T2.
......
// Copyright (C) 2013-2018 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2013-2019 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -96,9 +96,23 @@ Dhcpv6SrvTest::checkIA_NA(const Pkt6Ptr& rsp, uint32_t expected_iaid,
return (boost::shared_ptr<Option6IAAddr>());
}
EXPECT_EQ(expected_iaid, ia->getIAID());
EXPECT_EQ(expected_t1, ia->getT1());
EXPECT_EQ(expected_t2, ia->getT2());
if (expected_iaid != ia->getIAID()) {
ADD_FAILURE() << "ia->iaid: " << ia->getIAID()
<< " is not expected value: " << expected_iaid;
return (boost::shared_ptr<Option6IAAddr>());
}
if (expected_t1 != ia->getT1()) {
ADD_FAILURE() << "ia->t1: " << ia->getT1()
<< " is not expected value: " << expected_t1;
return (boost::shared_ptr<Option6IAAddr>());
}
if (expected_t2 != ia->getT2()) {
ADD_FAILURE() << "ia->t2: " << ia->getT2()
<< " is not expected value: " << expected_t2;
return (boost::shared_ptr<Option6IAAddr>());
}
tmp = ia->getOption(D6O_IAADDR);
boost::shared_ptr<Option6IAAddr> addr = boost::dynamic_pointer_cast<Option6IAAddr>(tmp);
......
......@@ -2741,8 +2741,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Renew) {
ASSERT_TRUE(tmp);
// Check that IA_NA was returned and that there's an address included
boost::shared_ptr<Option6IAAddr> addr_opt = checkIA_NA(reply, 234, subnet_->getT1(),
subnet_->getT2());
boost::shared_ptr<Option6IAAddr> addr_opt;
ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 234, subnet_->getT1(),
subnet_->getT2()));
ASSERT_TRUE(addr_opt);
// Check that the lease is really in the database
......@@ -2834,7 +2835,8 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Renew) {
ASSERT_TRUE(tmp);
// Check that IA_NA was returned and that there's an address included
boost::shared_ptr<Option6IAAddr> addr_opt = checkIA_NA(reply, 1000, 601, 802);
boost::shared_ptr<Option6IAAddr> addr_opt;
ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 1000, 602, 802));
ASSERT_TRUE(addr_opt);
// Check that the lease is really in the database
......@@ -3863,8 +3865,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Rebind) {
ASSERT_TRUE(tmp);
// Check that IA_NA was returned and that there's an address included
boost::shared_ptr<Option6IAAddr> addr_opt = checkIA_NA(reply, 234, subnet_->getT1(),
subnet_->getT2());
boost::shared_ptr<Option6IAAddr> addr_opt;
ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 234, subnet_->getT1(),
subnet_->getT2()));
ASSERT_TRUE(addr_opt);
// Check that the lease is really in the database
......@@ -3951,7 +3954,8 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Rebind) {
// Check that IA_NA was returned and that there's an address included
// Note we also verify that T1 and T2 were calculated correctly.
boost::shared_ptr<Option6IAAddr> addr_opt = checkIA_NA(reply, 1000, 601, 802);
boost::shared_ptr<Option6IAAddr> addr_opt;
ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 1000, 602, 802));
ASSERT_TRUE(addr_opt);
// Check that the lease is really in the database
......@@ -4757,7 +4761,8 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier) {
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);
boost::shared_ptr<Option6IAAddr> addr_opt;
ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(adv, 234, 1000, 2000));
ASSERT_TRUE(addr_opt);
ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText());
......@@ -4837,7 +4842,8 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier_hwaddr) {
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);
boost::shared_ptr<Option6IAAddr> addr_opt;
ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(adv, 234, 1000, 2000));
ASSERT_TRUE(addr_opt);
ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText());
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment