Skip to content

GitLab

Commit 2aaedcde authored by Francis Dupont's avatar Francis Dupont
Browse files
Options

[5443] Moved to extra argument vs deconst

parent 1d7c8941
Hide whitespace changes
Inline Side-by-side
Showing with 91 additions and 56 deletions
src/bin/dhcp4/dhcp4_srv.cc
View file @ 2aaedcde
......@@ -483,11 +483,11 @@ Dhcpv4Srv::shutdown() {
}
isc::dhcp::Subnet4Ptr
Dhcpv4Srv::selectSubnet(Pkt4Ptr& query) {
Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query, bool& drop) const {
// DHCPv4-over-DHCPv6 is a special (and complex) case
if (query->isDhcp4o6()) {
return (selectSubnet4o6(query));
return (selectSubnet4o6(query, drop));
}
Subnet4Ptr subnet;
......@@ -570,13 +570,12 @@ Dhcpv4Srv::selectSubnet(Pkt4Ptr& query) {
}
// Callouts decided to drop the packet. It is a superset of the
// skip case so no subnet will be selected. For the caller to
// know it has to drop the packet the query pointer is cleared.
// skip case so no subnet will be selected.
if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
DHCP4_HOOK_SUBNET4_SELECT_DROP)
.arg(query->getLabel());
query.reset();
drop = true;
return (Subnet4Ptr());
}
......@@ -605,7 +604,7 @@ Dhcpv4Srv::selectSubnet(Pkt4Ptr& query) {
}
isc::dhcp::Subnet4Ptr
Dhcpv4Srv::selectSubnet4o6(Pkt4Ptr& query) {
Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query, bool& drop) const {
Subnet4Ptr subnet;
......@@ -683,13 +682,12 @@ Dhcpv4Srv::selectSubnet4o6(Pkt4Ptr& query) {
}
// Callouts decided to drop the packet. It is a superset of the
// skip case so no subnet will be selected. For the caller to
// know it has to drop the packet the query pointer is cleared.
// skip case so no subnet will be selected.
if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
DHCP4_HOOK_SUBNET4_SELECT_DROP)
.arg(query->getLabel());
query.reset();
drop = true;
return (Subnet4Ptr());
}
......@@ -2341,10 +2339,11 @@ Pkt4Ptr
Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
sanityCheck(discover, FORBIDDEN);
Dhcpv4Exchange ex(alloc_engine_, discover, selectSubnet(discover));
bool drop = false;
Dhcpv4Exchange ex(alloc_engine_, discover, selectSubnet(discover, drop));
// Stop here if selectSubnet decided to drop the packet
if (!discover) {
if (drop) {
return (Pkt4Ptr());
}
......@@ -2398,10 +2397,11 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
/// @todo Uncomment this (see ticket #3116)
/// sanityCheck(request, MANDATORY);
Dhcpv4Exchange ex(alloc_engine_, request, selectSubnet(request));
bool drop = false;
Dhcpv4Exchange ex(alloc_engine_, request, selectSubnet(request, drop));
// Stop here if selectSubnet decided to drop the packet
if (!request) {
if (drop) {
return (Pkt4Ptr());
}
......@@ -2704,10 +2704,11 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
// DHCPINFORM MUST not include server identifier.
sanityCheck(inform, FORBIDDEN);
Dhcpv4Exchange ex(alloc_engine_, inform, selectSubnet(inform));
bool drop = false;
Dhcpv4Exchange ex(alloc_engine_, inform, selectSubnet(inform, drop));
// Stop here if selectSubnet decided to drop the packet
if (!inform) {
if (drop) {
return (Pkt4Ptr());
}
......@@ -2747,7 +2748,7 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
}
bool
Dhcpv4Srv::accept(Pkt4Ptr& query) {
Dhcpv4Srv::accept(const Pkt4Ptr& query) const {
// Check that the message type is accepted by the server. We rely on the
// function called to log a message if needed.
if (!acceptMessageType(query)) {
......@@ -2775,7 +2776,7 @@ Dhcpv4Srv::accept(Pkt4Ptr& query) {
}
bool
Dhcpv4Srv::acceptDirectRequest(Pkt4Ptr& pkt) {
Dhcpv4Srv::acceptDirectRequest(const Pkt4Ptr& pkt) const {
// Accept all relayed messages.
if (pkt->isRelayed()) {
return (true);
......@@ -2802,8 +2803,9 @@ Dhcpv4Srv::acceptDirectRequest(Pkt4Ptr& pkt) {
// we validate the message type prior to calling this function.
return (false);
}
bool result = (!pkt->getLocalAddr().isV4Bcast() || selectSubnet(pkt));
if (!pkt) {
bool drop = false;
bool result = (!pkt->getLocalAddr().isV4Bcast() || selectSubnet(pkt, drop));
if (drop) {
// The packet must be dropped.
return (false);
}
......
src/bin/dhcp4/dhcp4_srv.h
View file @ 2aaedcde
......@@ -348,7 +348,7 @@ protected:
///
/// @return true if the message should be further processed, or false if
/// the message should be discarded.
bool accept(Pkt4Ptr& query);
bool accept(const Pkt4Ptr& query) const;
/// @brief Check if a message sent by directly connected client should be
/// accepted or discarded.
......@@ -377,7 +377,7 @@ protected:
///
/// @return true if message is accepted for further processing, false
/// otherwise.
bool acceptDirectRequest(Pkt4Ptr& query);
bool acceptDirectRequest(const Pkt4Ptr& query) const;
/// @brief Check if received message type is valid for the server to
/// process.
......@@ -766,19 +766,19 @@ protected:
/// @brief Selects a subnet for a given client's packet.
///
/// When the packet has to be dropped the query pointer is cleared
///
/// @param query client's message
/// @param drop if it is true the packet will be dropped
/// @return selected subnet (or NULL if no suitable subnet was found)
isc::dhcp::Subnet4Ptr selectSubnet(Pkt4Ptr& query);
isc::dhcp::Subnet4Ptr selectSubnet(const Pkt4Ptr& query,
bool& drop) const;
/// @brief Selects a subnet for a given client's DHCP4o6 packet.
///
/// When the packet has to be dropped the query pointer is cleared
///
/// @param query client's message
/// @param drop if it is true the packet will be dropped
/// @return selected subnet (or NULL if no suitable subnet was found)
isc::dhcp::Subnet4Ptr selectSubnet4o6(Pkt4Ptr& query);
isc::dhcp::Subnet4Ptr selectSubnet4o6(const Pkt4Ptr& query,
bool& drop) const;
/// indicates if shutdown is in progress. Setting it to true will
/// initiate server shutdown procedure.
......
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
View file @ 2aaedcde
......@@ -2303,19 +2303,23 @@ TEST_F(Dhcpv4SrvTest, clientClassify) {
// This discover does not belong to foo class, so it will not
// be serviced
EXPECT_FALSE(srv_.selectSubnet(dis));
bool drop = false;
EXPECT_FALSE(srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// 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, drop));
EXPECT_FALSE(drop);
// Let's add it to matching class.
dis->addClass("foo");
// This time it should work
EXPECT_TRUE(srv_.selectSubnet(dis));
EXPECT_TRUE(srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
}
// Verifies last resort option 43 is backward compatible
......@@ -3493,31 +3497,38 @@ 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));
bool drop = false;
EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// 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, drop));
EXPECT_FALSE(drop);
// 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, drop));
EXPECT_FALSE(drop);
// 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, drop));
EXPECT_FALSE(drop);
// 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, drop));
EXPECT_FALSE(drop);
// 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, drop));
EXPECT_FALSE(drop);
}
// Checks if relay IP address specified in the relay-info structure can be
......@@ -3573,12 +3584,15 @@ 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));
bool drop = false;
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// 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, drop));
EXPECT_FALSE(drop);
}
// Checks if a RAI link selection sub-option works as expected
......@@ -3644,16 +3658,20 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
// This is just a sanity check, we're using regular method: ciaddr 192.0.3.1
// belongs to the second subnet, so it is selected
dis->setGiaddr(IOAddress("192.0.3.1"));
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
bool drop = false;
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Setup a relay override for the first subnet as it has a high precedence
dis->setGiaddr(IOAddress("192.0.5.1"));
EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Put a RAI to select back the second subnet as it has
// the highest precedence
dis->addOption(rai);
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Subnet select option has a lower precedence
OptionDefinitionPtr sbnsel_def = LibDHCP::getOptionDef(DHCP4_OPTION_SPACE,
......@@ -3663,7 +3681,8 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
ASSERT_TRUE(sbnsel);
sbnsel->writeAddress(IOAddress("192.0.2.3"));
dis->addOption(sbnsel);
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
dis->delOption(DHO_SUBNET_SELECTION);
// Check client-classification still applies
......@@ -3675,10 +3694,12 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
rai->addOption(ols);
dis->addOption(rai);
// Note it shall fail (vs. try the next criterion).
EXPECT_FALSE(srv_.selectSubnet(dis));
EXPECT_FALSE(srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Add the packet to the class and check again: now it shall succeed
dis->addClass("foo");
EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis));
EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Check it fails with a bad address in the sub-option
IOAddress addr_bad("10.0.0.1");
......@@ -3688,7 +3709,8 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
dis->delOption(DHO_DHCP_AGENT_OPTIONS);
rai->addOption(ols);
dis->addOption(rai);
EXPECT_FALSE(srv_.selectSubnet(dis));
EXPECT_FALSE(srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
}
// Checks if a subnet selection option works as expected
......@@ -3749,28 +3771,35 @@ TEST_F(Dhcpv4SrvTest, subnetSelect) {
// This is just a sanity check, we're using regular method: ciaddr 192.0.3.1
// belongs to the second subnet, so it is selected
dis->setGiaddr(IOAddress("192.0.3.1"));
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
bool drop = false;
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Setup a relay override for the first subnet as it has a high precedence
dis->setGiaddr(IOAddress("192.0.5.1"));
EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Put a subnet select option to select back the second subnet as
// it has the second highest precedence
dis->addOption(sbnsel);
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Check client-classification still applies
sbnsel->writeAddress(IOAddress("192.0.4.2"));
// Note it shall fail (vs. try the next criterion).
EXPECT_FALSE(srv_.selectSubnet(dis));
EXPECT_FALSE(srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Add the packet to the class and check again: now it shall succeed
dis->addClass("foo");
EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis));
EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
// Check it fails with a bad address in the sub-option
sbnsel->writeAddress(IOAddress("10.0.0.1"));
EXPECT_FALSE(srv_.selectSubnet(dis));
EXPECT_FALSE(srv_.selectSubnet(dis, drop));
EXPECT_FALSE(drop);
}
// This test verifies that the direct message is dropped when it has been
......
src/bin/dhcp4/tests/dhcp4_test_utils.cc
View file @ 2aaedcde
......@@ -631,8 +631,12 @@ Dhcpv4SrvTest::configure(const std::string& config, NakedDhcpv4Srv& srv,
}
Dhcpv4Exchange
Dhcpv4SrvTest::createExchange(Pkt4Ptr& query) {
return (Dhcpv4Exchange(srv_.alloc_engine_, query, srv_.selectSubnet(query)));
Dhcpv4SrvTest::createExchange(const Pkt4Ptr& query) {
bool drop = false;
Dhcpv4Exchange ex(srv_.alloc_engine_, query,
srv_.selectSubnet(query, drop));
EXPECT_FALSE(drop);
return (ex);
}
void
......
src/bin/dhcp4/tests/dhcp4_test_utils.h
View file @ 2aaedcde
......@@ -434,7 +434,7 @@ public:
uint8_t pkt_type, const std::string& stat_name);
/// @brief Create @c Dhcpv4Exchange from client's query.
Dhcpv4Exchange createExchange(Pkt4Ptr& query);
Dhcpv4Exchange createExchange(const Pkt4Ptr& query);
/// @brief Performs 4-way exchange to obtain new lease.
///
......
src/bin/dhcp4/tests/fqdn_unittest.cc
View file @ 2aaedcde
......@@ -396,7 +396,7 @@ public:
// Test that server generates the appropriate FQDN option in response to
// client's FQDN option.
void testProcessFqdn(Pkt4Ptr& query, const uint8_t exp_flags,
void testProcessFqdn(const Pkt4Ptr& query, const uint8_t exp_flags,
const std::string& exp_domain_name,
Option4ClientFqdn::DomainNameType
exp_domain_type = Option4ClientFqdn::FULL) {
......@@ -524,7 +524,7 @@ public:
/// packet must contain the hostname option
///
/// @return a pointer to the hostname option constructed by the server
OptionStringPtr processHostname(Pkt4Ptr& query,
OptionStringPtr processHostname(const Pkt4Ptr& query,
bool must_have_host = true) {
if (!getHostnameOption(query) && must_have_host) {
ADD_FAILURE() << "Hostname option not carried in the query";
......
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