BIND 10 trac3322, updated. 8174421f666416dd3cb6a2053b3b2dd6db4d32b1 [3322] Changes after review:
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Feb 19 16:54:59 UTC 2014
The branch, trac3322 has been updated
via 8174421f666416dd3cb6a2053b3b2dd6db4d32b1 (commit)
from a3db6f08b0321dcfbd7539e07764f0c124ac19af (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 8174421f666416dd3cb6a2053b3b2dd6db4d32b1
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Wed Feb 19 17:54:37 2014 +0100
[3322] Changes after review:
- Unit-tests for DHCPv4 now use configure() method.
- Typo fixed in DHCPv6 unit-tests.
- Removed, updated comments in cfgmgr
- Guide updated
-----------------------------------------------------------------------
Summary of changes:
doc/guide/bind10-guide.xml | 18 +++++-----
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 55 ++++++++---------------------
src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 2 +-
src/lib/dhcpsrv/cfgmgr.cc | 4 +--
src/lib/dhcpsrv/cfgmgr.h | 12 ++++---
5 files changed, 35 insertions(+), 56 deletions(-)
-----------------------------------------------------------------------
diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index 4a65ba1..e3319f0 100644
--- a/doc/guide/bind10-guide.xml
+++ b/doc/guide/bind10-guide.xml
@@ -5545,17 +5545,17 @@ should include options from the isc option space:
The DHCPv6 server may receive requests from local (connected to the
same subnet as the server) and remote (connecting via relays) clients.
As server may have many subnet configurations defined, it must select
- appropriate subnet for a given request. To do this, the server first
- checks if there is only one subnet defined and source of the packet is
- link-local. If this is the case, the server assumes that the only
- subnet defined is local and client is indeed connected to it. This
- check simplifies small deployments.
+ appropriate subnet for a given request.
</para>
<para>
- If there are two or more subnets defined, the server can not assume
- which of those (if any) subnets are local. Therefore an optional
- "interface" parameter is available within a subnet definition to
- designate that a given subnet is local, i.e. reachable directly over
+ The server can not assume which of configured subnets are local. It is
+ possible in IPv4, where there is reasonable expectation that the
+ server will have a (global) IPv4 address configured on the interface,
+ and can use that information to detect whether a subnet is local or
+ not. That assumption is not true in IPv6, as the DHCPv6 must be able
+ to operate with having link-local addresses only. Therefore an optional
+ "interface" parameter is available within a subnet definition
+ to designate that a given subnet is local, i.e. reachable directly over
specified interface. For example the server that is intended to serve
a local subnet over eth0 may be configured as follows:
<screen>
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index cdc9e1f..618208b 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -3314,10 +3314,6 @@ TEST_F(Dhcpv4SrvTest, clientClassification) {
// .clientClassification above.
TEST_F(Dhcpv4SrvTest, clientClassify2) {
- NakedDhcpv4Srv srv(0);
-
- ConstElementPtr status;
-
// This test configures 2 subnets. We actually only need the
// first one, but since there's still this ugly hack that picks
// the pool if there is only one, we must use more than one
@@ -3340,15 +3336,9 @@ TEST_F(Dhcpv4SrvTest, clientClassify2) {
"],"
"\"valid-lifetime\": 4000 }";
- ElementPtr json = Element::fromJSON(config);
-
- EXPECT_NO_THROW(status = configureDhcp4Server(srv, json));
-
- // check if returned status is OK
- ASSERT_TRUE(status);
- comment_ = config::parseAnswer(rcode_, status);
- ASSERT_EQ(0, rcode_);
+ ASSERT_NO_THROW(configure(config));
+ // Create a simple packet that we'll use for classification
Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
dis->setRemoteAddr(IOAddress("192.0.2.1"));
dis->setCiaddr(IOAddress("192.0.2.1"));
@@ -3358,27 +3348,25 @@ TEST_F(Dhcpv4SrvTest, clientClassify2) {
// This discover does not belong to foo class, so it will not
// be serviced
- EXPECT_FALSE(srv.selectSubnet(dis));
+ EXPECT_FALSE(srv_.selectSubnet(dis));
// Let's add the packet to bar class and try again.
dis->addClass("bar");
// Still not supported, because it belongs to wrong class.
- EXPECT_FALSE(srv.selectSubnet(dis));
+ EXPECT_FALSE(srv_.selectSubnet(dis));
// Let's add it to maching class.
dis->addClass("foo");
// This time it should work
- EXPECT_TRUE(srv.selectSubnet(dis));
+ EXPECT_TRUE(srv_.selectSubnet(dis));
}
// Checks if relay IP address specified in the relay-info structure in
// subnet4 is being used properly.
TEST_F(Dhcpv4SrvTest, relayOverride) {
- NakedDhcpv4Srv srv(0);
-
// We have 2 subnets defined. Note that both have a relay address
// defined. Both are not belonging to the subnets. That is
// important, because if the relay belongs to the subnet, there's
@@ -3401,12 +3389,7 @@ TEST_F(Dhcpv4SrvTest, relayOverride) {
"\"valid-lifetime\": 4000 }";
// Use this config to set up the server
- ElementPtr json = Element::fromJSON(config);
- ConstElementPtr status;
- EXPECT_NO_THROW(status = configureDhcp4Server(srv, json));
- ASSERT_TRUE(status);
- comment_ = config::parseAnswer(rcode_, status);
- ASSERT_EQ(0, rcode_);
+ ASSERT_NO_THROW(configure(config));
// Let's get the subnet configuration objects
const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4();
@@ -3429,41 +3412,37 @@ TEST_F(Dhcpv4SrvTest, relayOverride) {
// This is just a sanity check, we're using regular method: ciaddr 192.0.2.1
// belongs to the first subnet, so it is selected
dis->setGiaddr(IOAddress("192.0.2.1"));
- EXPECT_TRUE(subnet1 == srv.selectSubnet(dis));
+ EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
// Relay belongs to the second subnet, so it should be selected.
dis->setGiaddr(IOAddress("192.0.3.1"));
- EXPECT_TRUE(subnet2 == srv.selectSubnet(dis));
+ EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
// Now let's check if the relay override for the first subnets works
dis->setGiaddr(IOAddress("192.0.5.1"));
- EXPECT_TRUE(subnet1 == srv.selectSubnet(dis));
+ EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
// The same check for the second subnet...
dis->setGiaddr(IOAddress("192.0.5.2"));
- EXPECT_TRUE(subnet2 == srv.selectSubnet(dis));
+ EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
// And finally, let's check if mis-matched relay address will end up
// in not selecting a subnet at all
dis->setGiaddr(IOAddress("192.0.5.3"));
- EXPECT_FALSE(srv.selectSubnet(dis));
+ EXPECT_FALSE(srv_.selectSubnet(dis));
// Finally, check that the relay override works only with relay address
// (GIADDR) and does not affect client address (CIADDR)
dis->setGiaddr(IOAddress("0.0.0.0"));
dis->setHops(0);
dis->setCiaddr(IOAddress("192.0.5.1"));
- EXPECT_FALSE(srv.selectSubnet(dis));
+ EXPECT_FALSE(srv_.selectSubnet(dis));
}
// Checks if relay IP address specified in the relay-info structure can be
// used together with client-classification.
TEST_F(Dhcpv4SrvTest, relayOverrideAndClientClass) {
- NakedDhcpv4Srv srv(0);
-
- ConstElementPtr status;
-
// This test configures 2 subnets. They both are on the same link, so they
// have the same relay-ip address. Furthermore, the first subnet is
// reserved for clients that belong to class "foo".
@@ -3486,11 +3465,7 @@ TEST_F(Dhcpv4SrvTest, relayOverrideAndClientClass) {
"\"valid-lifetime\": 4000 }";
// Use this config to set up the server
- ElementPtr json = Element::fromJSON(config);
- EXPECT_NO_THROW(status = configureDhcp4Server(srv, json));
- ASSERT_TRUE(status);
- comment_ = config::parseAnswer(rcode_, status);
- ASSERT_EQ(0, rcode_);
+ ASSERT_NO_THROW(configure(config));
const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4();
ASSERT_EQ(2, subnets->size());
@@ -3514,12 +3489,12 @@ TEST_F(Dhcpv4SrvTest, relayOverrideAndClientClass) {
// subnet[0], even though the relay-ip matches. It should be accepted in
// subnet[1], because the subnet matches and there are no class
// requirements.
- EXPECT_TRUE(subnet2 == srv.selectSubnet(dis));
+ EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
// Now let's add this packet to class foo and recheck. This time it should
// be accepted in the first subnet, because both class and relay-ip match.
dis->addClass("foo");
- EXPECT_TRUE(subnet1 == srv.selectSubnet(dis));
+ EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
}
// This test verifies that the direct message is dropped when it has been
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 1a8e12b..0a7f36b 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -1807,7 +1807,7 @@ TEST_F(Dhcpv6SrvTest, clientClassify2) {
}
// Checks if relay IP address specified in the relay-info structure in
-// subnet4 is being used properly.
+// subnet6 is being used properly.
TEST_F(Dhcpv6SrvTest, relayOverride) {
NakedDhcpv6Srv srv(0);
diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc
index 1f7be14..ce9e419 100644
--- a/src/lib/dhcpsrv/cfgmgr.cc
+++ b/src/lib/dhcpsrv/cfgmgr.cc
@@ -155,7 +155,7 @@ CfgMgr::getSubnet6(const std::string& iface,
Subnet6Ptr
CfgMgr::getSubnet6(const isc::asiolink::IOAddress& hint,
const isc::dhcp::ClientClasses& classes,
- bool relay) {
+ const bool relay) {
// If there's only one subnet configured, let's just use it
// The idea is to keep small deployments easy. In a small network - one
@@ -240,7 +240,7 @@ void CfgMgr::addSubnet6(const Subnet6Ptr& subnet) {
Subnet4Ptr
CfgMgr::getSubnet4(const isc::asiolink::IOAddress& hint,
const isc::dhcp::ClientClasses& classes,
- bool relay /* = false */) const {
+ bool relay) const {
// Iterate over existing subnets to find a suitable one for the
// given address.
for (Subnet4Collection::const_iterator subnet = subnets4_.begin();
diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h
index 9f4917e..89ae4f4 100644
--- a/src/lib/dhcpsrv/cfgmgr.h
+++ b/src/lib/dhcpsrv/cfgmgr.h
@@ -169,10 +169,14 @@ public:
/// If there are any classes specified in a subnet, that subnet
/// will be selected only if the client belongs to appropriate class.
///
+ /// @note The client classification is checked before any relay
+ /// information checks are conducted.
+ ///
/// If relay is true then relay info overrides (i.e. value the sysadmin
- /// can configure in Dhcp4/subnet6[X]/relay/ip-address) can be used.
- /// That is true only for relays. Those overrides must not be used
- /// for client address or for client hints. They are for giaddr only.
+ /// can configure in Dhcp6/subnet6[X]/relay/ip-address) can be used.
+ /// That is applicable only for relays. Those overrides must not be used
+ /// for client address or for client hints. They are for link-addr field
+ /// in the RELAY_FORW message only.
///
/// @param hint an address that belongs to a searched subnet
/// @param classes classes the client belongs to
@@ -181,7 +185,7 @@ public:
/// @return a subnet object (or NULL if no suitable match was fount)
Subnet6Ptr getSubnet6(const isc::asiolink::IOAddress& hint,
const isc::dhcp::ClientClasses& classes,
- bool relay = false);
+ const bool relay = false);
/// @brief get IPv6 subnet by interface name
///
More information about the bind10-changes
mailing list