Commit b1fb6811 authored by Francis Dupont's avatar Francis Dupont
Browse files

[#897] Addressed comments

parent 6c785d6c
......@@ -152,8 +152,8 @@ server uses an appropriate pool or subnet to allocate IP addresses
(and/or prefixes), based on the assigned client classes. The details can
be found in :ref:`high-availability-library`.
The BOOTP hook uses the BOOTP class: when an incoming BOOTP query is
recognized it is put in this class and built BOOTP response too.
The BOOTP class is used by the BOOTP hook library to classify and
respond to inbound BOOTP queries.
Other examples are the ALL class, which all incoming packets belong to,
and the KNOWN class, assigned when host reservations exist for a
......
......@@ -1181,11 +1181,11 @@ for client classification and the Flex Identifier hook library
(See either :ref:`classification-using-expressions` or :ref:`flex-id`
for detailed description of the syntax).
The ``add`` and ``supersede`` actions use an expression returning a
string, doing nothing when it evaluates to the empty string. The
``remove`` application uses an expression returning true or false,
doing nothing on false. When it is necessary to set an option to the
empty value this mechanism does not work but a client class can be
The ``add`` and ``supersede`` actions use an expression returning a
string, doing nothing when it evaluates to the empty string. The
``remove`` application uses an expression returning true or false,
doing nothing on false. When it is necessary to set an option to the
empty value this mechanism does not work but a client class can be
used instead.
The ``add`` action adds an option only when the option does not already
......@@ -1215,7 +1215,7 @@ expression.
{ "library": "/usr/local/lib/libdhcp_flex_option.so",
"parameters": {
"options": [
{
{
"code": 67,
"add":
"ifelse(option[host-name].exists,concat(option[host-name].text,'.boot'),'')"
......
......@@ -2186,16 +2186,11 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
LOG_INFO(lease4_logger, DHCP4_LEASE_ADVERT)
.arg(query->getLabel())
.arg(lease->addr_.toText());
} else if (lease->valid_lft_ != Lease::INFINITY_LFT) {
LOG_INFO(lease4_logger, DHCP4_LEASE_ALLOC)
.arg(query->getLabel())
.arg(lease->addr_.toText())
.arg(lease->valid_lft_);
} else {
LOG_INFO(lease4_logger, DHCP4_LEASE_ALLOC)
.arg(query->getLabel())
.arg(lease->addr_.toText())
.arg("infinity");
.arg(Lease::lifetimeToText(lease->valid_lft_));
}
// We're logging this here, because this is the place where we know
......
......@@ -1900,18 +1900,12 @@ Dhcpv6Srv::assignIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
.arg(query->getLabel())
.arg(lease->addr_.toText())
.arg(ia->getIAID());
} else if (lease->valid_lft_ != Lease::INFINITY_LFT) {
LOG_INFO(lease6_logger, DHCP6_LEASE_ALLOC)
.arg(query->getLabel())
.arg(lease->addr_.toText())
.arg(ia->getIAID())
.arg(lease->valid_lft_);
} else {
LOG_INFO(lease6_logger, DHCP6_LEASE_ALLOC)
.arg(query->getLabel())
.arg(lease->addr_.toText())
.arg(ia->getIAID())
.arg("infinity");
.arg(Lease::lifetimeToText(lease->valid_lft_));
}
LOG_DEBUG(lease6_logger, DBG_DHCP6_DETAIL_DATA, DHCP6_LEASE_DATA)
.arg(query->getLabel())
......@@ -2028,20 +2022,13 @@ Dhcpv6Srv::assignIA_PD(const Pkt6Ptr& query, const Pkt6Ptr& /*answer*/,
.arg((*l)->addr_.toText())
.arg(static_cast<int>((*l)->prefixlen_))
.arg(ia->getIAID());
} else if ((*l)->valid_lft_ != Lease::INFINITY_LFT) {
LOG_INFO(lease6_logger, DHCP6_PD_LEASE_ALLOC)
.arg(query->getLabel())
.arg((*l)->addr_.toText())
.arg(static_cast<int>((*l)->prefixlen_))
.arg(ia->getIAID())
.arg((*l)->valid_lft_);
} else {
LOG_INFO(lease6_logger, DHCP6_PD_LEASE_ALLOC)
.arg(query->getLabel())
.arg((*l)->addr_.toText())
.arg(static_cast<int>((*l)->prefixlen_))
.arg(ia->getIAID())
.arg("infinity");
.arg(Lease::lifetimeToText((*l)->valid_lft_));
}
boost::shared_ptr<Option6IAPrefix>
......
......@@ -3549,20 +3549,22 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr,
isc_throw(BadValue, "Can't create a lease without a subnet");
}
// Use the dhcp-lease-time content or the default for lease length.
uint32_t valid_lft = ctx.subnet_->getValid();
OptionPtr opt = ctx.query_->getOption(DHO_DHCP_LEASE_TIME);
OptionUint32Ptr opt_lft;
if (opt) {
opt_lft = boost::dynamic_pointer_cast<OptionInt<uint32_t> >(opt);
}
if (opt_lft) {
valid_lft = ctx.subnet_->getValid().get(opt_lft->getValue());
}
// BOOTP uses infinite valid lifetime.
uint32_t valid_lft;
if (ctx.query_->inClass("BOOTP")) {
// BOOTP uses infinite valid lifetime.
valid_lft = Lease::INFINITY_LFT;
} else {
// Use the dhcp-lease-time content or the default for lease length.
OptionPtr opt = ctx.query_->getOption(DHO_DHCP_LEASE_TIME);
OptionUint32Ptr opt_lft;
if (opt) {
opt_lft = boost::dynamic_pointer_cast<OptionInt<uint32_t> >(opt);
}
if (opt_lft) {
valid_lft = ctx.subnet_->getValid().get(opt_lft->getValue());
} else {
valid_lft = ctx.subnet_->getValid();
}
}
time_t now = time(NULL);
......@@ -3986,20 +3988,21 @@ AllocEngine::updateLease4Information(const Lease4Ptr& lease,
lease->hwaddr_ = ctx.hwaddr_;
lease->client_id_ = ctx.subnet_->getMatchClientId() ? ctx.clientid_ : ClientIdPtr();
lease->cltt_ = time(NULL);
// Use the dhcp-lease-time content or the default for lease length.
OptionPtr opt = ctx.query_->getOption(DHO_DHCP_LEASE_TIME);
OptionUint32Ptr opt_lft;
if (opt) {
opt_lft = boost::dynamic_pointer_cast<OptionInt<uint32_t> >(opt);
}
if (opt_lft) {
lease->valid_lft_ = ctx.subnet_->getValid().get(opt_lft->getValue());
} else {
lease->valid_lft_ = ctx.subnet_->getValid();
}
// BOOTP uses infinite valid lifetime.
if (ctx.query_->inClass("BOOTP")) {
// BOOTP uses infinite valid lifetime.
lease->valid_lft_ = Lease::INFINITY_LFT;
} else {
// Use the dhcp-lease-time content or the default for lease length.
OptionPtr opt = ctx.query_->getOption(DHO_DHCP_LEASE_TIME);
OptionUint32Ptr opt_lft;
if (opt) {
opt_lft = boost::dynamic_pointer_cast<OptionInt<uint32_t> >(opt);
}
if (opt_lft) {
lease->valid_lft_ = ctx.subnet_->getValid().get(opt_lft->getValue());
} else {
lease->valid_lft_ = ctx.subnet_->getValid();
}
}
lease->fqdn_fwd_ = ctx.fwd_dns_update_;
lease->fqdn_rev_ = ctx.rev_dns_update_;
......
......@@ -304,8 +304,8 @@ StatementMap CqlLease4Exchange::tagged_statements_{
"fqdn_fwd, fqdn_rev, hostname, state, user_context "
"FROM lease4 "
"WHERE state = ? "
"AND expire < ? "
"AND valid_lifetime < 4294967295 "
"AND expire < ? "
"LIMIT ? "
"ALLOW FILTERING "}},
......@@ -1019,8 +1019,8 @@ StatementMap CqlLease6Exchange::tagged_statements_ = {
"hwaddr_source, state, user_context "
"FROM lease6 "
"WHERE state = ? "
"AND expire < ? "
"AND valid_lifetime < 4294967295 "
"AND expire < ? "
"LIMIT ? "
"ALLOW FILTERING "}},
......
......@@ -105,6 +105,23 @@
For details, see @ref isc::db::CqlConnection::openDatabase().
@subsection infinite-valid-lifetime Infinite Valid Lifetime
The @c isc::dhcp::Lease class uses cltt (client last transmission time)
and valid lifetime, backend lease uses expire and valid lifetime.
These quantities are bound by the equation:
@code
expire = cltt + valid_lifetime
@endcode
But when expire is a 32 bit date and valid lifetime is the infinity
special value (0xffffffff) this overflows so for MySQL and PostgreSQL
backends this becomes:
@code
expire = cltt + valid_lifetime if valid_lifetime != 0xffffffff
expire = cltt if valid_lifetime == 0xffffffff
@endcode
@section dhcpdb-host Host Backends
Host backends (known also as host data sources) are similar to lease
......
......@@ -25,6 +25,17 @@ const uint32_t Lease::STATE_DEFAULT = 0x0;
const uint32_t Lease::STATE_DECLINED = 0x1;
const uint32_t Lease::STATE_EXPIRED_RECLAIMED = 0x2;
std::string
Lease::lifetimeToText(uint32_t lifetime) {
ostringstream repr;
if (lifetime == INFINITY_LFT) {
repr << "infinity";
} else {
repr << lifetime;
}
return repr.str();
}
Lease::Lease(const isc::asiolink::IOAddress& addr,
uint32_t valid_lft, SubnetID subnet_id, time_t cltt,
const bool fqdn_fwd, const bool fqdn_rev,
......@@ -528,13 +539,9 @@ Lease6::toText() const {
<< "Address: " << addr_ << "\n"
<< "Prefix length: " << static_cast<int>(prefixlen_) << "\n"
<< "IAID: " << iaid_ << "\n"
<< "Pref life: " << preferred_lft_ << "\n";
if (valid_lft_ != INFINITY_LFT) {
stream << "Valid life: " << valid_lft_ << "\n";
} else {
stream << "Valid life: " << "infinity" << "\n";
}
stream << "Cltt: " << cltt_ << "\n"
<< "Pref life: " << lifetimeToText(preferred_lft_) << "\n"
<< "Valid life: " << lifetimeToText(valid_lft_) << "\n"
<< "Cltt: " << cltt_ << "\n"
<< "DUID: " << (duid_?duid_->toText():"(none)") << "\n"
<< "Hardware addr: " << (hwaddr_?hwaddr_->toText(false):"(none)") << "\n"
<< "Subnet ID: " << subnet_id_ << "\n"
......@@ -551,13 +558,9 @@ std::string
Lease4::toText() const {
ostringstream stream;
stream << "Address: " << addr_ << "\n";
if (valid_lft_ != INFINITY_LFT) {
stream << "Valid life: " << valid_lft_ << "\n";
} else {
stream << "Valid life: " << "infinity" << "\n";
}
stream << "Cltt: " << cltt_ << "\n"
stream << "Address: " << addr_ << "\n"
<< "Valid life: " << lifetimeToText(valid_lft_) << "\n"
<< "Cltt: " << cltt_ << "\n"
<< "Hardware addr: " << (hwaddr_ ? hwaddr_->toText(false) : "(none)") << "\n"
<< "Client id: " << (client_id_ ? client_id_->toText() : "(none)") << "\n"
<< "Subnet ID: " << subnet_id_ << "\n"
......
......@@ -37,6 +37,15 @@ struct Lease : public isc::data::UserContext, public isc::data::CfgToElement {
/// @brief Infinity (means static, i.e. never expire)
static const uint32_t INFINITY_LFT = 0xffffffff;
/// @brief Print lifetime
///
/// This converts a lifetime to a string taking into account the
/// infinity special value.
///
/// @param lifetime lifetime to print
/// @return a string representing the finite value or "infinity"
static std::string lifetimeToText(uint32_t lifetime);
/// @brief Type of lease or pool
typedef enum {
TYPE_NA = 0, ///< the lease contains non-temporary IPv6 address
......
......@@ -174,8 +174,9 @@ tagged_statements = { {
"fqdn_fwd, fqdn_rev, hostname, "
"state, user_context "
"FROM lease4 "
"WHERE state != ? AND expire < ?"
" AND valid_lifetime != 4294967295 "
"WHERE state != ?"
" AND valid_lifetime != 4294967295"
" AND expire < ? "
"ORDER BY expire ASC "
"LIMIT ?"},
{MySqlLeaseMgr::GET_LEASE6,
......@@ -260,8 +261,9 @@ tagged_statements = { {
"hwaddr, hwtype, hwaddr_source, "
"state, user_context "
"FROM lease6 "
"WHERE state != ? AND expire < ?"
" AND valid_lifetime != 4294967295 "
"WHERE state != ?"
" AND valid_lifetime != 4294967295"
" AND expire < ? "
"ORDER BY expire ASC "
"LIMIT ?"},
{MySqlLeaseMgr::INSERT_LEASE4,
......
......@@ -157,7 +157,7 @@ PgSqlTaggedStatement tagged_statements[] = {
"fqdn_fwd, fqdn_rev, hostname, "
"state, user_context "
"FROM lease4 "
"WHERE state != $1 AND expire < $2 AND valid_lifetime != 4294967295 "
"WHERE state != $1 AND valid_lifetime != 4294967295 AND expire < $2 "
"ORDER BY expire "
"LIMIT $3"},
......@@ -261,7 +261,7 @@ PgSqlTaggedStatement tagged_statements[] = {
"hwaddr, hwtype, hwaddr_source, "
"state, user_context "
"FROM lease6 "
"WHERE state != $1 AND expire < $2 AND valid_lifetime != 4294967295 "
"WHERE state != $1 AND valid_lifetime != 4294967295 AND expire < $2 "
"ORDER BY expire "
"LIMIT $3"},
......@@ -496,9 +496,10 @@ public:
// Avoid overflow
uint32_t valid_lft = lease_->valid_lft_;
if (valid_lft == Lease::INFINITY_LFT) {
valid_lft = 0;
expire_str_ = convertToDatabaseTime(lease->cltt_, 0);
} else {
expire_str_ = convertToDatabaseTime(lease->cltt_, valid_lft);
}
expire_str_ = convertToDatabaseTime(lease->cltt_, valid_lft);
bind_array.add(expire_str_);
subnet_id_str_ = boost::lexical_cast<std::string>(lease->subnet_id_);
......@@ -556,9 +557,10 @@ public:
// Recover from overflow
uint32_t valid_lft = valid_lifetime_;
if (valid_lft == Lease::INFINITY_LFT) {
valid_lft = 0;
cltt_ = expire_;
} else {
cltt_ = expire_ - valid_lft;
}
cltt_ = expire_ - valid_lft;
getColumnValue(r, row, FQDN_FWD_COL, fqdn_fwd_);
......@@ -737,9 +739,10 @@ public:
// Avoid overflow
uint32_t valid_lft = lease_->valid_lft_;
if (valid_lft == Lease::INFINITY_LFT) {
valid_lft = 0;
expire_str_ = convertToDatabaseTime(lease->cltt_, 0);
} else {
expire_str_ = convertToDatabaseTime(lease->cltt_, valid_lft);
}
expire_str_ = convertToDatabaseTime(lease->cltt_, valid_lft);
bind_array.add(expire_str_);
subnet_id_str_ = boost::lexical_cast<std::string>(lease->subnet_id_);
......@@ -848,9 +851,10 @@ public:
// Recover from overflow
uint32_t valid_lft = valid_lifetime_;
if (valid_lft == Lease::INFINITY_LFT) {
valid_lft = 0;
cltt_ = expire_;
} else {
cltt_ = expire_ - valid_lft;
}
cltt_ = expire_ - valid_lft;
getColumnValue(r, row , SUBNET_ID_COL, subnet_id_);
......
......@@ -625,10 +625,10 @@ TEST_F(CqlLeaseMgrTest, getExpiredLeases4) {
testCqlGetExpiredLeases4();
}
/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv4
/// leases will never expire.
TEST_F(CqlLeaseMgrTest, staticLeases4) {
testStaticLeases4();
/// @brief Checks that DHCPv4 leases with infinite valid lifetime
/// will never expire.
TEST_F(CqlLeaseMgrTest, getNeverExpiredLeases4) {
testGetNeverExpiredLeases4();
}
/// @brief Check that expired reclaimed DHCPv4 leases are removed.
......@@ -780,10 +780,10 @@ TEST_F(CqlLeaseMgrTest, getExpiredLeases6) {
testCqlGetExpiredLeases6();
}
/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv6
/// leases will never expire.
TEST_F(CqlLeaseMgrTest, staticLeases6) {
testStaticLeases6();
/// @brief Checks that DHCPv6 leases with infinite valid lifetime
/// will never expire.
TEST_F(CqlLeaseMgrTest, getNeverExpiredLeases6) {
testGetNeverExpiredLeases6();
}
/// @brief Check that expired reclaimed DHCPv6 leases are removed.
......
......@@ -127,7 +127,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
// The times used in the next tests are deliberately restricted - we
// should be able to cope with valid lifetimes up to 0xffffffff.
// However, this will lead to overflows.
// @TODO: test overflow conditions as now the code was fixed.
// @TODO: test overflow conditions when code has been fixed.
lease->valid_lft_ = 7000;
lease->cltt_ = 234567;
lease->subnet_id_ = 37;
......@@ -254,7 +254,7 @@ GenericLeaseMgrTest::initializeLease6(std::string address) {
// The times used in the next tests are deliberately restricted - we
// should be able to cope with valid lifetimes up to 0xffffffff.
// However, this will lead to overflows.
// @TODO: test overflow conditions as now the code was fixed.
// @TODO: test overflow conditions when code has been fixed.
lease->preferred_lft_ = 7200;
lease->valid_lft_ = 7000;
lease->cltt_ = 234567;
......@@ -2253,7 +2253,7 @@ GenericLeaseMgrTest::testGetExpiredLeases6() {
}
void
GenericLeaseMgrTest::testStaticLeases4() {
GenericLeaseMgrTest::testGetNeverExpiredLeases4() {
// Get the leases to be used for the test.
vector<Lease4Ptr> leases = createLeases4();
Lease4Ptr lease = leases[1];
......@@ -2274,7 +2274,7 @@ GenericLeaseMgrTest::testStaticLeases4() {
}
void
GenericLeaseMgrTest::testStaticLeases6() {
GenericLeaseMgrTest::testGetNeverExpiredLeases6() {
// Get the leases to be used for the test.
vector<Lease6Ptr> leases = createLeases6();
Lease6Ptr lease = leases[1];
......
......@@ -353,13 +353,13 @@ public:
/// - reclaimed leases are not returned.
void testGetExpiredLeases6();
/// @brief Checks that the static (i.e. with infinite valid lifetime)
/// DHCPv4 leases will never expire.
void testStaticLeases4();
/// @brief Checks that DHCPv4 leases with infinite valid lifetime
/// will never expire.
void testGetNeverExpiredLeases4();
/// @brief Checks that the static (i.e. with infinite valid lifetime)
/// DHCPv4 leases will never expire.
void testStaticLeases6();
/// @brief Checks that DHCPv6 leases with infinite valid lifetime
/// will never expire.
void testGetNeverExpiredLeases6();
/// @brief Checks that declined IPv4 leases that have expired can be retrieved.
///
......
......@@ -591,10 +591,10 @@ TEST_F(MySqlLeaseMgrTest, getExpiredLeases4MultiThreading) {
testGetExpiredLeases4();
}
/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv4
/// leases will never expire.
TEST_F(MySqlLeaseMgrTest, staticLeases4) {
testStaticLeases4();
/// @brief Checks that DHCPv4 leases with infinite valid lifetime
/// will never expire.
TEST_F(MySqlLeaseMgrTest, getNeverExpiredLeases4) {
testGetNeverExpiredLeases4();
}
/// @brief Check that expired reclaimed DHCPv4 leases are removed.
......@@ -856,10 +856,10 @@ TEST_F(MySqlLeaseMgrTest, getExpiredLeases6MultiThreading) {
testGetExpiredLeases6();
}
/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv6
/// leases will never expire.
TEST_F(MySqlLeaseMgrTest, staticLeases6) {
testStaticLeases6();
/// @brief Checks that DHCPv6 leases with infinite valid lifetime
/// will never expire.
TEST_F(MySqlLeaseMgrTest, getNeverExpiredLeases6) {
testGetNeverExpiredLeases6();
}
/// @brief Check that expired reclaimed DHCPv6 leases are removed.
......
......@@ -545,10 +545,10 @@ TEST_F(PgSqlLeaseMgrTest, getExpiredLeases4MultiThreading) {
testGetExpiredLeases4();
}
/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv4
/// leases will never expire.
TEST_F(PgSqlLeaseMgrTest, staticLeases4) {
testStaticLeases4();
/// @brief Checks that DHCPv4 leases with infinite valid lifetime
/// will never expire.
TEST_F(PgSqlLeaseMgrTest, getNeverExpiredLeases4) {
testGetNeverExpiredLeases4();
}
/// @brief Check that expired reclaimed DHCPv4 leases are removed.
......@@ -810,10 +810,10 @@ TEST_F(PgSqlLeaseMgrTest, getExpiredLeases6MultiThreading) {
testGetExpiredLeases6();
}
/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv6
/// leases will never expire.
TEST_F(PgSqlLeaseMgrTest, staticLeases6) {
testStaticLeases6();
/// @brief Checks that DHCPv6 leases with infinite valid lifetime
/// will never expire.
TEST_F(PgSqlLeaseMgrTest, getNeverExpiredLeases6) {
testGetNeverExpiredLeases6();
}
/// @brief Check that expired reclaimed DHCPv6 leases are removed.
......
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