[991] Code cleanup.

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -506,19 +506,25 @@ TEST_F(Dhcpv4SrvTest, basic) {
 // engine. See DiscoverBasic, DiscoverHint, DiscoverNoClientId
 // and DiscoverInvalidHint.
 TEST_F(Dhcpv4SrvTest, processDiscoverRelay) {
-    testDiscoverRequest(DHCPDISCOVER, IOAddress("192.0.2.56"), IOAddress("192.0.2.67"));
+    testDiscoverRequest(DHCPDISCOVER,
+                        IOAddress("192.0.2.56"),
+                        IOAddress("192.0.2.67"));
 }
 
 // Verifies that the non-relayed DISCOVER is processed correctly when
 // client source address is specified.
 TEST_F(Dhcpv4SrvTest, processDiscoverNoRelay) {
-    testDiscoverRequest(DHCPDISCOVER, IOAddress("0.0.0.0"), IOAddress("192.0.2.67"));
+    testDiscoverRequest(DHCPDISCOVER,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("192.0.2.67"));
 }
 
 // Verified that the non-relayed DISCOVER is processed correctly when
 // client source address is not specified.
 TEST_F(Dhcpv4SrvTest, processDiscoverNoClientAddr) {
-    testDiscoverRequest(DHCPDISCOVER, IOAddress("0.0.0.0"), IOAddress("0.0.0.0"));
+    testDiscoverRequest(DHCPDISCOVER,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("0.0.0.0"));
 }
 
 // Verifies that REQUEST received via relay can be processed correctly,
@@ -530,19 +536,25 @@ TEST_F(Dhcpv4SrvTest, processDiscoverNoClientAddr) {
 // engine. See DiscoverBasic, DiscoverHint, DiscoverNoClientId
 // and DiscoverInvalidHint.
 TEST_F(Dhcpv4SrvTest, processRequestRelay) {
-    testDiscoverRequest(DHCPREQUEST, IOAddress("192.0.2.56"), IOAddress("192.0.2.67"));
+    testDiscoverRequest(DHCPREQUEST,
+                        IOAddress("192.0.2.56"),
+                        IOAddress("192.0.2.67"));
 }
 
 // Verifies that the non-relayed REQUEST is processed correctly when
 // client source address is specified.
 TEST_F(Dhcpv4SrvTest, processRequestNoRelay) {
-    testDiscoverRequest(DHCPREQUEST, IOAddress("0.0.0.0"), IOAddress("192.0.2.67"));
+    testDiscoverRequest(DHCPREQUEST,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("192.0.2.67"));
 }
 
 // Verified that the non-relayed REQUEST is processed correctly when
 // client source address is not specified.
 TEST_F(Dhcpv4SrvTest, processRequestNoClientAddr) {
-    testDiscoverRequest(DHCPREQUEST, IOAddress("0.0.0.0"), IOAddress("0.0.0.0"));
+    testDiscoverRequest(DHCPREQUEST,
+                        IOAddress("0.0.0.0"),
+                        IOAddress("0.0.0.0"));
 }
 
 TEST_F(Dhcpv4SrvTest, processRelease) {

src/lib/dhcp/iface_mgr.cc

@@ -219,11 +219,12 @@ bool IfaceMgr::openSockets4(const uint16_t port) {
                 continue;
             }
 
-            // Using openSocket4 directly instead of using openSocket to
-            // pass boolean arguments that enable broadcast traffic.
-            sock = openSocket4(*iface, *addr, port, true, true);
+            // Open socket and enable broadcast traffic
+            // (two last arguments enable broadcast).
+            sock = openSocket(iface->getName(), *addr, port, true, true);
             if (sock < 0) {
-                isc_throw(SocketConfigError, "failed to open unicast socket");
+                isc_throw(SocketConfigError, "failed to open IPv4 socket"
+                          << " supporting broadcast traffic");
             }
 
             count++;
@@ -355,13 +356,14 @@ IfaceMgr::getIface(const std::string& ifname) {
 }
 
 int IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr,
-                         const uint16_t port) {
+                         const uint16_t port, const bool receive_bcast,
+                         const bool send_bcast) {
     Iface* iface = getIface(ifname);
     if (!iface) {
         isc_throw(BadValue, "There is no " << ifname << " interface present.");
     }
     if (addr.isV4()) {
-        return openSocket4(*iface, addr, port);
+        return openSocket4(*iface, addr, port, receive_bcast, send_bcast);
 
     } else if (addr.isV6()) {
         return openSocket6(*iface, addr, port);
@@ -587,8 +589,12 @@ int IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, uint16_t port) {
     return (sock);
 }
 
-int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, uint16_t port,
-                          bool receive_bcast, bool send_bcast) {
+int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr,
+                          const uint16_t port, const bool receive_bcast,
+                          const bool send_bcast) {
+
+    // Skip checking if the packet_filter_ is non-NULL because this check
+    // has been already done when packet filter object was set.
 
     int sock = packet_filter_->openSocket(iface, addr, port,
                                           receive_bcast, send_bcast);
@@ -709,6 +715,8 @@ IfaceMgr::send(const Pkt4Ptr& pkt) {
                   << pkt->getIface() << ") specified.");
     }
 
+    // Skip checking if packet filter is non-NULL because it has been
+    // already checked when packet filter was set.
     return (packet_filter_->send(getSocket(*pkt), pkt));
 }
 
@@ -807,6 +815,8 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
     }
 
     // Now we have a socket, let's get some data from it!
