BIND 10 trac2902, updated. 32931752aee5c1c61bf6758a3ad34a77eec04064 [2902] Changes as a result of the second review.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed May 22 15:50:47 UTC 2013
The branch, trac2902 has been updated
via 32931752aee5c1c61bf6758a3ad34a77eec04064 (commit)
from 9a49846cb4456b17526c30799df03c9a06c8a318 (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 32931752aee5c1c61bf6758a3ad34a77eec04064
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed May 22 17:50:33 2013 +0200
[2902] Changes as a result of the second review.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_srv.cc | 2 +-
src/bin/dhcp4/dhcp4_srv.h | 4 +-
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 60 ++++++++++++++---------------
src/lib/dhcp/pkt4.h | 4 ++
4 files changed, 37 insertions(+), 33 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 2290f0f..01e7da7 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -597,7 +597,7 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg) {
// which doesn't have an address assigned. The other case when response
// must be broadcasted is when our server does not support responding
// directly to a client without address assigned.
- bool bcast_flag = (question->getFlags() >> 0xF) ? true : false;
+ const bool bcast_flag = ((question->getFlags() & Pkt4::FLAG_BROADCAST_MASK) != 0);
if (!IfaceMgr::instance().isDirectResponseSupported() || bcast_flag) {
msg->setRemoteAddr(bcast_addr);
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 2f89f3c..f98233c 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -45,7 +45,7 @@ namespace dhcp {
/// Dhcpv4Srv and other classes, see \ref dhcpv4Session.
class Dhcpv4Srv : public boost::noncopyable {
- public:
+public:
/// @brief defines if certain option may, must or must not appear
typedef enum {
@@ -296,7 +296,7 @@ protected:
/// initiate server shutdown procedure.
volatile bool shutdown_;
- private:
+private:
/// @brief Constructs netmask option based on subnet4
/// @param subnet subnet for which the netmask will be calculated
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 399cdbd..1938098 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -70,6 +70,9 @@ public:
/// it is tested with PktFilterLPFTest unittest. The tests which belong
/// to PktFilterLPFTest can be enabled on demand when root privileges can
/// be guaranteed.
+ ///
+ /// @param port port number to listen on; the default value 0 indicates
+ /// that sockets should not be opened.
NakedDhcpv4Srv(uint16_t port = 0)
: Dhcpv4Srv(port, "type=memfile", false, false) {
}
@@ -425,7 +428,7 @@ public:
// Initialize the source HW address.
vector<uint8_t> mac(6);
- for (int i = 0; i < 6; i++) {
+ for (int i = 0; i < 6; ++i) {
mac[i] = i * 10;
}
// Initialized the destination HW address.
@@ -442,7 +445,7 @@ public:
// Set the name of the interface on which packet is received.
req->setIface("eth0");
// Set the interface index. It is just a dummy value and will
- // not be interprented.
+ // not be interpreted.
req->setIndex(17);
// Set the target HW address. This value is normally used to
// construct the data link layer header.
@@ -520,12 +523,32 @@ public:
}
- ~Dhcpv4SrvTest() {
+ /// @brief This function cleans up after the test.
+ virtual void TearDown() {
+
CfgMgr::instance().deleteSubnets4();
// Let's clean up if there is such a file.
unlink(SRVID_FILE);
- };
+
+ // Some unit tests override the default packet filtering class, used
+ // by the IfaceMgr. The dummy class, called PktFilterTest, reports the
+ // capability to directly respond to the clients without IP address
+ // assigned. This capability is not supported by the default packet
+ // filtering class: PktFilterInet. Therefore setting the dummy class
+ // allows to test scenarios, when server responds to the broadcast address
+ // on client's request, despite having support for direct response.
+ // The following call restores the use of original packet filtering class
+ // after the test.
+ try {
+ IfaceMgr::instance().setPacketFilter(PktFilterPtr(new PktFilterInet()));
+
+ } catch (const Exception& ex) {
+ FAIL() << "Failed to restore the default (PktFilterInet) packet filtering"
+ << " class after the test. Exception has been caught: "
+ << ex.what();
+ }
+ }
/// @brief A subnet used in most tests
Subnet4Ptr subnet_;
@@ -599,7 +622,7 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRelay) {
// address will take precedence.
req->setGiaddr(IOAddress("192.0.2.50"));
req->setCiaddr(IOAddress("192.0.2.11"));
- req->setFlags(0x8000);
+ req->setFlags(Pkt4::FLAG_BROADCAST_MASK);
resp->setYiaddr(IOAddress("192.0.2.100"));
// Clear remote address.
@@ -633,7 +656,7 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRenewRebind) {
// when new lease is acquired and server must make a decision
// whether to unicast the response to the acquired address or
// broadcast it.
- req->setFlags(0x8000);
+ req->setFlags(Pkt4::FLAG_BROADCAST_MASK);
// Create a response.
boost::shared_ptr<Pkt4> resp(new Pkt4(DHCPOFFER, 1234));
@@ -705,18 +728,6 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressSelect) {
// address assigned for the client.
ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
- // Since IfaceMgr is a singleton it will keep using the dummy packet
- // filtering class after this test completes. We want to avoid the
- // impact of this test on other tests so we will try to restore the
- // original packet filtering class. If this fails, we have to abort the
- // test because other tests would be otherwise affected.
- try {
- IfaceMgr::instance().setPacketFilter(PktFilterPtr(new PktFilterInet()));
- } catch (const Exception& ex) {
- FAIL() << "Failed to restore the default Packet Filter class"
- << " after replacing it with dummy class used for testing.";
- }
-
EXPECT_EQ("192.0.2.13", resp->getRemoteAddr().toText());
}
@@ -737,7 +748,7 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressBroadcast) {
req->setCiaddr(IOAddress("0.0.0.0"));
// Let's set the broadcast flag.
- req->setFlags(0x8000);
+ req->setFlags(Pkt4::FLAG_BROADCAST_MASK);
// Create a response.
boost::shared_ptr<Pkt4> resp(new Pkt4(DHCPOFFER, 1234));
@@ -763,17 +774,6 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressBroadcast) {
ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
- // Since IfaceMgr is a singleton it will keep using the dummy packet
- // filtering class after this test completes. We want to avoid the
- // impact of this test on other tests so we will try to restore the
- // original packet filtering class. If this fails, we have to abort the
- // test because other tests would be otherwise affected.
- try {
- IfaceMgr::instance().setPacketFilter(PktFilterPtr(new PktFilterInet()));
- } catch (const Exception& ex) {
- FAIL() << "Failed to restore the default Packet Filter class"
- << " after replacing it with dummy class used for testing.";
- }
// Server must repond to broadcast address when client desired that
// by setting the broadcast flag in its request.
EXPECT_EQ("255.255.255.255", resp->getRemoteAddr().toText());
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index 871ad4c..d232745 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -47,6 +47,10 @@ public:
/// specifies DHCPv4 packet header length (fixed part)
const static size_t DHCPV4_PKT_HDR_LEN = 236;
+ /// Mask for the value of flags field in the DHCPv4 message
+ /// to check whether client requested broadcast response.
+ const static uint16_t FLAG_BROADCAST_MASK = 0x8000;
+
/// Constructor, used in replying to a message.
///
/// @param msg_type type of message (e.g. DHCPDISOVER=1)
More information about the bind10-changes
mailing list