Commit 8174421f authored by Tomek Mrugalski's avatar Tomek Mrugalski 🛰
Browse files

[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
parent a3db6f08
......@@ -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
&quot;interface&quot; 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>
......
......@@ -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
......
......@@ -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);
......
......@@ -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();
......
......@@ -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
///
......
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