Commit d9c77f8a authored by Francis Dupont's avatar Francis Dupont

[5549a] Addressed comments

parent bf0d433c
......@@ -142,33 +142,6 @@
"relay": {
"ip-address": "192.168.1.1"
}
},
{
// This subnet is divided in two pools for unknown and
// known (i.e. which have a reservation) clients.
// The built-in KNOWN and UNKNOWN classes are set or not
// at host reservation lookup (KNOWN if this returns something,
// UNKNOWN if this finds nothing) and client classes depending
// on it are evaluated.
// This happens after subnet selection and before address
// allocation from pools.
"pools": [
{
"pool": "192.0.8.100 - 192.0.8.200",
"client-class": "UNKNOWN"
},
{
"pool": "192.0.9.100 - 192.0.9.200",
"client-class": "KNOWN"
}
],
"subnet": "192.0.8.0/23",
"reservations": [
{ "hw-address": "00:00:00:11:22:33", "hostname": "h1" },
{ "hw-address": "00:00:00:44:55:66", "hostname": "h4" },
{ "hw-address": "00:00:00:77:88:99", "hostname": "h7" },
{ "hw-address": "00:00:00:aa:bb:cc", "hostname": "ha" }
]
}
]
},
......
......@@ -126,7 +126,33 @@
} ],
"subnet": "192.0.4.0/23",
"interface": "ethY"
}
},
// This subnet is divided in two pools for unknown and known
// (i.e. which have a reservation) clients. The built-in KNOWN and
// UNKNOWN classes are set or not at host reservation lookup (KNOWN if
// this returns something, UNKNOWN if this finds nothing) and client
//classes depending on it are evaluated.
// This happens after subnet selection and before address allocation
//from pools.
{
"pools": [
{
"pool": "192.0.8.100 - 192.0.8.200",
"client-class": "UNKNOWN"
},
{
"pool": "192.0.9.100 - 192.0.9.200",
"client-class": "KNOWN"
}
],
"subnet": "192.0.8.0/23",
"reservations": [
{ "hw-address": "00:00:00:11:22:33", "hostname": "h1" },
{ "hw-address": "00:00:00:44:55:66", "hostname": "h4" },
{ "hw-address": "00:00:00:77:88:99", "hostname": "h7" },
{ "hw-address": "00:00:00:aa:bb:cc", "hostname": "ha" }
]
}
]
},
......
......@@ -139,33 +139,6 @@
"relay": {
"ip-address": "3000::1"
}
},
{
// This subnet is divided in two pools for unknown and
// known (i.e. which have a reservation) clients.
// The built-in KNOWN and UNKNOWN classes are set or not
// at host reservation lookup (KNOWN if this returns something,
// UNKNOWN if this finds nothing) and client classes depending
// on it are evaluated.
// This happens after subnet selection and before address/prefix
// allocation from [pd]pools.
"pools": [
{
"pool": "2001:db8:8::/64",
"client-class": "UNKNOWN"
},
{
"pool": "2001:db8:9::/64",
"client-class": "KNOWN"
}
],
"subnet": "2001:db8:8::/46",
"reservations": [
{ "hw-address": "00:00:00:11:22:33", "hostname": "h1" },
{ "hw-address": "00:00:00:44:55:66", "hostname": "h4" },
{ "hw-address": "00:00:00:77:88:99", "hostname": "h7" },
{ "hw-address": "00:00:00:aa:bb:cc", "hostname": "ha" }
]
}
]
},
......
......@@ -96,6 +96,32 @@
} ],
"subnet": "2001:db8:4::/64",
"interface": "ethY"
},
// This subnet is divided in two pools for unknown and known
// (i.e. which have a reservation) clients. The built-in KNOWN and
// UNKNOWN classes are set or not at host reservation lookup (KNOWN if
// this returns something, UNKNOWN if this finds nothing) and client
//classes depending on it are evaluated.
// This happens after subnet selection and before address allocation
//from pools.
{
"pools": [
{
"pool": "2001:db8:8::/64",
"client-class": "UNKNOWN"
},
{
"pool": "2001:db8:9::/64",
"client-class": "KNOWN"
}
],
"subnet": "2001:db8:8::/46",
"reservations": [
{ "hw-address": "00:00:00:11:22:33", "hostname": "h1" },
{ "hw-address": "00:00:00:44:55:66", "hostname": "h4" },
{ "hw-address": "00:00:00:77:88:99", "hostname": "h7" },
{ "hw-address": "00:00:00:aa:bb:cc", "hostname": "ha" }
]
}
]
......
......@@ -2241,7 +2241,7 @@ It is merely echoed by the server
<para>
In a similar way a pool can be constrained to serve only known
clients, i.e. clients which have a reservation, using the
build-n "KNOWN" or "UNKNOWN" classes. One can assign addresses
built-in "KNOWN" or "UNKNOWN" classes. One can assign addresses
to registered clients without giving a different address per
reservations, for instance when there is not enough available
addresses. The determination whether there is a reservation
......
......@@ -2239,7 +2239,7 @@ should include options from the isc option space:
<para>
In a similar way a pool can be constrained to serve only known
clients, i.e. clients which have a reservation, using the
build-n "KNOWN" or "UNKNOWN" classes. One can assign addresses
built-in "KNOWN" or "UNKNOWN" classes. One can assign addresses
to registered clients without giving a different address per
reservations, for instance when there is not enough available
addresses. The determination whether there is a reservation
......
......@@ -166,8 +166,14 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine,
// Set KNOWN builtin class if something was found, UNKNOWN if not.
if (!context_->hosts_.empty()) {
query->addClass("KNOWN");
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, DHCP4_CLASS_ASSIGNED)
.arg(query->getLabel())
.arg("KNOWN");
} else {
query->addClass("UNKNOWN");
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, DHCP4_CLASS_ASSIGNED)
.arg(query->getLabel())
.arg("UNKNOWN");
}
// Perform second pass of classification.
......
......@@ -856,6 +856,9 @@ public:
///
/// @note Second part of the classification.
///
/// Evaluate expressions of client classes: if it returns true the class
/// is added to the incoming packet.
///
/// @param pkt packet to be classified.
/// @param depend_on_known if false classes depending on the KNOWN or
/// UNKNOWN classes are skipped, if true only these classes are evaluated.
......
......@@ -2466,7 +2466,7 @@ TEST_F(Dhcpv4SrvTest, clientPoolClassify) {
EXPECT_FALSE(offer->getYiaddr().isV4Zero());
}
// Checks if the [UN]KNOWN built-in classes is indeed used for pool selection.
// Checks if the KNOWN built-in classes is indeed used for pool selection.
TEST_F(Dhcpv4SrvTest, clientPoolClassifyKnown) {
IfaceMgrTestConfig test_config(true);
IfaceMgr::instance().openSockets4();
......@@ -2518,6 +2518,66 @@ TEST_F(Dhcpv4SrvTest, clientPoolClassifyKnown) {
EXPECT_EQ("192.0.3.1", offer->getYiaddr().toText());
}
// Checks if the UNKNOWN built-in classes is indeed used for pool selection.
TEST_F(Dhcpv4SrvTest, clientPoolClassifyUnknown) {
IfaceMgrTestConfig test_config(true);
IfaceMgr::instance().openSockets4();
NakedDhcpv4Srv srv(0);
// This test configures 2 pools.
// The first one requires no reservation, the second does the opposite.
string config = "{ \"interfaces-config\": {"
" \"interfaces\": [ \"*\" ]"
"},"
"\"rebind-timer\": 2000, "
"\"renew-timer\": 1000, "
"\"subnet4\": [ "
"{ \"pools\": [ { "
" \"pool\": \"192.0.2.1 - 192.0.2.100\", "
" \"client-class\": \"UNKNOWN\" }, "
" { \"pool\": \"192.0.3.1 - 192.0.3.100\", "
" \"client-class\": \"KNOWN\" } ], "
" \"subnet\": \"192.0.0.0/16\", "
" \"reservations\": [ { "
" \"hw-address\": \"00:00:00:11:22:33\", "
" \"hostname\": \"foo.bar\" } ] } "
"],"
"\"valid-lifetime\": 4000 }";
ConstElementPtr json;
ASSERT_NO_THROW(json = parseDHCP4(config, true));
ConstElementPtr status;
EXPECT_NO_THROW(status = configureDhcp4Server(srv, json));
CfgMgr::instance().commit();
// check if returned status is OK
ASSERT_TRUE(status);
comment_ = config::parseAnswer(rcode_, status);
ASSERT_EQ(0, rcode_);
// 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"));
dis->setIface("eth0");
OptionPtr clientid = generateClientId();
dis->addOption(clientid);
// Set harware address / identifier
const HWAddr& hw = HWAddr::fromText("00:00:00:11:22:33");
HWAddrPtr hw_addr(new HWAddr(hw));
dis->setHWAddr(hw_addr);
// First pool requires no reservation so the second will be used
Pkt4Ptr offer = srv.processDiscover(dis);
ASSERT_TRUE(offer);
EXPECT_EQ(DHCPOFFER, offer->getType());
EXPECT_EQ("192.0.3.1", offer->getYiaddr().toText());
}
// Verifies last resort option 43 is backward compatible
TEST_F(Dhcpv4SrvTest, option43LastResort) {
IfaceMgrTestConfig test_config(true);
......
......@@ -383,8 +383,14 @@ Dhcpv6Srv::initContext(const Pkt6Ptr& pkt,
// Set KNOWN builtin class if something was found, UNKNOWN if not.
if (!ctx.hosts_.empty()) {
pkt->addClass("KNOWN");
LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_CLASS_ASSIGNED)
.arg(pkt->getLabel())
.arg("KNOWN");
} else {
pkt->addClass("UNKNOWN");
LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_CLASS_ASSIGNED)
.arg(pkt->getLabel())
.arg("UNKNOWN");
}
// Perform second pass of classification.
......
......@@ -678,6 +678,9 @@ protected:
///
/// @note Second part of the classification.
///
/// Evaluate expressions of client classes: if it returns true the class
/// is added to the incoming packet.
///
/// @param pkt packet to be classified.
/// @param depend_on_known if false classes depending on the KNOWN or
/// UNKNOWN classes are skipped, if true only these classes are evaluated.
......
......@@ -1031,7 +1031,7 @@ TEST_F(ClassifyTest, clientClassifyPool) {
" \"client-class\": \"xyzzy\" "
" } "
" ], "
" \"subnet\": \"2001:db8:2::/40\" "
" \"subnet\": \"2001:db8::/40\" "
" } "
"], "
"\"valid-lifetime\": 4000 }";
......@@ -1110,7 +1110,7 @@ TEST_F(ClassifyTest, clientClassifyPoolKnown) {
" \"client-class\": \"UNKNOWN\" "
" } "
" ], "
" \"subnet\": \"2001:db8:2::/40\", "
" \"subnet\": \"2001:db8::/40\", "
" \"reservations\": [ "
" { \"duid\": \"01:02:03:04\", \"hostname\": \"foo\" } ] "
" } "
......
......@@ -77,12 +77,6 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
<< getPosition("name", class_def_cfg) << ")");
}
// Let's try to parse the only-if-required flag
bool required = false;
if (class_def_cfg->contains("only-if-required")) {
required = getBoolean(class_def_cfg, "only-if-required");
}
// Parse matching expression
ExpressionPtr match_expr;
ConstElementPtr test_cfg = class_def_cfg->get("test");
......@@ -148,6 +142,12 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
// Parse user context
ConstElementPtr user_context = class_def_cfg->get("user-context");
// Let's try to parse the only-if-required flag
bool required = false;
if (class_def_cfg->contains("only-if-required")) {
required = getBoolean(class_def_cfg, "only-if-required");
}
// Let's try to parse the next-server field
IOAddress next_server("0.0.0.0");
if (class_def_cfg->contains("next-server")) {
......
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