Browse Source

[1230] Changes after review:

- ChangeLog from master merged.
- dhcp4_srv cleanup (echo remnants removed, assignLease renamed)
- Pkt4::unpack() now is void and throws in case of problems
Tomek Mrugalski 13 years ago
parent
commit
aac05f566c

+ 26 - 5
ChangeLog

@@ -1,16 +1,37 @@
-3XX.	[func]		tomek
+363.	[func]		tomek
+	dhcp4: Support for DISCOVER and OFFER implemented. b10-dhcp4 is
+	now able to offer hardcoded leases to DHCPv4 clients.
+	dhcp6: Code refactored to use the same approach as dhcp4.
+	(Trac #1230, git TBD)
+
+362.	[func]		tomek
 	libdhcp++: Interface detection in Linux implemented. libdhcp++
 	if now able to detect available network interfaces, its link-layer
 	addresses, flags and configured IPv4 and IPv6 addresses.
-	(Trac #1237, git TBD)
+	(Trac #1237, git 8a040737426aece7cc92a795f2b712d7c3407513)
 
-3XX.	[func]		tomek
+361.	[func]		tomek
 	libdhcp++: Transmission and reception of DHCPv4 packets is now
 	implemented. Low-level hacks are not implemented for transmission
 	to hosts that don't have IPv4 address yet, so currently the code
 	is usable for communication with relays only, not hosts on the
 	same link.
-	(Trac #1239, #1240, git TBD)
+	(Trac #1239, #1240, git f382050248b5b7ed1881b086d89be2d9dd8fe385)
+
+360.	[func] 		fdupont
+	Alpha version of DHCP benchmarking tool added.  "perfdhcp" is able to
+	test both IPv4 and IPv6 servers: it can time the four-packet exchange
+	(DORA and SARR) as well as time the initial two-packet exchange (DO and
+	SA).  More information can be obtained by invoking the utility (in
+	tests/tools/perfdhcp) with the "-h" flag.
+	(Trac #1450, git 85083a76107ba2236732b45524ce7018eefbaf90)
+
+359.	[func]*		vorner
+	The target parameter of ZoneFinder::find is no longer present, as the
+	interface was awkward. To get all the RRsets of a single domain, use
+	the new findAll method (the same applies to python version, the method
+	is named find_all).
+	(Trac #1483,#1484, git 0020456f8d118c9f3fd6fc585757c822b79a96f6)
 
 349.	[bug]		dvv
 	resolver: If an upstream server responds with FORMERR to an EDNS query,
@@ -32,7 +53,7 @@
 	values, and better errors if they are bad.
 	(Trac #1414, git 7b122af8489acf0f28f935a19eca2c5509a3677f)
 
-346.	[build]*		jreed
+346.	[build]*	jreed
 	Renamed libdhcp to libdhcp++.
 	(Trac #1446, git d394e64f4c44f16027b1e62b4ac34e054b49221d)
 

+ 3 - 15
src/bin/dhcp4/dhcp4_srv.cc

@@ -24,8 +24,6 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 
-// #define ECHO_SERVER
-
 // These are hardcoded parameters. Currently this is a skeleton server that only
 // grants those options and a single, fixed, hardcoded lease.
 const std::string HARDCODED_LEASE = "192.0.2.222"; // assigned lease
@@ -46,7 +44,6 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
     /// @todo: instantiate LeaseMgr here once it is imlpemented.
     IfaceMgr::instance().printIfaces();
 
-    // uncomment this once #1238, #992 and #1239 are merged
     IfaceMgr::instance().openSockets4(port);
 
     setServerID();
@@ -67,12 +64,6 @@ Dhcpv4Srv::run() {
 
         query = IfaceMgr::instance().receive4();
 
-#if defined(ECHO_SERVER)
-        query->repack();
-        IfaceMgr::instance().send(query);
-        continue;
-#endif
-
         if (query) {
             try {
                 query->unpack();
@@ -131,10 +122,7 @@ Dhcpv4Srv::run() {
                 if (rsp->pack()) {
                     cout << "Packet assembled correctly." << endl;
                 }
-#if 1
-                // uncomment this once ticket 1240 is merged.
                 IfaceMgr::instance().send(rsp);
-#endif
             }
         }
 
@@ -219,7 +207,7 @@ void Dhcpv4Srv::appendRequestedOptions(boost::shared_ptr<Pkt4>& msg) {
     msg->addOption(opt);
 }
 
-void Dhcpv4Srv::assignLease(boost::shared_ptr<Pkt4>& msg) {
+void Dhcpv4Srv::tryAssignLease(boost::shared_ptr<Pkt4>& msg) {
     boost::shared_ptr<Option> opt;
 
     // TODO: Implement actual lease assignment here
@@ -251,7 +239,7 @@ Dhcpv4Srv::processDiscover(boost::shared_ptr<Pkt4>& discover) {
     appendDefaultOptions(offer, DHCPOFFER);
     appendRequestedOptions(offer);
 
-    assignLease(offer);
+    tryAssignLease(offer);
 
     return (offer);
 }
@@ -265,7 +253,7 @@ Dhcpv4Srv::processRequest(boost::shared_ptr<Pkt4>& request) {
     appendDefaultOptions(ack, DHCPACK);
     appendRequestedOptions(ack);
 
-    assignLease(ack);
+    tryAssignLease(ack);
 
     return (ack);
 }

+ 2 - 2
src/bin/dhcp4/dhcp4_srv.h

@@ -120,7 +120,7 @@ protected:
     /// @brief Appends options requested by client.
     ///
     /// This method assigns options that were requested by client
-    /// or are enforced by server (sent out to all clients).
+    /// (sent in PRL) or are enforced by server.
     ///
     /// @param msg outgoing message (options will be added here)
     void appendRequestedOptions(boost::shared_ptr<Pkt4>& msg);
@@ -136,7 +136,7 @@ protected:
     /// used fixed, hardcoded lease.
     ///
     /// @param msg OFFER or ACK message (lease options will be added here)
-    void assignLease(boost::shared_ptr<Pkt4>& msg);
+    void tryAssignLease(boost::shared_ptr<Pkt4>& msg);
 
 
     /// @brief Appends default options to a message

+ 1 - 1
src/lib/dhcp/iface_mgr.cc

@@ -223,7 +223,7 @@ bool IfaceMgr::openSockets4(uint16_t port) {
              addr != addrs.end();
              ++addr) {
 
-            // skip IPv4 addresses
+            // skip IPv6 addresses
             if (addr->getFamily() != AF_INET) {
                 continue;
             }

+ 5 - 4
src/lib/dhcp/pkt4.cc

@@ -125,7 +125,8 @@ Pkt4::pack() {
 
     return (true);
 }
-bool
+
+void
 Pkt4::unpack() {
 
     // input buffer (used during message reception)
@@ -156,7 +157,7 @@ Pkt4::unpack() {
         // this is *NOT* DHCP packet. It does not have any DHCPv4 options. In
         // particular, it does not have magic cookie, a 4 byte sequence that
         // differentiates between DHCP and BOOTP packets.
-        return (true);
+        isc_throw(InvalidOperation, "Recevied BOOTP packet. BOOTP is not supported.");
     }
 
     if (bufferIn.getLength() - bufferIn.getPosition() < 4) {
@@ -176,9 +177,9 @@ Pkt4::unpack() {
     bufferIn.readVector(optsBuffer, opts_len);
     LibDHCP::unpackOptions4(optsBuffer, options_);
 
+    // TODO: check will need to be called separately, so hooks can be called after
+    // packet is parsed, but before its content is verified
     check();
-
-    return (true);
 }
 
 void Pkt4::check() {

+ 4 - 3
src/lib/dhcp/pkt4.h

@@ -74,9 +74,8 @@ public:
     /// Will create a collection of option objects that will
     /// be stored in options_ container.
     ///
-    /// @return true, if parsing was successful
-    bool
-    unpack();
+    /// Method with throw exception if packet parsing fails.
+    void unpack();
 
     /// @brief performs sanity check on a packet.
     ///
@@ -86,6 +85,8 @@ public:
     /// reasonable value. This method is expected to grow significantly.
     /// It makes sense to separate unpack() and check() for testing purposes.
     ///
+    /// TODO: It is called from unpack() directly. It should be separated.
+    ///
     /// Method will throw exception if anomaly is found.
     void check();
 

+ 11 - 1
src/lib/dhcp/tests/pkt4_unittest.cc

@@ -244,8 +244,18 @@ TEST(Pkt4Test, fixedFieldsPack) {
 TEST(Pkt4Test, fixedFieldsUnpack) {
     vector<uint8_t> expectedFormat = generateTestPacket2();
 
+    expectedFormat.push_back(0x63); // magic cookie
+    expectedFormat.push_back(0x82);
+    expectedFormat.push_back(0x53);
+    expectedFormat.push_back(0x63);
+
+    expectedFormat.push_back(0x35); // message-type
+    expectedFormat.push_back(0x1);
+    expectedFormat.push_back(0x1);
+
     boost::shared_ptr<Pkt4> pkt(new Pkt4(&expectedFormat[0],
-                                Pkt4::DHCPV4_PKT_HDR_LEN));
+                                         expectedFormat.size()));;
+
 
     EXPECT_NO_THROW(
         pkt->unpack()