Commit 223b19a3 authored by Tomek Mrugalski's avatar Tomek Mrugalski 🛰
Browse files

[1186] Part 2 of review changes

- Dhcpv6Srv::process*() methods does not use references anymore
- Dhcpv6Srv::shutdown variable added for ordered shutdown procedure
- Dhcpv6Srv is now in isc::dhcp namespace
- Dhcpv6Srv is now derived from boot/noncopyable
- IfaceMgr methods and fields are now commented appropriately
- IfaceMgr
- Dhcpv6Srv friend declaration removed
- Dhcpv6SrvTest now uses derived class to access protected methods
- Dhcpv6SrvTest Solicit_Basic test now includes client-id, checks for
  received client-id, server-id and content of ia
- IfaceMgrTest minor improvements

See comments in #1186 ticket for extensive list of smaller changes
and reasons behind them.
parent bfea6183
......@@ -35,6 +35,8 @@ Dhcpv6Srv::Dhcpv6Srv() {
/// @todo: instantiate LeaseMgr here once it is imlpemented.
setServerID();
shutdown = false;
}
Dhcpv6Srv::~Dhcpv6Srv() {
......@@ -43,7 +45,7 @@ Dhcpv6Srv::~Dhcpv6Srv() {
bool
Dhcpv6Srv::run() {
while (true) {
while (!shutdown) {
boost::shared_ptr<Pkt6> query; // client's message
boost::shared_ptr<Pkt6> rsp; // server's response
......@@ -132,12 +134,14 @@ Dhcpv6Srv::setServerID() {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6>& solicit) {
Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6> solicit) {
boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_ADVERTISE,
solicit->getTransid(),
Pkt6::UDP));
/// TODO Rewrite this once LeaseManager is implemented.
// answer client's IA (this is mostly a dummy,
// so let's answer only first IA and hope there is only one)
boost::shared_ptr<Option> ia_opt = solicit->getOption(D6O_IA_NA);
......@@ -171,7 +175,7 @@ Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6>& solicit) {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6>& request) {
Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6> request) {
/// TODO: Implement processRequest() for real
boost::shared_ptr<Pkt6> reply = processSolicit(request);
reply->setType(DHCPV6_REPLY);
......@@ -179,7 +183,7 @@ Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6>& request) {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6>& renew) {
Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6> renew) {
boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
renew->getTransid(),
Pkt6::UDP));
......@@ -187,7 +191,7 @@ Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6>& renew) {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6>& rebind) {
Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6> rebind) {
boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
rebind->getTransid(),
Pkt6::UDP));
......@@ -195,7 +199,7 @@ Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6>& rebind) {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6>& confirm) {
Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6> confirm) {
boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
confirm->getTransid(),
Pkt6::UDP));
......@@ -203,7 +207,7 @@ Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6>& confirm) {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6>& release) {
Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6> release) {
boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
release->getTransid(),
Pkt6::UDP));
......@@ -211,7 +215,7 @@ Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6>& release) {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6>& decline) {
Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6> decline) {
boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
decline->getTransid(),
Pkt6::UDP));
......@@ -219,7 +223,7 @@ Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6>& decline) {
}
boost::shared_ptr<Pkt6>
Dhcpv6Srv::processInfRequest(boost::shared_ptr<Pkt6>& infRequest) {
Dhcpv6Srv::processInfRequest(boost::shared_ptr<Pkt6> infRequest) {
boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
infRequest->getTransid(),
Pkt6::UDP));
......
......@@ -16,15 +16,14 @@
#define DHCPV6_SRV_H
#include <boost/shared_ptr.hpp>
#include <boost/noncopyable.hpp>
#include "dhcp/pkt6.h"
#include "dhcp/option.h"
#include <iostream>
namespace test {
class Dhcpv6SrvTest_Solicit_basic_Test;
}
namespace isc {
namespace dhcp {
/// @brief DHCPv6 server service.
///
/// This singleton class represents DHCPv6 server. It contains all
......@@ -33,8 +32,8 @@ namespace isc {
/// that is going to be used as server-identifier, receives incoming
/// packets, processes them, manages leases assignment and generates
/// appropriate responses.
class Dhcpv6Srv {
private:
class Dhcpv6Srv : public boost::noncopyable {
private:
/// @brief A private copy constructor.
///
......@@ -54,13 +53,6 @@ namespace isc {
///
Dhcpv6Srv& operator=(const Dhcpv6Srv& src);
/// @brief Returns server-intentifier option
///
/// @return reference to server-id option
///
boost::shared_ptr<isc::dhcp::Option>&
getServerID() { return serverid_; }
/// @brief Sets server-identifier.
///
/// This method attempts to set server-identifier DUID. It loads it
......@@ -88,6 +80,13 @@ namespace isc {
/// @brief Destructor. Shuts down DHCPv6 service.
~Dhcpv6Srv();
/// @brief Returns server-intentifier option
///
/// @return reference to server-id option
///
boost::shared_ptr<isc::dhcp::Option>&
getServerID() { return serverid_; }
/// @brief Main server processing loop.
///
/// Main server processing loop. Receives incoming packets, verifies
......@@ -115,7 +114,7 @@ namespace isc {
/// @return ADVERTISE, REPLY message or NULL
///
boost::shared_ptr<Pkt6>
processSolicit(boost::shared_ptr<Pkt6>& solicit);
processSolicit(boost::shared_ptr<Pkt6> solicit);
/// @brief Processes incoming REQUEST and returns REPLY response.
///
......@@ -129,36 +128,38 @@ namespace isc {
///
/// @return REPLY message or NULL
boost::shared_ptr<Pkt6>
processRequest(boost::shared_ptr<Pkt6>& request);
processRequest(boost::shared_ptr<Pkt6> request);
/// @brief Stub function that will handle incoming RENEW messages.
boost::shared_ptr<Pkt6>
processRenew(boost::shared_ptr<Pkt6>& renew);
processRenew(boost::shared_ptr<Pkt6> renew);
/// @brief Stub function that will handle incoming REBIND messages.
boost::shared_ptr<Pkt6>
processRebind(boost::shared_ptr<Pkt6>& rebind);
processRebind(boost::shared_ptr<Pkt6> rebind);
/// @brief Stub function that will handle incoming CONFIRM messages.
boost::shared_ptr<Pkt6>
processConfirm(boost::shared_ptr<Pkt6>& confirm);
processConfirm(boost::shared_ptr<Pkt6> confirm);
/// @brief Stub function that will handle incoming RELEASE messages.
boost::shared_ptr<Pkt6>
processRelease(boost::shared_ptr<Pkt6>& release);
processRelease(boost::shared_ptr<Pkt6> release);
/// @brief Stub function that will handle incoming DECLINE messages.
boost::shared_ptr<Pkt6>
processDecline(boost::shared_ptr<Pkt6>& decline);
processDecline(boost::shared_ptr<Pkt6> decline);
/// @brief Stub function that will handle incoming INF-REQUEST messages.
boost::shared_ptr<Pkt6>
processInfRequest(boost::shared_ptr<Pkt6>& infRequest);
processInfRequest(boost::shared_ptr<Pkt6> infRequest);
bool shutdown;
friend class test::Dhcpv6SrvTest_Solicit_basic_Test;
/// indicates if shutdown is in progress. Setting it to true will
/// initiate server shutdown procedure.
volatile bool shutdown;
};
};
}; // namespace isc::dhcp
}; // namespace isc
#endif // DHCP6_SRV_H
......@@ -25,6 +25,7 @@
using namespace std;
using namespace isc;
using namespace isc::asiolink;
using namespace isc::dhcp;
namespace isc {
......@@ -44,14 +45,16 @@ IfaceMgr::instanceCreate() {
IfaceMgr&
IfaceMgr::instance() {
if (instance_ == 0)
if (instance_ == 0) {
instanceCreate();
}
return (*instance_);
}
IfaceMgr::Iface::Iface(const std::string& name, int ifindex)
:name_(name), ifindex_(ifindex), mac_len_(0) {
memset(mac_, 0, 20);
memset(mac_, 0, sizeof(mac_));
}
std::string
......@@ -64,10 +67,11 @@ IfaceMgr::Iface::getFullName() const {
std::string
IfaceMgr::Iface::getPlainMac() const {
ostringstream tmp;
tmp.fill('0');
tmp << hex;
for (int i=0; i<mac_len_; i++) {
tmp.fill('0');
tmp.width(2);
tmp << (hex) << (int) mac_[i];
tmp << mac_[i];
if (i<mac_len_-1) {
tmp << ":";
}
......@@ -108,6 +112,7 @@ IfaceMgr::~IfaceMgr() {
control_buf_ = 0;
control_buf_len_ = 0;
}
control_buf_len_ = 0;
}
void
......@@ -225,18 +230,6 @@ IfaceMgr::getIface(const std::string& ifname) {
return (NULL); // not found
}
/**
* Opens UDP/IPv6 socket and binds it to specific address, interface and port.
*
* @param ifname name of the interface
* @param addr address to be bound.
* @param port UDP port.
* @param mcast Should multicast address also be bound?
*
* @return socket descriptor, if socket creation, binding and multicast
* group join were all successful. -1 otherwise.
*/
int
IfaceMgr::openSocket(const std::string& ifname,
const IOAddress& addr,
......
......@@ -17,34 +17,51 @@
#include <list>
#include <boost/shared_ptr.hpp>
#include <boost/noncopyable.hpp>
#include "asiolink/io_address.h"
#include "dhcp/pkt6.h"
namespace isc {
/**
* IfaceMgr is an interface manager class that detects available network
* interfaces, configured addresses, link-local addresses, and provides
* API for using sockets.
*
*/
class IfaceMgr {
namespace dhcp {
/// @brief handles network interfaces, transmission and reception
///
/// IfaceMgr is an interface manager class that detects available network
/// interfaces, configured addresses, link-local addresses, and provides
/// API for using sockets.
///
class IfaceMgr : public boost::noncopyable {
public:
/// type that defines list of addresses
typedef std::list<isc::asiolink::IOAddress> Addr6Lst;
struct Iface { // TODO: could be a class as well
std::string name_; // network interface name
int ifindex_; // interface index (a value that uniquely indentifies
// an interface
Addr6Lst addrs_;
char mac_[20]; // Infiniband used 20 bytes indentifiers
int mac_len_;
/// maximum MAC address length (Infiniband uses 20 bytes)
static const unsigned int MAX_MAC_LEN = 20;
/// @brief represents a single network interface
///
/// Iface structure represents network interface with all useful
/// information, like name, interface index, MAC address and
/// list of assigned addresses
struct Iface {
/// constructor
Iface(const std::string& name, int ifindex);
/// returns full interface name in format ifname/ifindex
std::string getFullName() const;
/// returns link-layer address a plain text
std::string getPlainMac() const;
int sendsock_; // socket used to sending data
int recvsock_; // socket used for receiving data
std::string name_; /// network interface name
int ifindex_; /// interface index (a value that uniquely
/// indentifies an interface
Addr6Lst addrs_; /// list of assigned addresses
uint8_t mac_[MAX_MAC_LEN]; /// link-layer address
int mac_len_; /// length of link-layer address (usually 6)
int sendsock_; /// socket used to sending data
int recvsock_; /// socket used for receiving data
// next field is not needed, let's keep it in cointainers
};
......@@ -54,31 +71,99 @@ namespace isc {
// also hide it (make it public make tests easier for now)
typedef std::list<Iface> IfaceLst;
/// IfaceMgr is a singleton class. This method returns reference
/// to its sole instance.
///
/// @return the only existing instance of interface manager
static IfaceMgr& instance();
Iface * getIface(int ifindex);
Iface * getIface(const std::string& ifname);
void printIfaces(std::ostream& out = std::cout);
bool send(boost::shared_ptr<Pkt6> pkt);
/// @brief Returns interface with specified interface index
///
/// @param ifindex index of searched interface
///
/// @return interface with requested index (or NULL if no such
/// interface is present)
///
Iface*
getIface(int ifindex);
/// @brief Returns interface with specified interface name
///
/// @param ifname name of searched interface
///
/// @return interface with requested name (or NULL if no such
/// interface is present)
///
Iface*
getIface(const std::string& ifname);
/// debugging method that prints out all available interfaces
///
/// @param out specifies stream to print list of interfaces to
void
printIfaces(std::ostream& out = std::cout);
/// @brief Sends a packet.
///
/// Sends a packet. All parameters regarding interface, destination
/// address are set in pkt object.
///
/// @param pkt packet to be sent
///
/// @return true if sending was successful
///
bool
send(boost::shared_ptr<Pkt6> pkt);
/// @brief Tries to receive packet over open sockets.
///
/// Attempts to receive a single packet of any of the open sockets.
/// If reception is successful and all information about its sender
/// are obtained, Pkt6 object is created and returned.
///
/// @return Pkt6 object representing received packet (or NULL)
///
boost::shared_ptr<Pkt6> receive();
// don't use private, we need derived classes in tests
protected:
IfaceMgr(); // don't create IfaceMgr directly, use instance() method
~IfaceMgr();
void detectIfaces();
/// @brief Protected constructor.
///
/// Protected constructor. This is a singleton class. We don't want
/// anyone to create instances of IfaceMgr. Use instance() method
IfaceMgr();
~IfaceMgr();
/// @brief Detects network interfaces.
///
/// This method will eventually detect available interfaces. For now
/// it offers stub implementation. First interface name and link-local
/// IPv6 address is read from intefaces.txt file.
void
detectIfaces();
///
/// Opens UDP/IPv6 socket and binds it to address, interface nad port.
///
/// @param ifname name of the interface
/// @param addr address to be bound.
/// @param port UDP port.
///
/// @return socket descriptor, if socket creation, binding and multicast
/// group join were all successful. -1 otherwise.
int openSocket(const std::string& ifname,
const isc::asiolink::IOAddress& addr,
int port);
// TODO: having 2 maps (ifindex->iface and ifname->iface would)
// probably be better for performance reasons
/// List of available interfaces
IfaceLst ifaces_;
/// a pointer to a sole instance of this class (a singleton)
static IfaceMgr * instance_;
// TODO: Also keep this interface on Iface once interface detection
......@@ -90,15 +175,38 @@ namespace isc {
// is bound to multicast address. And we all know what happens
// to people who try to use multicast as source address.
/// control-buffer, used in transmission and reception
char * control_buf_;
int control_buf_len_;
private:
bool openSockets();
static void instanceCreate();
bool joinMcast(int sock, const std::string& ifname,
const std::string& mcast);
/// Opens sockets on detected interfaces.
bool
openSockets();
/// creates a single instance of this class (a singleton implementation)
static void
instanceCreate();
/// @brief Joins IPv6 multicast group on a socket.
///
/// Socket must be created and bound to an address. Note that this
/// address is different than the multicast address. For example DHCPv6
/// server should bind its socket to link-local address (fe80::1234...)
/// and later join ff02::1:2 multicast group.
///
/// @param sock socket fd (socket must be bound)
/// @param ifname interface name (for link-scoped multicast groups)
/// @param mcast multicast address to join (e.g. "ff02::1:2")
///
/// @return true if multicast join was successful
///
bool
joinMcast(int sock, const std::string& ifname,
const std::string& mcast);
};
};
}; // namespace isc::dhcp
}; // namespace isc
#endif
......@@ -43,6 +43,7 @@ using namespace isc::config;
using namespace isc::util;
using namespace isc;
using namespace isc::dhcp;
namespace {
......
......@@ -31,6 +31,21 @@ using namespace isc::dhcp;
// Maybe it should be isc::test?
namespace test {
class NakedDhcpv6Srv: public Dhcpv6Srv {
// "naked" Interface Manager, exposes internal fields
public:
NakedDhcpv6Srv() { }
boost::shared_ptr<Pkt6>
processSolicit(boost::shared_ptr<Pkt6>& request) {
return Dhcpv6Srv::processSolicit(request);
}
boost::shared_ptr<Pkt6>
processRequest(boost::shared_ptr<Pkt6>& request) {
return Dhcpv6Srv::processRequest(request);
}
};
class Dhcpv6SrvTest : public ::testing::Test {
public:
Dhcpv6SrvTest() {
......@@ -45,7 +60,6 @@ TEST_F(Dhcpv6SrvTest, basic) {
// srv has stubbed interface detection. It will read
// interfaces.txt instead. It will pretend to have detected
// fe80::1234 link-local address on eth0 interface. Obviously
// an attempt to bind this socket will fail.
EXPECT_NO_THROW( {
Dhcpv6Srv * srv = new Dhcpv6Srv();
......@@ -55,15 +69,21 @@ TEST_F(Dhcpv6SrvTest, basic) {
}
TEST_F(Dhcpv6SrvTest,Solicit_basic) {
Dhcpv6Srv * srv = 0;
EXPECT_NO_THROW( srv = new Dhcpv6Srv(); );
TEST_F(Dhcpv6SrvTest, Solicit_basic) {
NakedDhcpv6Srv * srv = 0;
EXPECT_NO_THROW( srv = new NakedDhcpv6Srv(); );
// a dummy content for client-id
boost::shared_array<char> clntDuid(new char[32]);
for (int i=0; i<32; i++)
clntDuid[i] = 100+i;
boost::shared_ptr<Pkt6> sol =
boost::shared_ptr<Pkt6>(new Pkt6(DHCPV6_SOLICIT,
1234, Pkt6::UDP));
boost::shared_ptr<Option6IA> ia(new Option6IA(Option::V6, D6O_IA_NA, 2345));
boost::shared_ptr<Option6IA> ia =
boost::shared_ptr<Option6IA>(new Option6IA(Option::V6, D6O_IA_NA, 234));
ia->setT1(1501);
ia->setT2(2601);
sol->addOption(ia);
......@@ -74,6 +94,20 @@ TEST_F(Dhcpv6SrvTest,Solicit_basic) {
// ia->addOption(addr);
// sol->addOption(ia);
// constructed very simple SOLICIT message with:
// - client-id option (mandatory)
// - IA option (a request for address, without any addresses)
// expected returned ADVERTISE message:
// - copy of client-id
// - server-id
// - IA that includes IAADDR
boost::shared_ptr<Option> clientid =
boost::shared_ptr<Option>(new Option(Option::V6, D6O_CLIENTID,
clntDuid, 0, 16));
sol->addOption(clientid);
boost::shared_ptr<Pkt6> reply = srv->processSolicit(sol);
// check if we get response at all
......@@ -83,10 +117,26 @@ TEST_F(Dhcpv6SrvTest,Solicit_basic) {
EXPECT_EQ( 1234, reply->getTransid() );
boost::shared_ptr<Option> tmp = reply->getOption(D6O_IA_NA);
ASSERT_TRUE( tmp != boost::shared_ptr<Option>() );
ASSERT_TRUE( tmp );
Option6IA * reply_ia = dynamic_cast<Option6IA*> ( tmp.get() );
EXPECT_EQ( 2345, reply_ia->getIAID() );
EXPECT_EQ( 234, reply_ia->getIAID() );
// check that there's an address included
EXPECT_TRUE( reply_ia->getOption(D6O_IAADDR));
// check that server included our own client-id
tmp = reply->getOption(D6O_CLIENTID);
ASSERT_TRUE( tmp );
EXPECT_EQ(clientid->getType(), tmp->getType() );
ASSERT_EQ(clientid->len(), tmp->len() );
EXPECT_FALSE(memcmp(clientid->getData(), tmp->getData(), tmp->len() ) );
// check that server included its server-id
tmp = reply->getOption(D6O_SERVERID);
EXPECT_EQ(tmp->getType(), srv->getServerID()->getType() );
ASSERT_EQ(tmp->len(), srv->getServerID()->len() );
EXPECT_FALSE( memcmp(tmp->getData(), srv->getServerID()->getData(),
tmp->len()) );
// more checks to be implemented
delete srv;
......
......@@ -27,6 +27,7 @@
using namespace std;
using namespace isc;
using namespace isc::asiolink;
using namespace isc::dhcp;
// name of loopback interface detection
char LOOPBACK[32] = "lo";
......@@ -61,6 +62,9 @@ public:
// is named lo on Linux and lo0 on BSD boxes. We need to find out
// which is available. This is not a real test, but rather a workaround
// that will go away when interface detection is implemented.
// NOTE: At this stage of development, write access to current directory
// during running tests is required.
TEST_F(IfaceMgrTest, loDetect) {
unlink("interfaces.txt");
......@@ -365,7 +369,7 @@ TEST_F(IfaceMgrTest, DISABLED_sendReceive) {
rcvPkt = ifacemgr->receive();