Commit 0ff431f9 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[5457] Packet can be parked after leases4_committed callout.

parent 25fa13fa
...@@ -272,6 +272,10 @@ This debug message is printed when a callout installed on lease4_release ...@@ -272,6 +272,10 @@ This debug message is printed when a callout installed on lease4_release
hook point set the next step status to SKIP. For this particular hook point, the hook point set the next step status to SKIP. For this particular hook point, the
value set by a callout instructs the server to not release a lease. value set by a callout instructs the server to not release a lease.
% DHCP4_HOOK_LEASES4_COMMITTED_DROP %1: packet is dropped, because a callout set the next step tp DROP
This debug message is printed when a callout installed on the leases4_committed
hook point sets the next step to DROP.
% DHCP4_HOOK_PACKET_RCVD_SKIP %1: packet is dropped, because a callout set the next step to SKIP % DHCP4_HOOK_PACKET_RCVD_SKIP %1: packet is dropped, because a callout set the next step to SKIP
This debug message is printed when a callout installed on the pkt4_receive This debug message is printed when a callout installed on the pkt4_receive
hook point sets the next step to SKIP. For this particular hook point, the hook point sets the next step to SKIP. For this particular hook point, the
......
...@@ -835,72 +835,12 @@ Dhcpv4Srv::run_one() { ...@@ -835,72 +835,12 @@ Dhcpv4Srv::run_one() {
return; return;
} }
try {
// Now all fields and options are constructed into output wire buffer.
// Option objects modification does not make sense anymore. Hooks
// can only manipulate wire buffer at this stage.
// Let's execute all callouts registered for buffer4_send
if (HooksManager::calloutsPresent(Hooks.hook_index_buffer4_send_)) {
CalloutHandlePtr callout_handle = getCalloutHandle(query); CalloutHandlePtr callout_handle = getCalloutHandle(query);
processPacketBufferSend(callout_handle, rsp);
// Delete previously set arguments
callout_handle->deleteAllArguments();
// Enable copying options from the packet within hook library.
ScopedEnableOptionsCopy<Pkt4> resp4_options_copy(rsp);
// Pass incoming packet as argument
callout_handle->setArgument("response4", rsp);
// Call callouts
HooksManager::callCallouts(Hooks.hook_index_buffer4_send_,
*callout_handle);
// Callouts decided to skip the next processing step. The next
// processing step would to parse the packet, so skip at this
// stage means drop.
if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) ||
(callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP)) {
LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
DHCP4_HOOK_BUFFER_SEND_SKIP)
.arg(rsp->getLabel());
return;
}
callout_handle->getArgument("response4", rsp);
}
LOG_DEBUG(packet4_logger, DBG_DHCP4_BASIC, DHCP4_PACKET_SEND)
.arg(rsp->getLabel())
.arg(rsp->getName())
.arg(static_cast<int>(rsp->getType()))
.arg(rsp->getLocalAddr().isV4Zero() ? "*" : rsp->getLocalAddr().toText())
.arg(rsp->getLocalPort())
.arg(rsp->getRemoteAddr())
.arg(rsp->getRemotePort())
.arg(rsp->getIface().empty() ? "to be determined from routing" :
rsp->getIface());
LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL_DATA,
DHCP4_RESPONSE_DATA)
.arg(rsp->getLabel())
.arg(rsp->getName())
.arg(static_cast<int>(rsp->getType()))
.arg(rsp->toText());
sendPacket(rsp);
// Update statistics accordingly for sent packet.
processStatsSent(rsp);
} catch (const std::exception& e) {
LOG_ERROR(packet4_logger, DHCP4_PACKET_SEND_FAIL)
.arg(rsp->getLabel())
.arg(e.what());
}
} }
void void
Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) { Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) {
// Log reception of the packet. We need to increase it early, as any // Log reception of the packet. We need to increase it early, as any
// failures in unpacking will cause the packet to be dropped. We // failures in unpacking will cause the packet to be dropped. We
// will increase type specific statistic further down the road. // will increase type specific statistic further down the road.
...@@ -1129,25 +1069,21 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) { ...@@ -1129,25 +1069,21 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) {
} }
} }
callout_handle->setArgument("deleted_leases4", deleted_leases); callout_handle->setArgument("deleted_leases4", deleted_leases);
callout_handle->setArgument("io_service", getIOService()); callout_handle->setArgument("io_service", getIOService());
// Call all installed callouts // Call all installed callouts
HooksManager::callCallouts(Hooks.hook_index_leases4_committed_, HooksManager::callCallouts(Hooks.hook_index_leases4_committed_,
*callout_handle); *callout_handle);
if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
DHCP4_HOOK_LEASES4_COMMITTED_DROP)
.arg(query->getLabel());
rsp.reset();
switch (callout_handle->getStatus()) { } else if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_PARK)
// If next step is set to "skip" we simply terminate here, because the && allow_packet_park) {
// next step would be to send the packet.
case CalloutHandle::NEXT_STEP_SKIP:
return;
case CalloutHandle::NEXT_STEP_PARK:
packet_park = true; packet_park = true;
break;
default:
;
} }
} }
...@@ -1155,22 +1091,34 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) { ...@@ -1155,22 +1091,34 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp) {
return; return;
} }
// PARKING SPOT after leases4_committed hook point.
CalloutHandlePtr callout_handle = getCalloutHandle(query);
if (packet_park) { if (packet_park) {
// Park the packet. The function we bind here will be executed when the hook // Park the packet. The function we bind here will be executed when the hook
// library unparks the packet. // library unparks the packet.
HooksManager::park("leases4_committed", query, HooksManager::park("leases4_committed", query,
std::bind(&Dhcpv4Srv::leases4CommittedContinue, this, [this, callout_handle, query, rsp]() mutable {
getCalloutHandle(query), query, rsp)); processPacketPktSend(callout_handle, query, rsp);
processPacketBufferSend(callout_handle, rsp);
});
// If we have parked the packet, let's reset the pointer to the
// response to indicate to the caller that it should return, as
// the packet processing will continue via the callback.
rsp.reset();
} else { } else {
// Packet is not to be parked, so let's continue processing. processPacketPktSend(callout_handle, query, rsp);
leases4CommittedContinue(getCalloutHandle(query), query, rsp);
} }
} }
void void
Dhcpv4Srv::leases4CommittedContinue(const CalloutHandlePtr& callout_handle, Dhcpv4Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle,
Pkt4Ptr& query, Pkt4Ptr& rsp) { Pkt4Ptr& query, Pkt4Ptr& rsp) {
if (!rsp) {
return;
}
// Specifies if server should do the packing // Specifies if server should do the packing
bool skip_pack = false; bool skip_pack = false;
...@@ -1222,6 +1170,76 @@ Dhcpv4Srv::leases4CommittedContinue(const CalloutHandlePtr& callout_handle, ...@@ -1222,6 +1170,76 @@ Dhcpv4Srv::leases4CommittedContinue(const CalloutHandlePtr& callout_handle,
} }
} }
void
Dhcpv4Srv::processPacketBufferSend(CalloutHandlePtr& callout_handle,
Pkt4Ptr& rsp) {
if (!rsp) {
return;
}
try {
// Now all fields and options are constructed into output wire buffer.
// Option objects modification does not make sense anymore. Hooks
// can only manipulate wire buffer at this stage.
// Let's execute all callouts registered for buffer4_send
if (HooksManager::calloutsPresent(Hooks.hook_index_buffer4_send_)) {
// Delete previously set arguments
callout_handle->deleteAllArguments();
// Enable copying options from the packet within hook library.
ScopedEnableOptionsCopy<Pkt4> resp4_options_copy(rsp);
// Pass incoming packet as argument
callout_handle->setArgument("response4", rsp);
// Call callouts
HooksManager::callCallouts(Hooks.hook_index_buffer4_send_,
*callout_handle);
// Callouts decided to skip the next processing step. The next
// processing step would to parse the packet, so skip at this
// stage means drop.
if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) ||
(callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP)) {
LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
DHCP4_HOOK_BUFFER_SEND_SKIP)
.arg(rsp->getLabel());
return;
}
callout_handle->getArgument("response4", rsp);
}
LOG_DEBUG(packet4_logger, DBG_DHCP4_BASIC, DHCP4_PACKET_SEND)
.arg(rsp->getLabel())
.arg(rsp->getName())
.arg(static_cast<int>(rsp->getType()))
.arg(rsp->getLocalAddr().isV4Zero() ? "*" : rsp->getLocalAddr().toText())
.arg(rsp->getLocalPort())
.arg(rsp->getRemoteAddr())
.arg(rsp->getRemotePort())
.arg(rsp->getIface().empty() ? "to be determined from routing" :
rsp->getIface());
LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL_DATA,
DHCP4_RESPONSE_DATA)
.arg(rsp->getLabel())
.arg(rsp->getName())
.arg(static_cast<int>(rsp->getType()))
.arg(rsp->toText());
sendPacket(rsp);
// Update statistics accordingly for sent packet.
processStatsSent(rsp);
} catch (const std::exception& e) {
LOG_ERROR(packet4_logger, DHCP4_PACKET_SEND_FAIL)
.arg(rsp->getLabel())
.arg(e.what());
}
}
string string
Dhcpv4Srv::srvidToString(const OptionPtr& srvid) { Dhcpv4Srv::srvidToString(const OptionPtr& srvid) {
if (!srvid) { if (!srvid) {
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include <boost/noncopyable.hpp> #include <boost/noncopyable.hpp>
#include <functional>
#include <iostream> #include <iostream>
#include <queue> #include <queue>
...@@ -260,18 +261,11 @@ public: ...@@ -260,18 +261,11 @@ public:
/// ///
/// @param query A pointer to the packet to be processed. /// @param query A pointer to the packet to be processed.
/// @param rsp A pointer to the response /// @param rsp A pointer to the response
void processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp); /// @param allow_packet_park Indicates if parking a packet is allowed.
void processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp,
bool allow_packet_park = true);
/// @brief Continues DHCPv4 packet processing after leases4_committed hook
/// point.
///
/// @param callout_handle Callout handle to be used.
/// @param query A pointer to the packet to be processed.
/// @param rsp A pointer to the response
void leases4CommittedContinue(const hooks::CalloutHandlePtr& callout_handle,
Pkt4Ptr& query, Pkt4Ptr& rsp);
/// @brief Instructs the server to shut down. /// @brief Instructs the server to shut down.
void shutdown(); void shutdown();
...@@ -837,6 +831,21 @@ protected: ...@@ -837,6 +831,21 @@ protected:
/// @param query Pointer to the client message. /// @param query Pointer to the client message.
void deferredUnpack(Pkt4Ptr& query); void deferredUnpack(Pkt4Ptr& query);
/// @brief Executes pkt4_send callout.
///
/// @param callout_handle pointer to the callout handle.
/// @param query Pointer to a query.
/// @param rsp Pointer to a response.
void processPacketPktSend(hooks::CalloutHandlePtr& callout_handle,
Pkt4Ptr& query, Pkt4Ptr& rsp);
/// @brief Executes buffer4_send callout and sends the response.
///
/// @param callout_handle pointer to the callout handle.
/// @param pkt Pointer to a response.
void processPacketBufferSend(hooks::CalloutHandlePtr& callout_handle,
Pkt4Ptr& rsp);
/// @brief Allocation Engine. /// @brief Allocation Engine.
/// Pointer to the allocation engine that we are currently using /// Pointer to the allocation engine that we are currently using
/// It must be a pointer, because we will support changing engines /// It must be a pointer, because we will support changing engines
......
...@@ -103,7 +103,7 @@ void Dhcp4to6Ipc::handler() { ...@@ -103,7 +103,7 @@ void Dhcp4to6Ipc::handler() {
// From Dhcpv4Srv::run_one() processing and after // From Dhcpv4Srv::run_one() processing and after
Pkt4Ptr rsp; Pkt4Ptr rsp;
ControlledDhcpv4Srv::getInstance()->processPacket(query, rsp); ControlledDhcpv4Srv::getInstance()->processPacket(query, rsp, false);
if (!rsp) { if (!rsp) {
return; return;
......
...@@ -406,6 +406,15 @@ Dhcp4Client::doRequest() { ...@@ -406,6 +406,15 @@ Dhcp4Client::doRequest() {
} }
} }
void
Dhcp4Client::receiveResponse() {
context_.response_ = receiveOneMsg();
// If the server has responded, store the configuration received.
if (context_.response_) {
applyConfiguration();
}
}
void void
Dhcp4Client::includeClientId(const std::string& clientid) { Dhcp4Client::includeClientId(const std::string& clientid) {
if (clientid.empty()) { if (clientid.empty()) {
...@@ -519,7 +528,14 @@ Dhcp4Client::sendMsg(const Pkt4Ptr& msg) { ...@@ -519,7 +528,14 @@ Dhcp4Client::sendMsg(const Pkt4Ptr& msg) {
msg_copy->setLocalAddr(dest_addr_); msg_copy->setLocalAddr(dest_addr_);
msg_copy->setIface(iface_name_); msg_copy->setIface(iface_name_);
srv_->fakeReceive(msg_copy); srv_->fakeReceive(msg_copy);
srv_->run();
try {
// Invoke run_one instead of run, because we want to avoid triggering
// IO service.
srv_->run_one();
} catch (...) {
// Suppress errors, as the DHCPv4 server does.
}
} }
void void
......
...@@ -206,6 +206,15 @@ public: ...@@ -206,6 +206,15 @@ public:
/// DHCPACK message is applied and can be obtained from the @c config_. /// DHCPACK message is applied and can be obtained from the @c config_.
void doRequest(); void doRequest();
/// @brief Receives a response from the server.
///
/// This method is useful to receive response from the server after
/// parking a packet. In this case, the packet is not received as a
/// result of initial exchange, e.g. @c doRequest. The test can call
/// this method to complete the transaction when it expects that the
/// packet has been unparked.
void receiveResponse();
/// @brief Generates a hardware address used by the client. /// @brief Generates a hardware address used by the client.
/// ///
/// It assigns random values to the bytes of the hardware address. /// It assigns random values to the bytes of the hardware address.
......
...@@ -658,8 +658,7 @@ public: ...@@ -658,8 +658,7 @@ public:
static void static void
leases4_committed_unpark(ParkingLotHandlePtr parking_lot, Pkt4Ptr query) { leases4_committed_unpark(ParkingLotHandlePtr parking_lot, Pkt4Ptr query) {
std::cout << "unparking" << std::endl; parking_lot->unpark(query);
std::cout << parking_lot->unpark(query) << std::endl;
} }
/// Test callback which asks the server to park the packet. /// Test callback which asks the server to park the packet.
...@@ -1926,19 +1925,26 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) { ...@@ -1926,19 +1925,26 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) {
EXPECT_FALSE(callback_deleted_lease4_); EXPECT_FALSE(callback_deleted_lease4_);
} }
TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequest) { // This test verifies that it is possible to park a packet as a result of
// the leases4_committed callouts.
TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequests) {
IfaceMgrTestConfig test_config(true); IfaceMgrTestConfig test_config(true);
IfaceMgr::instance().openSockets4(); IfaceMgr::instance().openSockets4();
// This callout uses provided IO service object to post a function
// that unparks the packet. The packet is pared and can be unparked
// by simply calling IOService::poll.
ASSERT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( ASSERT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
"leases4_committed", leases4_committed_park_callout)); "leases4_committed", leases4_committed_park_callout));
Dhcp4Client client(Dhcp4Client::SELECTING); // Create first client and perform DORA.
client.setIfaceName("eth1"); Dhcp4Client client1(Dhcp4Client::SELECTING);
ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<IOAddress>(new IOAddress("192.0.2.100")))); client1.setIfaceName("eth1");
ASSERT_NO_THROW(client1.doDORA(boost::shared_ptr<IOAddress>(new IOAddress("192.0.2.100"))));
// Make sure that we received a response // We should be offerred an address but the DHCPACK should not arrive
ASSERT_FALSE(client.getContext().response_); // at this point, because the packet is parked.
ASSERT_FALSE(client1.getContext().response_);
// Check that the callback called is indeed the one we installed // Check that the callback called is indeed the one we installed
EXPECT_EQ("leases4_committed", callback_name_); EXPECT_EQ("leases4_committed", callback_name_);
...@@ -1953,7 +1959,7 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequest) { ...@@ -1953,7 +1959,7 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequest) {
sort(expected_argument_names.begin(), expected_argument_names.end()); sort(expected_argument_names.begin(), expected_argument_names.end());
EXPECT_TRUE(callback_argument_names_ == expected_argument_names); EXPECT_TRUE(callback_argument_names_ == expected_argument_names);
// Newly allocated lease should be returned. // Newly allocated lease should be passed to the callout.
ASSERT_TRUE(callback_lease4_); ASSERT_TRUE(callback_lease4_);
EXPECT_EQ("192.0.2.100", callback_lease4_->addr_.toText()); EXPECT_EQ("192.0.2.100", callback_lease4_->addr_.toText());
...@@ -1963,7 +1969,39 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequest) { ...@@ -1963,7 +1969,39 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequest) {
// Pkt passed to a callout must be configured to copy retrieved options. // Pkt passed to a callout must be configured to copy retrieved options.
EXPECT_TRUE(callback_qry_options_copy_); EXPECT_TRUE(callback_qry_options_copy_);
// client.getServer()->getIOService()->run_one(); // Reset all indicators because we'll be now creating a second client.
resetCalloutBuffers();
// Create the second client to test that it may communicate with the
// server while the previous packet is parked.
Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
client2.setIfaceName("eth1");
ASSERT_NO_THROW(client2.doDORA(boost::shared_ptr<IOAddress>(new IOAddress("192.0.2.101"))));
// The DHCPOFFER should have been returned but not DHCPACK, as this
// packet got parked too.
ASSERT_FALSE(client2.getContext().response_);
// Check that the callback called is indeed the one we installed.
EXPECT_EQ("leases4_committed", callback_name_);
// There should be now two actions schedulede on our IO service
// by the invoked callouts. They unpark both DHCPACK messages.
ASSERT_NO_THROW(client1.getServer()->getIOService()->poll());
// Receive and check the first response.
ASSERT_NO_THROW(client1.receiveResponse());
ASSERT_TRUE(client1.getContext().response_);
Pkt4Ptr rsp = client1.getContext().response_;
EXPECT_EQ(DHCPACK, rsp->getType());
EXPECT_EQ("192.0.2.100", rsp->getYiaddr().toText());
// Receive and check the second response.
ASSERT_NO_THROW(client2.receiveResponse());
ASSERT_TRUE(client2.getContext().response_);
rsp = client2.getContext().response_;
EXPECT_EQ(DHCPACK, rsp->getType());
EXPECT_EQ("192.0.2.101", rsp->getYiaddr().toText());
} }
// This test verifies that valid RELEASE triggers lease4_release callouts // This test verifies that valid RELEASE triggers lease4_release callouts
......
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