BIND 10 trac2681, updated. 5950991881a51d5b88ec6234d310adda43e11e8a [2681] Address review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Feb 11 13:42:35 UTC 2013
The branch, trac2681 has been updated
via 5950991881a51d5b88ec6234d310adda43e11e8a (commit)
from 964547642571d9146219cf2f9d6e08fa03970a81 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 5950991881a51d5b88ec6234d310adda43e11e8a
Author: Stephen Morris <stephen at isc.org>
Date: Mon Feb 11 13:42:18 2013 +0000
[2681] Address review comments
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_messages.mes | 4 +-
src/bin/dhcp4/dhcp4_srv.cc | 12 ++-
src/bin/dhcp6/dhcp6_messages.mes | 2 +-
src/lib/dhcpsrv/alloc_engine.cc | 218 +++++++++++++++++++-------------------
4 files changed, 122 insertions(+), 114 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index c57d811..247d6c3 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -1,4 +1,4 @@
-# Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
#
# Permission to use, copy, modify, and/or distribute this software for any
# purpose with or without fee is hereby granted, provided that the above
@@ -94,7 +94,7 @@ server is about to open sockets on the specified port.
The IPv4 DHCP server has received a packet that it is unable to
interpret. The reason why the packet is invalid is included in the message.
-% DHCP4_PACKET_PROCESS_FAIL failed to process received packet: %1
+% DHCP4_PACKET_PROCESS_FAIL failed to process packet received from %1: %2
This is a general catch-all message indicating that the processing of a
received packet failed. The reason is given in the message. The server
will not send a response but will instead ignore the packet.
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 3871b06..c910ff8 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -173,8 +173,16 @@ Dhcpv4Srv::run() {
// as a debug message because debug is disabled by default -
// it prevents a DDOS attack based on the sending of problem
// packets.)
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
- DHCP4_PACKET_PROCESS_FAIL).arg(e.what());
+ if (dhcp4_logger.isDebugEnabled(DBG_DHCP4_BASIC)) {
+ std::string source = "unknown";
+ HWAddrPtr hwptr = query->getHWAddr();
+ if (hwptr) {
+ source = hwptr->toText();
+ }
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
+ DHCP4_PACKET_PROCESS_FAIL)
+ .arg(source).arg(e.what());
+ }
}
if (rsp) {
diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes
index 2b9b675..a2fee47 100644
--- a/src/bin/dhcp6/dhcp6_messages.mes
+++ b/src/bin/dhcp6/dhcp6_messages.mes
@@ -1,4 +1,4 @@
-# Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
#
# Permission to use, copy, modify, and/or distribute this software for any
# purpose with or without fee is hereby granted, provided that the above
diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 90d56a0..bcb7851 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -170,62 +170,62 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
const IOAddress& hint,
bool fake_allocation /* = false */ ) {
- // That check is not necessary. We create allocator in AllocEngine
- // constructor
- if (!allocator_) {
- isc_throw(InvalidOperation, "No allocator selected");
- }
+ try {
+ // That check is not necessary. We create allocator in AllocEngine
+ // constructor
+ if (!allocator_) {
+ isc_throw(InvalidOperation, "No allocator selected");
+ }
- // check if there's existing lease for that subnet/duid/iaid combination.
- Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(*duid, iaid, subnet->getID());
- if (existing) {
- // we have a lease already. This is a returning client, probably after
- // his reboot.
- return (existing);
- }
+ // check if there's existing lease for that subnet/duid/iaid combination.
+ Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(*duid, iaid, subnet->getID());
+ if (existing) {
+ // we have a lease already. This is a returning client, probably after
+ // his reboot.
+ return (existing);
+ }
- // check if the hint is in pool and is available
- if (subnet->inPool(hint)) {
- existing = LeaseMgrFactory::instance().getLease6(hint);
- if (!existing) {
- /// @todo: check if the hint is reserved once we have host support
- /// implemented
+ // check if the hint is in pool and is available
+ if (subnet->inPool(hint)) {
+ existing = LeaseMgrFactory::instance().getLease6(hint);
+ if (!existing) {
+ /// @todo: check if the hint is reserved once we have host support
+ /// implemented
- // the hint is valid and not currently used, let's create a lease for it
- Lease6Ptr lease = createLease6(subnet, duid, iaid, hint, fake_allocation);
+ // the hint is valid and not currently used, let's create a lease for it
+ Lease6Ptr lease = createLease6(subnet, duid, iaid, hint, fake_allocation);
- // It can happen that the lease allocation failed (we could have lost
- // the race condition. That means that the hint is lo longer usable and
- // we need to continue the regular allocation path.
- if (lease) {
- return (lease);
- }
- } else {
- if (existing->expired()) {
- return (reuseExpiredLease(existing, subnet, duid, iaid,
- fake_allocation));
- }
+ // It can happen that the lease allocation failed (we could have lost
+ // the race condition. That means that the hint is lo longer usable and
+ // we need to continue the regular allocation path.
+ if (lease) {
+ return (lease);
+ }
+ } else {
+ if (existing->expired()) {
+ return (reuseExpiredLease(existing, subnet, duid, iaid,
+ fake_allocation));
+ }
+ }
}
- }
- // Hint is in the pool but is not available. Search the pool until first of
- // the following occurs:
- // - we find a free address
- // - we find an address for which the lease has expired
- // - we exhaust number of tries
- //
- // @todo: Current code does not handle pool exhaustion well. It will be
- // improved. Current problems:
- // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
- // 10 addresses), we will iterate over it 100 times before giving up
- // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
- // 3. the whole concept of infinite attempts is just asking for infinite loop
- // We may consider some form or reference counting (this pool has X addresses
- // left), but this has one major problem. We exactly control allocation
- // moment, but we currently do not control expiration time at all
+ // Hint is in the pool but is not available. Search the pool until first of
+ // the following occurs:
+ // - we find a free address
+ // - we find an address for which the lease has expired
+ // - we exhaust number of tries
+ //
+ // @todo: Current code does not handle pool exhaustion well. It will be
+ // improved. Current problems:
+ // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
+ // 10 addresses), we will iterate over it 100 times before giving up
+ // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
+ // 3. the whole concept of infinite attempts is just asking for infinite loop
+ // We may consider some form or reference counting (this pool has X addresses
+ // left), but this has one major problem. We exactly control allocation
+ // moment, but we currently do not control expiration time at all
- try {
unsigned int i = attempts_;
do {
IOAddress candidate = allocator_->pickAddress(subnet, duid, hint);
@@ -277,82 +277,82 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
const IOAddress& hint,
bool fake_allocation /* = false */ ) {
- // Allocator is always created in AllocEngine constructor and there is
- // currently no other way to set it, so that check is not really necessary.
- if (!allocator_) {
- isc_throw(InvalidOperation, "No allocator selected");
- }
-
- // Check if there's existing lease for that subnet/clientid/hwaddr combination.
- Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(hwaddr->hwaddr_, subnet->getID());
- if (existing) {
- // We have a lease already. This is a returning client, probably after
- // its reboot.
- existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
- if (existing) {
- return (existing);
+ try {
+ // Allocator is always created in AllocEngine constructor and there is
+ // currently no other way to set it, so that check is not really necessary.
+ if (!allocator_) {
+ isc_throw(InvalidOperation, "No allocator selected");
}
- // If renewal failed (e.g. the lease no longer matches current configuration)
- // let's continue the allocation process
- }
-
- if (clientid) {
- existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
+ // Check if there's existing lease for that subnet/clientid/hwaddr combination.
+ Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(hwaddr->hwaddr_, subnet->getID());
if (existing) {
- // we have a lease already. This is a returning client, probably after
+ // We have a lease already. This is a returning client, probably after
// its reboot.
existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
- // @todo: produce a warning. We haven't found him using MAC address, but
- // we found him using client-id
if (existing) {
return (existing);
}
+
+ // If renewal failed (e.g. the lease no longer matches current configuration)
+ // let's continue the allocation process
}
- }
- // check if the hint is in pool and is available
- if (subnet->inPool(hint)) {
- existing = LeaseMgrFactory::instance().getLease4(hint);
- if (!existing) {
- /// @todo: Check if the hint is reserved once we have host support
- /// implemented
+ if (clientid) {
+ existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
+ if (existing) {
+ // we have a lease already. This is a returning client, probably after
+ // its reboot.
+ existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
+ // @todo: produce a warning. We haven't found him using MAC address, but
+ // we found him using client-id
+ if (existing) {
+ return (existing);
+ }
+ }
+ }
- // The hint is valid and not currently used, let's create a lease for it
- Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint, fake_allocation);
+ // check if the hint is in pool and is available
+ if (subnet->inPool(hint)) {
+ existing = LeaseMgrFactory::instance().getLease4(hint);
+ if (!existing) {
+ /// @todo: Check if the hint is reserved once we have host support
+ /// implemented
- // It can happen that the lease allocation failed (we could have lost
- // the race condition. That means that the hint is lo longer usable and
- // we need to continue the regular allocation path.
- if (lease) {
- return (lease);
- }
- } else {
- if (existing->expired()) {
- return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
- fake_allocation));
- }
+ // The hint is valid and not currently used, let's create a lease for it
+ Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint, fake_allocation);
+
+ // It can happen that the lease allocation failed (we could have lost
+ // the race condition. That means that the hint is lo longer usable and
+ // we need to continue the regular allocation path.
+ if (lease) {
+ return (lease);
+ }
+ } else {
+ if (existing->expired()) {
+ return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
+ fake_allocation));
+ }
+ }
}
- }
- // Hint is in the pool but is not available. Search the pool until first of
- // the following occurs:
- // - we find a free address
- // - we find an address for which the lease has expired
- // - we exhaust the number of tries
- //
- // @todo: Current code does not handle pool exhaustion well. It will be
- // improved. Current problems:
- // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
- // 10 addresses), we will iterate over it 100 times before giving up
- // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
- // 3. the whole concept of infinite attempts is just asking for infinite loop
- // We may consider some form or reference counting (this pool has X addresses
- // left), but this has one major problem. We exactly control allocation
- // moment, but we currently do not control expiration time at all
+ // Hint is in the pool but is not available. Search the pool until first of
+ // the following occurs:
+ // - we find a free address
+ // - we find an address for which the lease has expired
+ // - we exhaust the number of tries
+ //
+ // @todo: Current code does not handle pool exhaustion well. It will be
+ // improved. Current problems:
+ // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
+ // 10 addresses), we will iterate over it 100 times before giving up
+ // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
+ // 3. the whole concept of infinite attempts is just asking for infinite loop
+ // We may consider some form or reference counting (this pool has X addresses
+ // left), but this has one major problem. We exactly control allocation
+ // moment, but we currently do not control expiration time at all
- try {
unsigned int i = attempts_;
do {
IOAddress candidate = allocator_->pickAddress(subnet, clientid, hint);
More information about the bind10-changes
mailing list