+    // Skip checking if packet filter is non-NULL because it has been
+    // already checked when packet filter was set.
     return (packet_filter_->receive(*iface, *candidate));
 }
 

src/lib/dhcp/iface_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -316,7 +316,7 @@ public:
     /// @return the only existing instance of interface manager
     static IfaceMgr& instance();
 
-    /// @brief Can be packet can be send directly to the client without address.
+    /// @brief Check if packet be sent directly to the client having no address.
     ///
     /// Checks if IfaceMgr can send DHCPv4 packet to the client
     /// who hasn't got address assigned. If this is not supported
@@ -454,6 +454,10 @@ public:
     /// @param ifname name of the interface
     /// @param addr address to be bound.
     /// @param port UDP port.
+    /// @param receive_bcast configure IPv4 socket to receive broadcast messages.
+    /// This parameter is ignored for IPv6 sockets.
+    /// @param send_bcast configure IPv4 socket to send broadcast messages.
+    /// This parameter is ignored for IPv6 sockets.
     ///
     /// Method will throw if socket creation, socket binding or multicast
     /// join fails.
@@ -462,7 +466,9 @@ public:
     /// group join were all successful.
     int openSocket(const std::string& ifname,
                    const isc::asiolink::IOAddress& addr,
-                   const uint16_t port);
+                   const uint16_t port,
+                   const bool receive_bcast = false,
+                   const bool send_bcast = false);
 
     /// @brief Opens UDP/IP socket and binds it to interface specified.
     ///
@@ -524,10 +530,6 @@ public:
     /// @return true if any sockets were open
     bool openSockets6(const uint16_t port = DHCP6_SERVER_PORT);
 
-    /// @brief Closes all open sockets.
-    /// Is used in destructor, but also from Dhcpv4_srv and Dhcpv6_srv classes.
-    void closeSockets();
-
     /// Opens IPv4 sockets on detected interfaces.
     /// Will throw exception if socket creation fails.
     ///
@@ -537,6 +539,10 @@ public:
     /// @return true if any sockets were open
     bool openSockets4(const uint16_t port = DHCP4_SERVER_PORT);
 
+    /// @brief Closes all open sockets.
+    /// Is used in destructor, but also from Dhcpv4_srv and Dhcpv6_srv classes.
+    void closeSockets();
+
     /// @brief returns number of detected interfaces
     ///
     /// @return number of detected interfaces
@@ -554,6 +560,17 @@ public:
         session_callback_ = callback;
     }
 
