Commit 0b2ec985 authored by Francis Dupont's avatar Francis Dupont
Browse files

[4109a] Addressed comments

parent f1f3aca8
...@@ -3279,6 +3279,33 @@ should include options from the isc option space: ...@@ -3279,6 +3279,33 @@ should include options from the isc option space:
</entry> </entry>
</row> </row>
<row>
<entry>pkt6-dhcpv4-query-received</entry>
<entry>integer</entry>
<entry>
Number of DHCPv4-QUERY packets received. This
statistic is expected to grow if there are devices
that are using DHCPv4-over-DHCPv6. DHCPv4-QUERY
messages are used by DHCPv4 clients on an IPv6 only
line so that encapsulate requests over DHCPv6.
</entry>
</row>
<row>
<entry>pkt6-dhcpv4-response-received</entry>
<entry>integer</entry>
<entry>
Number of DHCPv4-RESPONSE packets received. This
statistic is expected to remain zero at all times, as
DHCPv4-RESPONSE packets are sent by the server and the
server is never expected to receive them. A non-zero
value indicates an error. One likely cause would be a
misbehaving relay agent that incorrectly forwards
DHCPv4-RESPONSE message towards the server, rather
back to the clients.
</entry>
</row>
<row> <row>
<entry>pkt6-unknown-received</entry> <entry>pkt6-unknown-received</entry>
<entry>integer</entry> <entry>integer</entry>
...@@ -3319,6 +3346,16 @@ should include options from the isc option space: ...@@ -3319,6 +3346,16 @@ should include options from the isc option space:
</entry> </entry>
</row> </row>
<row>
<entry>pkt6-dhcpv4-response-sent</entry>
<entry>integer</entry>
<entry>Number of DHCPv4-RESPONSE packets sent. This
statistic is expected to grow in most cases after a
DHCPv4-QUERY is processed. There are certain cases where
there is no response.
</entry>
</row>
<row> <row>
<entry>subnet[id].total-nas</entry> <entry>subnet[id].total-nas</entry>
<entry>integer</entry> <entry>integer</entry>
......
...@@ -165,8 +165,6 @@ createStatusCode(const Pkt6& pkt, const Option6IA& ia, const uint16_t status_cod ...@@ -165,8 +165,6 @@ createStatusCode(const Pkt6& pkt, const Option6IA& ia, const uint16_t status_cod
namespace isc { namespace isc {
namespace dhcp { namespace dhcp {
int Dhcpv6Srv::hook_index_buffer6_send = Hooks.hook_index_buffer6_send_;
const std::string Dhcpv6Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_"); const std::string Dhcpv6Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
Dhcpv6Srv::Dhcpv6Srv(uint16_t port) Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
...@@ -3092,6 +3090,13 @@ void Dhcpv6Srv::processStatsReceived(const Pkt6Ptr& query) { ...@@ -3092,6 +3090,13 @@ void Dhcpv6Srv::processStatsReceived(const Pkt6Ptr& query) {
case DHCPV6_INFORMATION_REQUEST: case DHCPV6_INFORMATION_REQUEST:
stat_name = "pkt6-infrequest-received"; stat_name = "pkt6-infrequest-received";
break; break;
case DHCPV6_DHCPV4_QUERY:
stat_name = "pkt6-dhcpv4-query-received";
break;
case DHCPV6_DHCPV4_RESPONSE:
// Should not happen, but let's keep a counter for it
stat_name = "pkt6-dhcpv4-response-received";
break;
default: default:
; // do nothing ; // do nothing
} }
...@@ -3112,6 +3117,9 @@ void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) { ...@@ -3112,6 +3117,9 @@ void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) {
case DHCPV6_REPLY: case DHCPV6_REPLY:
stat_name = "pkt6-reply-sent"; stat_name = "pkt6-reply-sent";
break; break;
case DHCPV6_DHCPV4_RESPONSE:
stat_name = "pkt6-dhcpv4-response-sent";
break;
default: default:
// That should never happen // That should never happen
return; return;
...@@ -3120,5 +3128,9 @@ void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) { ...@@ -3120,5 +3128,9 @@ void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) {
StatsMgr::instance().addValue(stat_name, static_cast<int64_t>(1)); StatsMgr::instance().addValue(stat_name, static_cast<int64_t>(1));
} }
int Dhcpv6Srv::getHookIndexBuffer6Send() {
return (Hooks.hook_index_buffer6_send_);
}
}; };
}; };
...@@ -292,7 +292,9 @@ protected: ...@@ -292,7 +292,9 @@ protected:
/// @brief Processes incoming DHCPv4-query message. /// @brief Processes incoming DHCPv4-query message.
/// ///
/// @param dhcp4_query message received from client /// @param dhcp4_query message received from client
/// @return Reply (empty) message to (not) be sent to the client. /// @return Reply message to be sent to the client (always NULL
/// as the response will be returned after by
/// @ref isc::dhcp::Dhcp6to4Ipc::handler()).
Pkt6Ptr processDhcp4Query(const Pkt6Ptr& dhcp4_query); Pkt6Ptr processDhcp4Query(const Pkt6Ptr& dhcp4_query);
/// @brief Selects a subnet for a given client's packet. /// @brief Selects a subnet for a given client's packet.
...@@ -791,14 +793,15 @@ private: ...@@ -791,14 +793,15 @@ private:
uint16_t port_; uint16_t port_;
public: public:
/// @note used by DHCPv4-over-DHCPv6 so must be public /// @note used by DHCPv4-over-DHCPv6 so must be public and static
/// @brief Updates statistics for transmitted packets /// @brief Updates statistics for transmitted packets
/// @param response packet transmitted /// @param response packet transmitted
static void processStatsSent(const Pkt6Ptr& response); static void processStatsSent(const Pkt6Ptr& response);
/// the index of the buffer6_send hook /// @brief Returns the index of the buffer6_send hook
static int hook_index_buffer6_send; /// @return the index of the buffer6_send hook
static int getHookIndexBuffer6Send();
protected: protected:
......
...@@ -85,7 +85,8 @@ void Dhcp6to4Ipc::handler() { ...@@ -85,7 +85,8 @@ void Dhcp6to4Ipc::handler() {
buf.clear(); buf.clear();
pkt->pack(); pkt->pack();
// Don't use getType(): get the message type from the buffer // Don't use getType(): get the message type from the buffer as we
// want to know if it is a relayed message (vs. internal message type).
uint8_t msg_type = buf[0]; uint8_t msg_type = buf[0];
if ((msg_type == DHCPV6_RELAY_FORW) || (msg_type == DHCPV6_RELAY_REPL)) { if ((msg_type == DHCPV6_RELAY_FORW) || (msg_type == DHCPV6_RELAY_REPL)) {
pkt->setRemotePort(DHCP6_SERVER_PORT); pkt->setRemotePort(DHCP6_SERVER_PORT);
...@@ -99,7 +100,7 @@ void Dhcp6to4Ipc::handler() { ...@@ -99,7 +100,7 @@ void Dhcp6to4Ipc::handler() {
try { try {
// Let's execute all callouts registered for buffer6_send // Let's execute all callouts registered for buffer6_send
if (HooksManager::calloutsPresent(Dhcpv6Srv::hook_index_buffer6_send)) { if (HooksManager::calloutsPresent(Dhcpv6Srv::getHookIndexBuffer6Send())) {
CalloutHandlePtr callout_handle = getCalloutHandle(pkt); CalloutHandlePtr callout_handle = getCalloutHandle(pkt);
// Delete previously set arguments // Delete previously set arguments
...@@ -109,7 +110,7 @@ void Dhcp6to4Ipc::handler() { ...@@ -109,7 +110,7 @@ void Dhcp6to4Ipc::handler() {
callout_handle->setArgument("response6", pkt); callout_handle->setArgument("response6", pkt);
// Call callouts // Call callouts
HooksManager::callCallouts(Dhcpv6Srv::hook_index_buffer6_send, HooksManager::callCallouts(Dhcpv6Srv::getHookIndexBuffer6Send(),
*callout_handle); *callout_handle);
// Callouts decided to skip the next processing step. The next // Callouts decided to skip the next processing step. The next
......
...@@ -2585,6 +2585,13 @@ TEST_F(Dhcpv6SrvTest, receiveReplyStat) { ...@@ -2585,6 +2585,13 @@ TEST_F(Dhcpv6SrvTest, receiveReplyStat) {
testReceiveStats(DHCPV6_REPLY, "pkt6-reply-received"); testReceiveStats(DHCPV6_REPLY, "pkt6-reply-received");
} }
// Test checks if pkt6-dhcpv4-response-received is bumped up correctly.
// Note that in properly configured network the server never receives
// Dhcpv4-Response messages.
TEST_F(Dhcpv6SrvTest, receiveDhcpv4ResponseStat) {
testReceiveStats(DHCPV6_DHCPV4_RESPONSE, "pkt6-dhcpv4-response-received");
}
// Test checks if pkt6-unknown-received is bumped up correctly. // Test checks if pkt6-unknown-received is bumped up correctly.
TEST_F(Dhcpv6SrvTest, receiveUnknownStat) { TEST_F(Dhcpv6SrvTest, receiveUnknownStat) {
testReceiveStats(123, "pkt6-unknown-received"); testReceiveStats(123, "pkt6-unknown-received");
...@@ -2610,6 +2617,11 @@ TEST_F(Dhcpv6SrvTest, receiveDeclineStat) { ...@@ -2610,6 +2617,11 @@ TEST_F(Dhcpv6SrvTest, receiveDeclineStat) {
testReceiveStats(DHCPV6_DECLINE, "pkt6-decline-received"); testReceiveStats(DHCPV6_DECLINE, "pkt6-decline-received");
} }
// Test checks if pkt6-dhcpv4-query-received is bumped up correctly.
TEST_F(Dhcpv6SrvTest, receiveDhcpv4QueryStat) {
testReceiveStats(DHCPV6_DHCPV4_QUERY, "pkt6-dhcpv4-query-received");
}
// Test checks if reception of a malformed packet increases pkt-parse-failed // Test checks if reception of a malformed packet increases pkt-parse-failed
// and pkt6-receive-drop // and pkt6-receive-drop
TEST_F(Dhcpv6SrvTest, receiveParseFailedStat) { TEST_F(Dhcpv6SrvTest, receiveParseFailedStat) {
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include <dhcp6/dhcp6to4_ipc.h> #include <dhcp6/dhcp6to4_ipc.h>
#include <dhcpsrv/cfgmgr.h> #include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/testutils/dhcp4o6_test_ipc.h> #include <dhcpsrv/testutils/dhcp4o6_test_ipc.h>
#include <stats/stats_mgr.h>
#include <hooks/callout_handle.h> #include <hooks/callout_handle.h>
#include <hooks/hooks_manager.h> #include <hooks/hooks_manager.h>
#include <hooks/library_handle.h> #include <hooks/library_handle.h>
...@@ -25,6 +26,7 @@ using namespace isc; ...@@ -25,6 +26,7 @@ using namespace isc;
using namespace isc::asiolink; using namespace isc::asiolink;
using namespace isc::dhcp; using namespace isc::dhcp;
using namespace isc::dhcp::test; using namespace isc::dhcp::test;
using namespace isc::stats;
using namespace isc::hooks; using namespace isc::hooks;
using namespace isc::util; using namespace isc::util;
...@@ -51,6 +53,8 @@ public: ...@@ -51,6 +53,8 @@ public:
// Install buffer6_send_callout // Install buffer6_send_callout
EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle(). EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().
registerCallout("buffer6_send", buffer6_send_callout)); registerCallout("buffer6_send", buffer6_send_callout));
// Let's wipe all existing statistics.
StatsMgr::instance().removeAll();
} }
/// @brief Configure DHCP4o6 port. /// @brief Configure DHCP4o6 port.
...@@ -128,8 +132,15 @@ TEST_F(Dhcp6to4IpcTest, receive) { ...@@ -128,8 +132,15 @@ TEST_F(Dhcp6to4IpcTest, receive) {
ASSERT_NO_THROW(ipc.open()); ASSERT_NO_THROW(ipc.open());
ASSERT_NO_THROW(src_ipc.open()); ASSERT_NO_THROW(src_ipc.open());
// Get statistics
StatsMgr& mgr = StatsMgr::instance();
ObservationPtr pkt6_snd = mgr.getObservation("pkt6-sent");
ObservationPtr d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
EXPECT_FALSE(pkt6_snd);
EXPECT_FALSE(d4_resp);
// Create message to be sent over IPC. // Create message to be sent over IPC.
Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_QUERY, 1234)); Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_RESPONSE, 1234));
pkt->addOption(createDHCPv4MsgOption()); pkt->addOption(createDHCPv4MsgOption());
pkt->setIface("eth0"); pkt->setIface("eth0");
pkt->setRemoteAddr(IOAddress("2001:db8:1::123")); pkt->setRemoteAddr(IOAddress("2001:db8:1::123"));
...@@ -152,13 +163,20 @@ TEST_F(Dhcp6to4IpcTest, receive) { ...@@ -152,13 +163,20 @@ TEST_F(Dhcp6to4IpcTest, receive) {
EXPECT_TRUE(forwarded->getOption(D6O_DHCPV4_MSG)); EXPECT_TRUE(forwarded->getOption(D6O_DHCPV4_MSG));
EXPECT_EQ("eth0", forwarded->getIface()); EXPECT_EQ("eth0", forwarded->getIface());
EXPECT_EQ("2001:db8:1::123", forwarded->getRemoteAddr().toText()); EXPECT_EQ("2001:db8:1::123", forwarded->getRemoteAddr().toText());
// Verify statistics
pkt6_snd = mgr.getObservation("pkt6-sent");
d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
ASSERT_TRUE(pkt6_snd);
ASSERT_TRUE(d4_resp);
EXPECT_EQ(1, pkt6_snd->getInteger().first);
EXPECT_EQ(1, d4_resp->getInteger().first);
} }
// #4296 addresses this
#if 0
// This test verifies that the DHCPv4 endpoint of the DHCPv4o6 IPC can // This test verifies that the DHCPv4 endpoint of the DHCPv4o6 IPC can
// receive relayed messages. // receive relayed messages.
TEST_F(Dhcp6to4IpcTest, receiveRelayed) { // This is currently not supported: it is a known defect addressed by #4296.
TEST_F(Dhcp6to4IpcTest, DISABLED_receiveRelayed) {
// Create instance of the IPC endpoint under test. // Create instance of the IPC endpoint under test.
Dhcp6to4Ipc& ipc = Dhcp6to4Ipc::instance(); Dhcp6to4Ipc& ipc = Dhcp6to4Ipc::instance();
// Create instance of the IPC endpoint being used as a source of messages. // Create instance of the IPC endpoint being used as a source of messages.
...@@ -168,8 +186,15 @@ TEST_F(Dhcp6to4IpcTest, receiveRelayed) { ...@@ -168,8 +186,15 @@ TEST_F(Dhcp6to4IpcTest, receiveRelayed) {
ASSERT_NO_THROW(ipc.open()); ASSERT_NO_THROW(ipc.open());
ASSERT_NO_THROW(src_ipc.open()); ASSERT_NO_THROW(src_ipc.open());
// Get statistics
StatsMgr& mgr = StatsMgr::instance();
ObservationPtr pkt6_snd = mgr.getObservation("pkt6-sent");
ObservationPtr d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
EXPECT_FALSE(pkt6_snd);
EXPECT_FALSE(d4_resp);
// Create relayed message to be sent over IPC. // Create relayed message to be sent over IPC.
Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_QUERY, 1234)); Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_RESPONSE, 1234));
pkt->addOption(createDHCPv4MsgOption()); pkt->addOption(createDHCPv4MsgOption());
Pkt6::RelayInfo relay; Pkt6::RelayInfo relay;
relay.linkaddr_ = IOAddress("3000:1::1"); relay.linkaddr_ = IOAddress("3000:1::1");
...@@ -199,7 +224,14 @@ TEST_F(Dhcp6to4IpcTest, receiveRelayed) { ...@@ -199,7 +224,14 @@ TEST_F(Dhcp6to4IpcTest, receiveRelayed) {
EXPECT_EQ("eth0", forwarded->getIface()); EXPECT_EQ("eth0", forwarded->getIface());
EXPECT_EQ("2001:db8:1::123", forwarded->getRemoteAddr().toText()); EXPECT_EQ("2001:db8:1::123", forwarded->getRemoteAddr().toText());
EXPECT_EQ(DHCP6_CLIENT_PORT, forwarded->getRemotePort()); EXPECT_EQ(DHCP6_CLIENT_PORT, forwarded->getRemotePort());
// Verify statistics
pkt6_snd = mgr.getObservation("pkt6-sent");
d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
ASSERT_TRUE(pkt6_snd);
ASSERT_TRUE(d4_resp);
EXPECT_EQ(1, pkt6_snd->getInteger().first);
EXPECT_EQ(1, d4_resp->getInteger().first);
} }
#endif
} // end of anonymous namespace } // end of anonymous namespace
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