BIND 10 trac2765, updated. ddf389fa6171e8b6b57bf8b359d5b2db45727dc1 [2765] Addressed review comments.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Dec 3 12:58:50 UTC 2013
The branch, trac2765 has been updated
via ddf389fa6171e8b6b57bf8b359d5b2db45727dc1 (commit)
via 9fdc1f5af7844126fdc5b578110ab91879666132 (commit)
from fe717f9cfeebf48714de35b1c8f4b1ce323ca67f (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit ddf389fa6171e8b6b57bf8b359d5b2db45727dc1
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Dec 3 13:58:31 2013 +0100
[2765] Addressed review comments.
commit 9fdc1f5af7844126fdc5b578110ab91879666132
Author: Marcin Siodelski <marcin at isc.org>
Date: Mon Dec 2 11:58:18 2013 +0100
[2765] Updated Developer's guide with the section about raw sockets use.
-----------------------------------------------------------------------
Summary of changes:
doc/devel/mainpage.dox | 1 +
src/lib/dhcp/iface_mgr.cc | 4 +-
src/lib/dhcp/iface_mgr.h | 70 ++++++++++++++++++++++++++----
src/lib/dhcp/libdhcp++.dox | 50 ++++++++++++++++++++-
src/lib/dhcp/pkt_filter.cc | 21 +++++----
src/lib/dhcp/pkt_filter_inet.h | 8 ++++
src/lib/dhcp/pkt_filter_lpf.cc | 13 ++++--
src/lib/dhcp/tests/iface_mgr_unittest.cc | 4 +-
8 files changed, 146 insertions(+), 25 deletions(-)
-----------------------------------------------------------------------
diff --git a/doc/devel/mainpage.dox b/doc/devel/mainpage.dox
index 2f4b339..c85615d 100644
--- a/doc/devel/mainpage.dox
+++ b/doc/devel/mainpage.dox
@@ -66,6 +66,7 @@
* - @subpage libdhcpIntro
* - @subpage libdhcpRelay
* - @subpage libdhcpIfaceMgr
+ * - @subpage libdhcpPktFilter
* - @subpage libdhcpsrv
* - @subpage leasemgr
* - @subpage cfgmgr
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index bd13598..44934ec 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -89,7 +89,7 @@ Iface::closeSockets(const uint16_t family) {
// next one.
close(sock->sockfd_);
// Close fallback socket if open.
- if (sock->fallbackfd_) {
+ if (sock->fallbackfd_ >= 0) {
close(sock->fallbackfd_);
}
sockets_.erase(sock++);
@@ -153,7 +153,7 @@ bool Iface::delSocket(uint16_t sockfd) {
if (sock->sockfd_ == sockfd) {
close(sockfd);
// Close fallback socket if open.
- if (sock->fallbackfd_) {
+ if (sock->fallbackfd_ >= 0) {
close(sock->fallbackfd_);
}
sockets_.erase(sock);
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index d308d2a..0302e24 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -617,7 +617,7 @@ public:
const uint16_t port);
- /// Opens IPv6 sockets on detected interfaces.
+ /// @brief Opens IPv6 sockets on detected interfaces.
///
/// @todo This function will throw an exception immediately when a socket
/// fails to open. This is undersired behavior because it will preclude
@@ -638,16 +638,64 @@ public:
/// @return true if any sockets were open
bool openSockets6(const uint16_t port = DHCP6_SERVER_PORT);
- /// Opens IPv4 sockets on detected interfaces.
+ /// @brief Opens IPv4 sockets on detected interfaces.
+ ///
+ /// This function attempts to open sockets on all interfaces which have been
+ /// detected by @c IfaceMgr and meet the following conditions:
+ /// - interface is not a local loopback,
+ /// - interface is running (connected),
+ /// - interface is up,
+ /// - interface is active, e.g. selected from the configuration to be used
+ /// to listen DHCPv4 messages,
+ /// - interface has an IPv4 address assigned.
+ ///
+ /// The type of the socket being open depends on the selected Packet Filter
+ /// represented by a class derived from @c isc::dhcp::PktFilter abstract
+ /// class.
+ ///
+ /// It is possible to specify whether sockets should be broadcast capable.
+ /// In most of the cases, the sockets should support broadcast traffic, e.g.
+ /// DHCPv4 server and relay need to listen to broadcast messages sent by
+ /// clients. If the socket has to be open on the particular interface, this
+ /// interface must have broadcast flag set. If this condition is not met,
+ /// the socket will be created in the unicast-only mode. If there are
+ /// multiple broadcast-capable interfaces present, they may be all open
+ /// in a broadcast mode only if the OS supports SO_BINDTODEVICE (bind socket
+ /// to a device) socket option. If this option is not supported, only the
+ /// first broadcast-capable socket will be opened in the broadcast mode.
+ /// The error will be reported for sockets being opened on other interfaces.
+ /// If the socket is bound to a device (interface), the broadcast traffic
+ /// sent to this interface will be received on this interface only.
+ /// This allows the DHCPv4 server or relay to detect the interface on which
+ /// the broadcast message has been received. This interface is later used
+ /// to send a response.
+ ///
+ /// On the systems with multiple interfaces, it is often desired that the
+ /// failure to open a socket on a particular interface doesn't cause a
+ /// fatal error and sockets should be opened on remaining interfaces.
+ /// However, the warning about the failure for the particular socket should
+ /// be communicated to the caller. The libdhcp++ is a common library with
+ /// no logger associated with it. Most of the functions in this library
+ /// communicate errors via exceptions. In case of openSockets4 function
+ /// exception must not be thrown if the function is supposed to continue
+ /// opening sockets, despite an error. Therefore, if such a behavior is
+ /// desired, the error handler function can be passed as a parameter.
+ /// This error handler is called (if present) with an error string.
+ /// Typically, error handler will simply log an error using an application
+ /// logger, but it can do more sophisticated error handling too.
+ ///
+ /// @todo It is possible that additional parameters will have to be added
+ /// to the error handler, e.g. Iface if it was really supposed to do
+ /// some more sophisticated error handling.
+ ///
+ /// If the error handler is not installed (is NULL), the exception is thrown
+ /// for each failure (default behavior).
///
/// @param port specifies port number (usually DHCP4_SERVER_PORT)
/// @param use_bcast configure sockets to support broadcast messages.
- /// @param error_handler A pointer to a callback function which is called
- /// by the openSockets4 when it fails to open a socket. This parameter
- /// can be NULL to indicate that the callback should not be used. In such
- /// case the @c isc::dhcp::SocketConfigError exception is thrown instead.
- /// When a callback is installed the function will continue when callback
- /// returns control.
+ /// @param error_handler A pointer to an error handler function which is
+ /// called by the openSockets4 when it fails to open a socket. This
+ /// parameter can be NULL to indicate that the callback should not be used.
///
/// @throw SocketOpenFailure if tried and failed to open socket and callback
/// function hasn't been specified.
@@ -883,13 +931,17 @@ private:
getLocalAddress(const isc::asiolink::IOAddress& remote_addr,
const uint16_t port);
- /// @brief Handles an error which occurs during operation on the socket.
+ /// @brief Handles an error which occurs during configuration of a socket.
///
/// If the handler callback is specified (non-NULL), this handler is
/// called and the specified error message is passed to it. If the
/// handler is not specified, the @c isc::dhcpSocketConfigError exception
/// is thrown with the specified message.
///
+ /// This function should be called to handle errors which occur during
+ /// socket opening, binding or configuration (e.g. setting socket options
+ /// etc).
+ ///
/// @param errmsg An error message to be passed to a handlder function or
/// to the @c isc::dhcp::SocketConfigError exception.
/// @param handler An error handler function or NULL.
diff --git a/src/lib/dhcp/libdhcp++.dox b/src/lib/dhcp/libdhcp++.dox
index eabc392..34993af 100644
--- a/src/lib/dhcp/libdhcp++.dox
+++ b/src/lib/dhcp/libdhcp++.dox
@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-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
@@ -137,4 +137,52 @@ Another useful methods are dedicated to transmission
Note that receive4() and receive6() methods may return NULL, e.g.
when timeout is reached or if dhcp daemon receives a signal.
+ at section libdhcpPktFilter Switchable Packet Filter objects used by Interface Manager
+
+The well known problem of DHCPv4 implementation is that it must be able to
+provision devices which don't have an IPv4 address yet (the IPv4 address is
+one of the configuration parameters provided by DHCP server to a client).
+One way to communicate with such a device is to send server's response to
+a broadcast address. An obvious drawback of this approach is that the server's
+response will be received and processed by all clients in the particular
+network. Therefore, the preferred approach is that the server unicasts its
+response to a new address being assigned for the client. This client will
+identify itself as a target of this message by checking chaddr and/or
+Client Identifier value. In the same time, the other clients in the network
+will not receive the unicast message. The major problem that arises with this
+approach is that the client without an IP address doesn't respond to ARP
+messages. As a result, server's response will not be sent over IP/UDP
+socket because the system kernel will fail to resolve client's link-layer
+address.
+
+Kea supports the use of raw sockets to create a complete Data-link/IP/UDP/DHCPv4
+stack. By creating each layer of the outgoing packet, the Kea logic has full
+control over the frame contents and it may bypass the use of ARP to inject the
+link layer address into the frame. The raw socket is bound to a specific interface,
+not to the IP address/UDP port. Therefore, the system kernel doesn't have
+means to verify that Kea is listening to the DHCP traffic on the specific address
+and port. This has two major implications:
+- It is possible to run another DHCPv4 sever instance which will bind socket to the
+same address and port.
+- An attempt to send a unicast message to the DHCPv4 server will result in ICMP
+"Port Unreachable" message being sent by the kernel (which is unaware that the
+DHCPv4 service is actually running).
+In order to overcome these issues, the isc::dhcp::PktFilterLPF opens a
+regular IP/UDP socket which coexists with the raw socket. The socket is referred
+to as "fallback socket" in the Kea code. All packets received through this socket
+are discarded.
+
+In general, the use of datagram sockets is preferred over raw sockets.
+For convenience, the switchable Packet Filter objects are used to manage
+sockets for different purposes. These objects implement the socket opening
+operation and sending/receiving messages over this socket. For example:
+the isc::dhcp::PktFilterLPF object opens a raw socket.
+The isc::dhcp::PktFilterLPF::send and isc::dhcp::PktFilterLPF::receive
+methods encode/decode full data-link/IP/UDP/DHCPv4 stack. The
+isc::dhcp::PktFilterInet supports sending and receiving messages over
+the regular IP/UDP socket. The isc::dhcp::PktFilterInet should be used in all
+cases when an application using the libdhcp++ doesn't require sending
+DHCP messages to a device which doesn't have an address yet.
+
+
*/
diff --git a/src/lib/dhcp/pkt_filter.cc b/src/lib/dhcp/pkt_filter.cc
index 1dfa0a0..9c1995d 100644
--- a/src/lib/dhcp/pkt_filter.cc
+++ b/src/lib/dhcp/pkt_filter.cc
@@ -28,8 +28,9 @@ PktFilter::openFallbackSocket(const isc::asiolink::IOAddress& addr,
// Create socket.
int sock = socket(AF_INET, SOCK_DGRAM, 0);
if (sock < 0) {
- isc_throw(SocketConfigError, "failed to create fallback socket for address "
- << addr.toText() << ", port " << port);
+ isc_throw(SocketConfigError, "failed to create fallback socket for"
+ " address " << addr.toText() << ", port " << port
+ << ", reason: " << strerror(errno));
}
// Bind the socket to a specified address and port.
struct sockaddr_in addr4;
@@ -38,21 +39,23 @@ PktFilter::openFallbackSocket(const isc::asiolink::IOAddress& addr,
addr4.sin_addr.s_addr = htonl(addr);
addr4.sin_port = htons(port);
- if (bind(sock, reinterpret_cast<struct sockaddr*>(&addr4), sizeof(addr4)) < 0) {
+ if (bind(sock, reinterpret_cast<struct sockaddr*>(&addr4),
+ sizeof(addr4)) < 0) {
// Remember to close the socket if we failed to bind it.
close(sock);
- isc_throw(SocketConfigError, "failed to bind fallback socket to address "
- << addr.toText() << ", port " << port << " - is another DHCP "
- "server running?");
+ isc_throw(SocketConfigError, "failed to bind fallback socket to"
+ " address " << addr.toText() << ", port " << port
+ << ", reason: " << strerror(errno)
+ << " - is another DHCP server running?");
}
- // Set socket to non-blocking mode. This is to prevent the read from the fallback
- // socket to block message processing on the primary socket.
+ // Set socket to non-blocking mode. This is to prevent the read from the
+ // fallback socket to block message processing on the primary socket.
if (fcntl(sock, F_SETFL, O_NONBLOCK) != 0) {
close(sock);
isc_throw(SocketConfigError, "failed to set SO_NONBLOCK option on the"
" fallback socket, bound to " << addr.toText() << ", port "
- << port);
+ << port << ", reason: " << strerror(errno));
}
// Successfully created and bound a fallback socket. Return a descriptor.
return (sock);
diff --git a/src/lib/dhcp/pkt_filter_inet.h b/src/lib/dhcp/pkt_filter_inet.h
index 0a506f0..690622c 100644
--- a/src/lib/dhcp/pkt_filter_inet.h
+++ b/src/lib/dhcp/pkt_filter_inet.h
@@ -53,6 +53,8 @@ public:
/// @param send_bcast Configure socket to send broadcast messages.
///
/// @return A structure describing a primary and fallback socket.
+ /// @throw isc::dhcp::SocketConfigError if error occurs when opening,
+ /// binding or configuring the socket.
virtual SocketInfo openSocket(const Iface& iface,
const isc::asiolink::IOAddress& addr,
const uint16_t port,
@@ -65,6 +67,10 @@ public:
/// @param socket_info structure holding socket information
///
/// @return Received packet
+ /// @throw isc::dhcp::SocketReadError if an error occurs during reception
+ /// of the packet.
+ /// @throw An execption thrown by the isc::dhcp::Pkt4 object if DHCPv4
+ /// message parsing fails.
virtual Pkt4Ptr receive(const Iface& iface, const SocketInfo& socket_info);
/// @brief Send packet over specified socket.
@@ -74,6 +80,8 @@ public:
/// @param pkt packet to be sent
///
/// @return result of sending a packet. It is 0 if successful.
+ /// @throw isc::dhcp::SocketWriteError if an error occures during sending
+ /// a DHCP message through the socket.
virtual int send(const Iface& iface, uint16_t sockfd,
const Pkt4Ptr& pkt);
diff --git a/src/lib/dhcp/pkt_filter_lpf.cc b/src/lib/dhcp/pkt_filter_lpf.cc
index 315fa7b..426d58c 100644
--- a/src/lib/dhcp/pkt_filter_lpf.cc
+++ b/src/lib/dhcp/pkt_filter_lpf.cc
@@ -157,8 +157,7 @@ PktFilterLPF::openSocket(const Iface& iface,
<< "' to interface '" << iface.getName() << "'");
}
- SocketInfo sock_desc(addr, port, sock, fallback);
- return (sock_desc);
+ return (SocketInfo(addr, port, sock, fallback));
}
@@ -171,10 +170,18 @@ PktFilterLPF::receive(const Iface& iface, const SocketInfo& socket_info) {
// end after receiving one packet. The call to recv returns immediately
// when there is no data left on the socket because the socket is
// non-blocking.
+ // @todo In the normal conditions, both the primary socket and the fallback
+ // socket are in sync as they are set to receive packets on the same
+ // address and port. The reception of packets on the fallback socket
+ // shouldn't cause significant lags in packet reception. If we find in the
+ // future that it does, the sort of threshold could be set for the maximum
+ // bytes received on the fallback socket in a single round. Further
+ // optimizations would include an asynchronous read from the fallback socket
+ // when the DHCP server is idle.
int datalen;
do {
datalen = recv(socket_info.fallbackfd_, raw_buf, sizeof(raw_buf), 0);
- } while (datalen >= 0);
+ } while (datalen > 0);
// Now that we finished getting data from the fallback socket, we
// have to get the data from the raw socket too.
diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index e7fb2bd..d020db8 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -1107,7 +1107,7 @@ TEST_F(IfaceMgrTest, setMatchingPacketFilter) {
TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) {
IOAddress loAddr("127.0.0.1");
- int socket1 = 0, socket2 = 0;
+ int socket1 = -1, socket2 = -1;
// Create two instances of IfaceMgr.
boost::scoped_ptr<NakedIfaceMgr> iface_mgr1(new NakedIfaceMgr());
ASSERT_TRUE(iface_mgr1);
@@ -1138,6 +1138,8 @@ TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) {
// to prevent resource leak.
if (socket2 >= 0) {
close(socket2);
+ ADD_FAILURE() << "Two sockets opened and bound to the same IP"
+ " address and UDP port";
}
if (socket1 >= 0) {
More information about the bind10-changes
mailing list