+    /// @brief Set Packet Filter object to handle send/receive packets.
+    ///
+    /// Packet Filters expose low-level functions handling sockets opening
+    /// and sending/receiving packets through those sockets. This function
+    /// sets custom Packet Filter (represented by a class derived from PktFilter)
+    /// to be used by IfaceMgr.
+    ///
+    /// @param packet_filter new packet filter to be used by IfaceMgr to send/receive
+    /// packets and open sockets.
+    ///
+    /// @throw InvalidPacketFilter if provided packet filter object is NULL.
     void setPacketFilter(const boost::shared_ptr<PktFilter>& packet_filter) {
         if (!packet_filter) {
             isc_throw(InvalidPacketFilter, "NULL packet filter object specified");
@@ -588,8 +605,9 @@ protected:
     /// @param send_bcast configure socket to send broadcast messages.
     ///
     /// @return socket descriptor
-    int openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port,
-                    bool receive_bcast = false, bool send_bcast = false);
+    int openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr,
+                    const uint16_t port, const bool receive_bcast = false,
+                    const bool send_bcast = false);
 
     /// @brief Opens IPv6 socket.
     ///
@@ -709,7 +727,14 @@ private:
     getLocalAddress(const isc::asiolink::IOAddress& remote_addr,
                     const uint16_t port);
 
-    /// Low level packet handler used to open socket and send/receive packets.
+    /// Holds instance of a class derived from PktFilter, used by the
+    /// IfaceMgr to open sockets and send/receive packets through these
+    /// sockets. It is possible to supply custom object using
+    /// setPacketFilter class. Various Packet Filters differ mainly by using
+    /// different types of sockets, e.g. SOCK_DGRAM,  SOCK_RAW and different
+    /// families, e.g. AF_INET, AF_PACKET etc. Another possible type of
+    /// Packet Filter is the one used for unit testing, which doesn't
+    /// open sockets but rather mimics their behavior (mock object).
     boost::shared_ptr<PktFilter> packet_filter_;
 };
 

src/lib/dhcp/pkt_filter_lpf.cc

@@ -24,18 +24,20 @@ int
 PktFilterLPF::openSocket(const Iface&, const isc::asiolink::IOAddress&,
                          const uint16_t, const bool,
                          const bool) {
-
-    return (-1);
+    isc_throw(isc::NotImplemented,
+              "Linux Packet Filtering is not implemented yet");
 }
 
 Pkt4Ptr
 PktFilterLPF::receive(const Iface&, const SocketInfo&) {
-    return (Pkt4Ptr());
+    isc_throw(isc::NotImplemented,
+              "Linux Packet Filtering is not implemented yet");
 }
 
 int
 PktFilterLPF::send(uint16_t, const Pkt4Ptr&) {
-    return (-1);
+    isc_throw(isc::NotImplemented,
+              "Linux Packet Filtering is not implemented yet");
 }
 
 

src/lib/dhcp/pkt_filter_lpf.h

@@ -24,6 +24,9 @@ namespace dhcp {
 ///
 /// This class provides methods to send and recive packet using raw sockets
 /// and Linux Packet Filtering.
+///
+/// @warning This class is not implemented yet. Therefore all functions
+/// currently throw isc::NotImplemented exception.
 class PktFilterLPF : public PktFilter {
 public:
 
@@ -35,6 +38,7 @@ public:
     /// @param receive_bcast configure socket to receive broadcast messages
     /// @param send_bcast configure socket to send broadcast messages.
     ///
+    /// @throw isc::NotImplemented always
     /// @return created socket's descriptor
     virtual int openSocket(const Iface& iface,
                            const isc::asiolink::IOAddress& addr,
@@ -47,6 +51,7 @@ public:
     /// @param iface interface
     /// @param socket_info structure holding socket information
     ///
+    /// @throw isc::NotImplemented always
     /// @return Received packet
     virtual Pkt4Ptr receive(const Iface& iface, const SocketInfo& socket_info);
 
@@ -55,6 +60,7 @@ public:
     /// @param sockfd socket descriptor
     /// @param pkt packet to be sent
     ///
+    /// @throw isc::NotImplemented always
     /// @return result of sending a packet. It is 0 if successful.
     virtual int send(uint16_t sockfd, const Pkt4Ptr& pkt);