Commit 4de75117 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[1960] Implemented changes suggested in code review.

parent 5da8f0ce
......@@ -39,6 +39,9 @@
namespace isc {
namespace dhcp {
/* IPv4 Broadcast address */
#define DHCP_IPV4_BROADCAST_ADDRESS "255.255.255.255"
/* BOOTP (rfc951) message types */
enum BOOTPTypes {
BOOTREQUEST = 1,
......
......@@ -483,7 +483,7 @@ IfaceMgr::getLocalAddress(const IOAddress& remote_addr, const uint16_t port) {
// If remote address is broadcast address we have to
// allow this on the socket.
if (remote_addr.getAddress().is_v4() &&
(remote_addr == IOAddress("255.255.255.255"))) {
(remote_addr == IOAddress(DHCP_IPV4_BROADCAST_ADDRESS))) {
// Socket has to be open prior to setting the broadcast
// option. Otherwise set_option will complain about
// bad file descriptor.
......
......@@ -146,10 +146,6 @@ CommandOptions::initialize(int argc, char** argv) {
stream << " " << optarg;
}
switch (opt) {
case 'v':
version();
return (true);
case '1':
use_first_ = true;
break;
......@@ -337,6 +333,10 @@ CommandOptions::initialize(int argc, char** argv) {
}
break;
case 'v':
version();
return (true);
case 'w':
wrapped_ = nonEmptyString("command for wrapped mode:"
" -w<command> must be specified");
......@@ -396,14 +396,14 @@ CommandOptions::initialize(int argc, char** argv) {
server_name_ = argv[optind];
// Decode special cases
if ((ipversion_ == 4) && (server_name_.compare("all") == 0)) {
broadcast_ = 1;
// 255.255.255.255 is IPv4 broadcast address
server_name_ = "255.255.255.255";
broadcast_ = true;
// Use broadcast address as server name.
server_name_ = DHCP_IPV4_BROADCAST_ADDRESS;
} else if ((ipversion_ == 6) && (server_name_.compare("all") == 0)) {
server_name_ = "FF02::1:2";
server_name_ = ALL_DHCP_RELAY_AGENTS_AND_SERVERS;
} else if ((ipversion_ == 6) &&
(server_name_.compare("servers") == 0)) {
server_name_ = "FF05::1:3";
server_name_ = ALL_DHCP_SERVERS;
}
}
......@@ -411,10 +411,10 @@ CommandOptions::initialize(int argc, char** argv) {
if (!localname_.empty()) {
if (server_name_.empty()) {
if (is_interface_ && (ipversion_ == 4)) {
broadcast_ = 1;
server_name_ = "255.255.255.255";
broadcast_ = true;
server_name_ = DHCP_IPV4_BROADCAST_ADDRESS;
} else if (is_interface_ && (ipversion_ == 6)) {
server_name_ = "FF02::1:2";
server_name_ = ALL_DHCP_RELAY_AGENTS_AND_SERVERS;
}
}
}
......@@ -443,11 +443,7 @@ CommandOptions::initClientsNum() {
long long clients_num = 0;
try {
clients_num = boost::lexical_cast<long long>(optarg);
} catch (boost::bad_lexical_cast&) {
isc_throw(isc::InvalidParameter, errmsg.c_str());
}
check(clients_num < 0, errmsg);
try {
check(clients_num < 0, errmsg);
clients_num_ = boost::lexical_cast<uint32_t>(optarg);
} catch (boost::bad_lexical_cast&) {
isc_throw(isc::InvalidParameter, errmsg);
......@@ -487,7 +483,8 @@ CommandOptions::decodeMac(const std::string& base) {
// Strip string from mac=
size_t found = base.find('=');
static const char* errmsg = "expected -b<base> format for"
" mac address is -b mac=00::0C::01::02::03::04";
" mac address is -b mac=00::0C::01::02::03::04 or"
" -b mac=00:0C:01:02:03:04";
check(found == std::string::npos, errmsg);
// Decode mac address to vector of uint8_t
......
......@@ -37,11 +37,11 @@ main(int argc, char* argv[]) {
}
} catch(isc::Exception& e) {
ret_code = 1;
std::cout << "Error parsing command line options: "
std::cerr << "Error parsing command line options: "
<< e.what() << std::endl;
command_options.usage();
if (diags.find('e') != std::string::npos) {
std::cout << "Fatal error" << std::endl;
std::cerr << "Fatal error" << std::endl;
}
return (ret_code);
}
......@@ -50,9 +50,9 @@ main(int argc, char* argv[]) {
ret_code = test_control.run();
} catch (isc::Exception& e) {
ret_code = 1;
std::cout << "Error running perfdhcp: " << e.what() << std::endl;
std::cerr << "Error running perfdhcp: " << e.what() << std::endl;
if (diags.find('e') != std::string::npos) {
std::cout << "Fatal error" << std::endl;
std::cerr << "Fatal error" << std::endl;
}
}
return (ret_code);
......
......@@ -84,7 +84,7 @@ public:
}
const CustomCounter& operator+=(int val) {
counter_ = counter_ + val;
counter_ += val;
return (*this);
}
......
......@@ -673,6 +673,47 @@ TestControl::openSocket() const {
return (sock);
}
void
TestControl::sendPackets(const TestControlSocket& socket,
const uint64_t packets_num,
const bool preload /* = false */) {
CommandOptions& options = CommandOptions::instance();
for (uint64_t i = packets_num; i > 0; --i) {
if (options.getIpVersion() == 4) {
// No template packets means that no -T option was specified.
// We have to build packets ourselfs.
if (template_buffers_.size() == 0) {
sendDiscover4(socket, preload);
} else {
// @todo add defines for packet type index that can be
// used to access template_buffers_.
sendDiscover4(socket, template_buffers_[0], preload);
}
} else {
// No template packets means that no -T option was specified.
// We have to build packets ourselfs.
if (template_buffers_.size() == 0) {
sendSolicit6(socket, preload);
} else {
// @todo add defines for packet type index that can be
// used to access template_buffers_.
sendSolicit6(socket, template_buffers_[0], preload);
}
}
// If we preload server we don't want to receive any packets.
if (!preload) {
uint64_t latercvd = receivePackets(socket);
if (testDiags('i')) {
if (options.getIpVersion() == 4) {
stats_mgr4_->incrementCounter("latercvd", latercvd);
} else if (options.getIpVersion() == 6) {
stats_mgr6_->incrementCounter("latercvd", latercvd);
}
}
}
}
}
void
TestControl::printDiagnostics() const {
CommandOptions& options = CommandOptions::instance();
......@@ -948,7 +989,7 @@ TestControl::receivePackets(const TestControlSocket& socket) {
}
}
}
return received;
return (received);
}
void
......@@ -1049,14 +1090,14 @@ TestControl::run() {
isc_throw(InvalidOperation,
"command options must be parsed before running a test");
} else if (options.getIpVersion() == 4) {
setTransidGenerator(NumberGeneratorPtr(new SequencialGenerator()));
setTransidGenerator(NumberGeneratorPtr(new SequentialGenerator()));
} else {
setTransidGenerator(NumberGeneratorPtr(new SequencialGenerator(0x00FFFFFF)));
setTransidGenerator(NumberGeneratorPtr(new SequentialGenerator(0x00FFFFFF)));
}
uint32_t clients_num = options.getClientsNum() == 0 ?
1 : options.getClientsNum();
setMacAddrGenerator(NumberGeneratorPtr(new SequencialGenerator(clients_num)));
setMacAddrGenerator(NumberGeneratorPtr(new SequentialGenerator(clients_num)));
// Diagnostics are command line options mainly.
printDiagnostics();
......@@ -1080,37 +1121,9 @@ TestControl::run() {
}
// If user interrupts the program we will exit gracefully.
signal(SIGINT, TestControl::handleInterrupt);
// Preload server with number of packets.
const bool do_preload = true;
for (int i = 0; i < options.getPreload(); ++i) {
if (options.getIpVersion() == 4) {
// No template buffer means no -T option specified.
// We will build packet ourselves.
if (template_buffers_.size() == 0) {
sendDiscover4(socket, do_preload);
} else {
// Pick template #0 if Discover is being sent.
// For Request it would be #1.
// @todo add defines for packet type index that can be
// used to access template_buffers_.
sendDiscover4(socket, template_buffers_[0],
do_preload);
}
} else if (options.getIpVersion() == 6) {
// No template buffer means no -T option specified.
// We will build packet ourselfs.
if (template_buffers_.size() == 0) {
sendSolicit6(socket, do_preload);
} else {
// Pick template #0 if Solicit is being sent.
// For Request it would be #1.
// @todo add defines for packet type index that can be
// used to access template_buffers_.
sendSolicit6(socket, template_buffers_[0],
do_preload);
}
}
}
// Preload server with the number of packets.
sendPackets(socket, options.getPreload(), true);
// Fork and run command specified with -w<wrapped-command>
if (!options.getWrapped().empty()) {
......@@ -1144,39 +1157,9 @@ TestControl::run() {
break;
}
// Send packets.
for (uint64_t i = packets_due; i > 0; --i) {
if (options.getIpVersion() == 4) {
// No template packets means that no -T option was specified.
// We have to build packets ourselfs.
if (template_buffers_.size() == 0) {
sendDiscover4(socket);
} else {
// @todo add defines for packet type index that can be
// used to access template_buffers_.
sendDiscover4(socket, template_buffers_[0]);
}
} else {
// No template packets means that no -T option was specified.
// We have to build packets ourselfs.
if (template_buffers_.size() == 0) {
sendSolicit6(socket);
} else {
// @todo add defines for packet type index that can be
// used to access template_buffers_.
sendSolicit6(socket, template_buffers_[0]);
}
}
// Receive late packets.
uint64_t latercvd = receivePackets(socket);
if (testDiags('i')) {
if (options.getIpVersion() == 4) {
stats_mgr4_->incrementCounter("latercvd", latercvd);
} else if (options.getIpVersion() == 6) {
stats_mgr6_->incrementCounter("latercvd", latercvd);
}
}
}
// Initiate new DHCP packet exchanges.
sendPackets(socket, packets_due);
// Report delay means that user requested printing number
// of sent/received/dropped packets repeatedly.
if (options.getReportDelay() > 0) {
......@@ -1186,8 +1169,8 @@ TestControl::run() {
printStats();
if (!options.getWrapped().empty()) {
const bool do_stop = true;
runWrapped(do_stop);
// true means that we execute wrapped command with 'stop' argument.
runWrapped(true);
}
// Print packet timestamps
......@@ -1234,13 +1217,31 @@ TestControl::runWrapped(bool do_stop /*= false */) const {
if (pid < 0) {
isc_throw(Unexpected, "unable to fork");
} else if (pid == 0) {
execlp(options.getWrapped().c_str(),
execlp(options.getWrapped().c_str(),
do_stop ? "stop" : "start",
NULL);
}
}
}
void
TestControl::saveFirstPacket(const Pkt4Ptr& pkt) {
if (testDiags('T')) {
if (template_packets_v4_.find(pkt->getType()) == template_packets_v4_.end()) {
template_packets_v4_[pkt->getType()] = pkt;
}
}
}
void
TestControl::saveFirstPacket(const Pkt6Ptr& pkt) {
if (testDiags('T')) {
if (template_packets_v6_.find(pkt->getType()) == template_packets_v6_.end()) {
template_packets_v6_[pkt->getType()] = pkt;
}
}
}
void
TestControl::sendDiscover4(const TestControlSocket& socket,
const bool preload /*= false*/) {
......@@ -1278,10 +1279,7 @@ TestControl::sendDiscover4(const TestControlSocket& socket,
}
stats_mgr4_->passSentPacket(StatsMgr4::XCHG_DO, pkt4);
}
if (testDiags('T') &&
(template_packets_v4_.find(DHCPDISCOVER) == template_packets_v4_.end())) {
template_packets_v4_[DHCPDISCOVER] = pkt4;
}
saveFirstPacket(pkt4);
}
void
......@@ -1335,10 +1333,7 @@ TestControl::sendDiscover4(const TestControlSocket& socket,
stats_mgr4_->passSentPacket(StatsMgr4::XCHG_DO,
boost::static_pointer_cast<Pkt4>(pkt4));
}
if (testDiags('T') &&
(template_packets_v4_.find(DHCPDISCOVER) == template_packets_v4_.end())) {
template_packets_v4_[DHCPDISCOVER] = pkt4;
}
saveFirstPacket(pkt4);
}
void
......@@ -1402,10 +1397,7 @@ TestControl::sendRequest4(const TestControlSocket& socket,
"hasn't been initialized");
}
stats_mgr4_->passSentPacket(StatsMgr4::XCHG_RA, pkt4);
if (testDiags('T') &&
(template_packets_v4_.find(DHCPREQUEST) == template_packets_v4_.end())) {
template_packets_v4_[DHCPREQUEST] = pkt4;
}
saveFirstPacket(pkt4);
}
void
......@@ -1508,10 +1500,7 @@ TestControl::sendRequest4(const TestControlSocket& socket,
// Update packet stats.
stats_mgr4_->passSentPacket(StatsMgr4::XCHG_RA,
boost::static_pointer_cast<Pkt4>(pkt4));
if (testDiags('T') &&
(template_packets_v4_.find(DHCPREQUEST) == template_packets_v4_.end())) {
template_packets_v4_[DHCPREQUEST] = pkt4;
}
saveFirstPacket(pkt4);
}
void
......@@ -1564,10 +1553,7 @@ TestControl::sendRequest6(const TestControlSocket& socket,
"hasn't been initialized");
}
stats_mgr6_->passSentPacket(StatsMgr6::XCHG_RR, pkt6);
if (testDiags('T') &&
(template_packets_v6_.find(DHCPV6_REQUEST) == template_packets_v6_.end())) {
template_packets_v6_[DHCPV6_REQUEST] = pkt6;
}
saveFirstPacket(pkt6);
}
void
......@@ -1671,6 +1657,12 @@ TestControl::sendRequest6(const TestControlSocket& socket,
}
// Update packet stats.
stats_mgr6_->passSentPacket(StatsMgr6::XCHG_RR, pkt6);
// When 'T' diagnostics flag is specified it means that user requested
// printing packet contents. It will be just one (first) packet which
// contents will be printed. Here we check if this packet has been already
// collected. If it hasn't we save this packet so as we can print its
// contents when test is finished.
if (testDiags('T') &&
(template_packets_v6_.find(DHCPV6_REQUEST) == template_packets_v6_.end())) {
template_packets_v6_[DHCPV6_REQUEST] = pkt6;
......@@ -1708,10 +1700,8 @@ TestControl::sendSolicit6(const TestControlSocket& socket,
}
stats_mgr6_->passSentPacket(StatsMgr6::XCHG_SA, pkt6);
}
if (testDiags('T') &&
(template_packets_v6_.find(DHCPV6_SOLICIT) == template_packets_v6_.end())) {
template_packets_v6_[DHCPV6_SOLICIT] = pkt6;
}
saveFirstPacket(pkt6);
}
void
......@@ -1756,10 +1746,7 @@ TestControl::sendSolicit6(const TestControlSocket& socket,
// Update packet stats.
stats_mgr6_->passSentPacket(StatsMgr6::XCHG_SA, pkt6);
}
if (testDiags('T') &&
(template_packets_v6_.find(DHCPV6_SOLICIT) == template_packets_v6_.end())) {
template_packets_v6_[DHCPV6_SOLICIT] = pkt6;
}
saveFirstPacket(pkt6);
}
......
......@@ -35,8 +35,45 @@ namespace perfdhcp {
/// \brief Test Control class.
///
/// This class is responsible for executing DHCP performance
/// test end to end.
/// This singleton class is used to run performance test with
/// with \ref TestControl::run function. This function can be executed
/// multiple times if desired because it resets TestControl's internal
/// state efery time it is executed. Prior to running \ref TestControl::run
/// one must make sure to parse command line options by calling
/// \ref CommandOptions::parse. Failing to do this will result in exception.
/// The following major stages of the test are performed by this class:
/// - set default transaction id and MAC address generators - the generator
/// is the object of \ref TestControl::NumberGenerator type and it provides
/// the custom randomization algorithms,
/// - print command line arguments,
/// - register option factory functions which are used to generate DHCP options
/// being sent to a server,
/// - create the socket for communication with a server,
/// - read packet templates if user specified template files with '-T' command
/// line option,
/// - set the interrupt handler (invoked when ^C is pressed) which makes
/// perfdhcp stop gracefully and print the test results before exiting,
/// - executes external command (if specified '-w' option), e.g. if user specified
/// -w ./foo in the command line then program will execute ./foo start at the
/// beginning of the test and ./foo stop when test ends,
/// - initialize Statistics Manager,
/// - execute the main loop:
/// - calculate how many packets must be send to satisfy desired rate,
/// - receive incoming packets from the server,
/// - check the exit conditions - terminate the program if exit criteria
/// are fulfiled, e.g. reached maximum number of packet drops,
/// - send number of packets appropriate to satisfy the desired rate,
/// - optionally print intermediate reports,
/// - print statistics, e.g. achived rate,
/// - optionally print some diagnostics.
///
/// With the '-w' command line option user may specify the external application
/// or script to be executed first time when test starts and second time when
/// test ends. This external script or application must support 'start' and 'stop'
/// arguments. The first time it is called it is called with'start' and
/// the second time with 'stop'. The way it is executed is to fork() the current
/// perfdhcp process and in turn executed execlp function that replaces current
/// process image with new image.
///
/// Option factory functions are registered using
/// \ref dhcp::LibDHCP::OptionFactoryRegister. Registered factory functions
......@@ -138,7 +175,7 @@ public:
/// This is default numbers generator class. The member function is
/// used to generate uint32_t values. Other generator classes should
/// derive from this one to implement generation algorithms
/// (e.g. sequencial or based on random function).
/// (e.g. sequential or based on random function).
class NumberGenerator {
public:
/// \brief Generate number.
......@@ -150,14 +187,14 @@ public:
/// The default generator pointer.
typedef boost::shared_ptr<NumberGenerator> NumberGeneratorPtr;
/// \brief Sequencial numbers generatorc class.
class SequencialGenerator : public NumberGenerator {
/// \brief Sequential numbers generatorc class.
class SequentialGenerator : public NumberGenerator {
public:
/// \brief Constructor.
///
/// \param range maximum number generated. If 0 is given then
/// range defaults to maximym uint32_t value.
SequencialGenerator(uint32_t range = 0xFFFFFFFF) :
/// range defaults to maximum uint32_t value.
SequentialGenerator(uint32_t range = 0xFFFFFFFF) :
NumberGenerator(),
num_(0),
range_(range) {
......@@ -166,7 +203,7 @@ public:
}
}
/// \brief Generate number sequencialy.
/// \brief Generate number sequentialy.
///
/// \return generated number.
virtual uint32_t generate() {
......@@ -176,7 +213,7 @@ public:
}
private:
uint32_t num_; ///< Current number.
uint32_t range_; ///< Maximum number generated.
uint32_t range_; ///< Number of unique numbers generated.
};
/// \brief Length of the Ethernet HW address (MAC) in bytes.
......@@ -538,6 +575,36 @@ protected:
/// called before new test is started.
void reset();
/// \brief Save the first DHCPv4 sent packet of the specified type.
///
/// This method saves first packet of the specified being sent
/// to the server if user requested diagnostics flag 'T'. In
/// such case program has to print contents of selected packets
/// being sent to the server. It collects first packets of each
/// type and keeps them around until test finishes. Then they
/// are printed to the user. If packet of specified type has
/// been already stored this function perfroms no operation.
/// This function does not perform sainty check if packet
/// pointer is valid. Make sure it is before calling it.
///
/// \param pkt packet to be stored.
inline void saveFirstPacket(const dhcp::Pkt4Ptr& pkt);
/// \brief Save the first DHCPv6 sent packet of the specified type.
///
/// This method saves first packet of the specified being sent
/// to the server if user requested diagnostics flag 'T'. In
/// such case program has to print contents of selected packets
/// being sent to the server. It collects first packets of each
/// type and keeps them around until test finishes. Then they
/// are printed to the user. If packet of specified type has
/// been already stored this function perfroms no operation.
/// This function does not perform sainty check if packet
/// pointer is valid. Make sure it is before calling it.
///
/// \param pkt packet to be stored.
inline void saveFirstPacket(const dhcp::Pkt6Ptr& pkt);
/// \brief Send DHCPv4 DISCOVER message.
///
/// Method creates and sends DHCPv4 DISCOVER message to the server
......@@ -575,6 +642,31 @@ protected:
const std::vector<uint8_t>& template_buf,
const bool preload = false);
/// \brief Send number of packets to initiate new exchanges.
///
/// Method initiates the new DHCP exchanges by sending number
/// of DISCOVER (DHCPv4) or SOLICIT (DHCPv6) packets. If preload
/// mode was requested sent packets will not be counted in
/// the statistics. The responses from the server will be
/// received and counted as orphans because corresponding sent
/// packets are not included in StatsMgr for match.
/// When preload mode is disabled and diagnostics flag 'i' is
/// specified then function will be trying to receive late packets
/// before new packets are sent to the server. Statistics of
/// late received packets is updated accordingly.
///
/// \todo do not count responses in preload mode as orphans.
///
/// \param socket socket to be used to send packets.
/// \param packets_num number of packets to be sent.
/// \param preload preload mode, packets not included in statistics.
/// \throw isc::Unexpected if thrown by packet sending method.
/// \throw isc::InvalidOperation if thrown by packet sending method.
/// \throw isc::OutOfRange if thrown by packet sending method.
void sendPackets(const TestControlSocket &socket,
const uint64_t packets_num,
const bool preload = false);
/// \brief Send DHCPv4 REQUEST message.
///
/// Method creates and sends DHCPv4 REQUEST message to the server.
......
......@@ -581,19 +581,19 @@ TEST_F(CommandOptionsTest, Server) {
// set to broadcast address because 'all' was specified.
EXPECT_TRUE(opt.isBroadcast());
// The broadcast address is 255.255.255.255.
EXPECT_EQ("255.255.255.255", opt.getServerName());
EXPECT_EQ(DHCP_IPV4_BROADCAST_ADDRESS, opt.getServerName());
// When all is specified for DHCPv6 mode we expect
// FF02::1:2 as a server name which means All DHCP
// servers and relay agents in local network segment
ASSERT_NO_THROW(process("perfdhcp -6 all"));
EXPECT_EQ("FF02::1:2", opt.getServerName());
EXPECT_EQ(ALL_DHCP_RELAY_AGENTS_AND_SERVERS, opt.getServerName());
// When server='servers' in DHCPv6 mode we expect
// FF05::1:3 as server name which means All DHCP
// servers in local network.
ASSERT_NO_THROW(process("perfdhcp -6 servers"));
EXPECT_EQ("FF05::1:3", opt.getServerName());
EXPECT_EQ(ALL_DHCP_SERVERS, opt.getServerName());
// If server name is neither 'all' nor 'servers'
// the given argument value is expected to be
......
......@@ -92,7 +92,7 @@ public:
NakedTestControl() : TestControl() {