[master] Merge branch 'trac4626' (fixed fields in classification)

# Conflicts:
#	src/bin/dhcp4/dhcp4_srv.cc

doc/guide/dhcp4-srv.xml

@@ -1670,9 +1670,7 @@ It is merely echoed by the server
     <section id="dhcp4-client-classifier">
       <title>Client Classification in DHCPv4</title>
       <para>
-      The DHCPv4 server includes support for client classification.  At the
-      current time the capabilities of the classification process are limited
-      but it is expected they will be expanded in the future. For a deeper
+      The DHCPv4 server includes support for client classification.  For a deeper
       discussion of the classification process see <xref linkend="classify"/>.
       </para>
 
@@ -1723,6 +1721,52 @@ It is merely echoed by the server
       </para></note>
 
       <section>
+        <title>Setting fixed fields in Classification</title>
+        <para>
+          It is possible to specify that clients belonging to a particular class
+          should receive packets with specific values in certain fixed fields.
+          In particular, three fixed fields are supported:
+          <command>next-server</command> (that conveys an IPv4 address, which is
+          set in the siaddr field), <command>server-hostname</command> (that
+          conveys a server hostname, can be up to 64 bytes long and will be sent
+          in the sname field) and <command>boot-file-name</command> (that
+          conveys the configuration file, can be up to 128 bytes long and will
+          be sent using file field).
+        </para>
+        <para>
+          Obviously, there are many ways to assign clients to specific classes,
+          but for the iPXE clients the client architecture type option (code 93,
+          defined in RFC4578, Section 2.1) seems to be particularly suited to
+          make the distinction. The following example checks if the client
+          identifies itself as PXE device with architecture EFI x86-64, and
+          sets several fields if it does. See Section 2.1 of the RFC4578 or
+          documentation of your client for specific values.
+        </para>
+          <screen>
+"Dhcp4": {
+    "client-classes": [
+        {
+            "name": "ipxe_efi_x64",
+            "test": "option[93].hex == 0x0009",
+            <userinput>"next-server": "192.0.2.254",
+            "server-hostname": "hal9000",
+            "boot-file-name": "/dev/null"</userinput>
+        },
+        ...
+    ],
+    ...
+          }</screen>
+
+          <para>
+            If there are multiple classes defined and incoming packet is matched
+            to multiple classes, then the classes are processed in the order in
+            which they're defined. In this case the last matching class
+            "overwrites" any values that may have been set by earlier matched
+            classes.
+          </para>
+        </section>
+
+      <section>
         <title>Using Vendor Class Information in Classification</title>
         <para>
         The server checks whether an incoming packet includes the vendor class identifier
@@ -1733,12 +1777,11 @@ It is merely echoed by the server
         </para>
 
         <para>
-        For clients that belong to the VENDOR_CLASS_docsis3.0 class, the siaddr
-        field is set to the value of next-server (if specified in a subnet). If
-        there is a boot-file-name option specified, its value is also set in the
-        file field in the DHCPv4 packet. For eRouter1.0 class, the siaddr is
-        always set to 0.0.0.0. That capability is expected to be moved to
-        an external hook library that will be dedicated to cable modems.
+          Note: Kea 1.0 and earlier versions performed special actions for
+          clients that were in VENDOR_CLASS_docsis3.0. This is no longer the
+          case in Kea 1.1 and newer. In those newer versions the old behavior
+          can be achieved by defining VENDOR_CLASS_docsis3.0 and setting
+          its next-server and boot-file-name values appropriately.
         </para>
 
         <para>

src/bin/dhcp4/dhcp4_messages.mes

@@ -233,11 +233,6 @@ information. The second and third argument contains the packet name
 and type respectively. The fourth argument contains detailed packet
 information.
 
-% DHCP4_DISCOVER_CLASS_PROCESSING_FAILED %1: client class specific processing failed for DHCPDISCOVER
-This debug message means that the server processing that is unique for each
-client class has reported a failure. The response packet will not be sent.
-The argument holds the client and transaction identification information.
-
 % DHCP4_DYNAMIC_RECONFIGURATION initiate server reconfiguration using file: %1, after receiving SIGHUP signal
 This is the info message logged when the DHCPv4 server starts reconfiguration
 as a result of receiving SIGHUP signal.
@@ -314,12 +309,6 @@ will be only able to offer global options - no addresses will be assigned.
 The argument specifies the client and transaction identification
 information.
 
-% DHCP4_INFORM_CLASS_PROCESSING_FAILED %1: client class specific processing failed for DHCPINFORM
-This debug message means that the server processing that is unique for each
-client class has reported a failure. The response packet will not be sent.
-The argument specifies the client and the transaction identification
-information.
-
 % DHCP4_INFORM_DIRECT_REPLY %1: DHCPACK in reply to the DHCPINFORM will be sent directly to %2 over %3
 This debug message is issued when the DHCPACK will be sent directly to the
 client, rather than via a relay. The first argument contains the client
@@ -615,12 +604,6 @@ and the lease. The first argument includes the client and the
 transaction identification information. The second argument specifies
 the leased address.
 
-% DHCP4_REQUEST_CLASS_PROCESSING_FAILED %1: client class specific processing failed for DHCPREQUEST
-This debug message means that the server processing that is unique for each
-client class has reported a failure. The response packet will not be sent.
-The argument contains the client and transaction identification
-information.
-
 % DHCP4_RESPONSE_DATA %1: responding with packet %2 (type %3), packet details: %4
 A debug message including the detailed data about the packet being sent
 to the client. The first argument contains the client and the transaction

src/bin/dhcp4/dhcp4_srv.cc

@@ -153,9 +153,6 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine,
 
             // Check for static reservations.
             alloc_engine->findReservation(*context_);
-
-            // Set siaddr, sname and file.
-            setReservedMessageFields();
         }
     }
 };
@@ -1945,6 +1942,76 @@ Dhcpv4Srv::adjustRemoteAddr(Dhcpv4Exchange& ex) {
     }
 }
 
+void
+Dhcpv4Srv::setFixedFields(Dhcpv4Exchange& ex) {
+    Pkt4Ptr query = ex.getQuery();
+    Pkt4Ptr response = ex.getResponse();
+
+    // Step 1: Start with fixed fields defined on subnet level.
+    Subnet4Ptr subnet = ex.getContext()->subnet_;
+    if (subnet) {
+        IOAddress subnet_next_server = subnet->getSiaddr();
+        if (!subnet_next_server.isV4Zero()) {
+            response->setSiaddr(subnet_next_server);
+        }
+    }
+
+    // Step 2: Try to set the values based on classes.
+    // Any values defined in classes will override those from subnet level.
+    const ClientClasses classes = query->getClasses();
+    if (!classes.empty()) {
+
+        // Let's get class definitions
+        const ClientClassDefMapPtr& defs = CfgMgr::instance().getCurrentCfg()->
+            getClientClassDictionary()->getClasses();
+
+        // Now we need to iterate over the classes assigned to the
+        // query packet and find corresponding class defintions for it.
+        for (ClientClasses::const_iterator name = classes.begin();
+             name != classes.end(); ++name) {
+
+            ClientClassDefMap::const_iterator cl = defs->find(*name);
+            if (cl == defs->end()) {
+                // Let's skip classes that don't have definitions. Currently
+                // these are automatic classes VENDOR_CLASS_something, but there
+                // may be other classes assigned under other circumstances, e.g.
+                // by hooks.
+                continue;
+            }
+
+            IOAddress next_server = cl->second->getNextServer();
+            if (!next_server.isV4Zero()) {
+                response->setSiaddr(next_server);
+            }
+
+            const string& sname = cl->second->getSname();
+            if (!sname.empty()) {
+                // Converting string to (const uint8_t*, size_t len) format is
+                // tricky. reineterpret_cast is not the most elegant solution,
+                // but it does avoid us making unnecessary copy. We will convert
+                // sname and file fields in Pkt4 to string one day and live
+                // will be easier.
+                response->setSname(reinterpret_cast<const uint8_t*>(sname.c_str()),
+                                   sname.size());
+            }
+            
+            const string& filename = cl->second->getFilename();
+            if (!filename.empty()) {
+                // Converting string to (const uint8_t*, size_t len) format is
+                // tricky. reineterpret_cast is not the most elegant solution,
+                // but it does avoid us making unnecessary copy. We will convert
+                // sname and file fields in Pkt4 to string one day and live
+                // will be easier.
+                response->setFile(reinterpret_cast<const uint8_t*>(filename.c_str()),
+                                  filename.size());
+            }
+        }
+    }
+
+    // Step 3: try to set values using HR. Any values coming from there will override
+    // the subnet or class values.
+    ex.setReservedMessageFields();
+}
 
 OptionPtr
 Dhcpv4Srv::getNetmaskOption(const Subnet4Ptr& subnet) {
@@ -1985,6 +2052,10 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
         // them we append them for him.
         appendBasicOptions(ex);
 
+        // Set fixed fields (siaddr, sname, filename) if defined in
+        // the reservation, class or subnet specific configuration.
+        setFixedFields(ex);
+
     } else {
         // If the server can't offer an address, it drops the packet.
         return (Pkt4Ptr());
@@ -1997,14 +2068,6 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 
     appendServerID(ex);
 
-    /// @todo: decide whether we want to add a new hook point for
-    /// doing class specific processing.
-    if (!vendorClassSpecificProcessing(ex)) {
-        /// @todo add more verbosity here
-        LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL, DHCP4_DISCOVER_CLASS_PROCESSING_FAILED)
-            .arg(discover->getLabel());
-    }
-
     return (ex.getResponse());
 }
 
@@ -2040,6 +2103,10 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
         // include in the response. If client did not request
         // them we append them for him.
         appendBasicOptions(ex);
+
+        // Set fixed fields (siaddr, sname, filename) if defined in
+        // the reservation, class or subnet specific configuration.
+        setFixedFields(ex);
     }
 
     // Set the src/dest IP address, port and interface for the outgoing
@@ -2048,14 +2115,6 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 
     appendServerID(ex);
 
-    /// @todo: decide whether we want to add a new hook point for
-    /// doing class specific processing.
-    if (!vendorClassSpecificProcessing(ex)) {
-        /// @todo add more verbosity here
-        LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL, DHCP4_REQUEST_CLASS_PROCESSING_FAILED)
-            .arg(ex.getQuery()->getLabel());
-    }
-
     return (ex.getResponse());
 }
 
@@ -2325,6 +2384,10 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
     appendBasicOptions(ex);
     adjustIfaceData(ex);
 
+    // Set fixed fields (siaddr, sname, filename) if defined in
+    // the reservation, class or subnet specific configuration.
+    setFixedFields(ex);
+
     // There are cases for the DHCPINFORM that the server receives it via
     // relay but will send the response to the client's unicast address
     // carried in the ciaddr. In this case, the giaddr and hops field should
@@ -2343,14 +2406,6 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
     // The DHCPACK must contain server id.
     appendServerID(ex);
 
-    /// @todo: decide whether we want to add a new hook point for
-    /// doing class specific processing.
-    if (!vendorClassSpecificProcessing(ex)) {
-        LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL,
-                  DHCP4_INFORM_CLASS_PROCESSING_FAILED)
-            .arg(inform->getLabel());
-    }
-
     return (ex.getResponse());
 }
 
@@ -2574,17 +2629,8 @@ void Dhcpv4Srv::classifyByVendor(const Pkt4Ptr& pkt, std::string& classes) {
     // is indeed a modem, John B. suggested to check whether chaddr field
     // quals subscriber-id option that was inserted by the relay (CMTS).
     // This kind of logic will appear here soon.
-    if (vendor_class->getValue().find(DOCSIS3_CLASS_MODEM) != std::string::npos) {
-        pkt->addClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_MODEM);
-        classes += string(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_MODEM) + " ";
-    } else
-    if (vendor_class->getValue().find(DOCSIS3_CLASS_EROUTER) != std::string::npos) {
-        pkt->addClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER);
-        classes += string(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER) + " ";
-    } else {
-        pkt->addClass(VENDOR_CLASS_PREFIX + vendor_class->getValue());
-        classes += VENDOR_CLASS_PREFIX + vendor_class->getValue();
-    }
+    pkt->addClass(VENDOR_CLASS_PREFIX + vendor_class->getValue());
+    classes += VENDOR_CLASS_PREFIX + vendor_class->getValue();
 }
 
 void Dhcpv4Srv::classifyPacket(const Pkt4Ptr& pkt) {
@@ -2639,61 +2685,6 @@ void Dhcpv4Srv::classifyPacket(const Pkt4Ptr& pkt) {
     }
 }
 
-bool
-Dhcpv4Srv::vendorClassSpecificProcessing(const Dhcpv4Exchange& ex) {
-
-    Subnet4Ptr subnet = ex.getContext()->subnet_;
-    Pkt4Ptr query = ex.getQuery();
-    Pkt4Ptr rsp = ex.getResponse();
-
-    // If any of those is missing, there is nothing to do.
-    if (!subnet || !query || !rsp) {
-        return (true);
-    }
-
-    if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_MODEM)) {
-
-        // Do not override
-        if (rsp->getSiaddr().isV4Zero()) {
-            // Set next-server. This is TFTP server address. Cable modems will
-            // download their configuration from that server.
-            rsp->setSiaddr(subnet->getSiaddr());
-        }
-
-        // Now try to set up file field in DHCPv4 packet. We will just copy
-        // content of the boot-file option, which contains the same information.
-        const CfgOptionList& co_list = ex.getCfgOptionList();
-        for (CfgOptionList::const_iterator copts = co_list.begin();
-             copts != co_list.end(); ++copts) {
-            OptionDescriptor desc = (*copts)->get("dhcp4", DHO_BOOT_FILE_NAME);
-
-            if (desc.option_) {
-                boost::shared_ptr<OptionString> boot =
-                    boost::dynamic_pointer_cast<OptionString>(desc.option_);
-                if (boot) {
-                    std::string filename = boot->getValue();
-                    rsp->setFile((const uint8_t*)filename.c_str(), filename.size());
-                    break;
-                }
-            }
-        }
-    }
-
-    if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER)) {
-
-        // Do not set TFTP server address for eRouter devices.
-        rsp->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS());
-    }
-
-    // Set up siaddr. Do not override siaddr if host specific value or
-    // vendor class specific value present.
-    if (rsp->getSiaddr().isV4Zero()) {
-        rsp->setSiaddr(subnet->getSiaddr());
-    }
-
-    return (true);
-}
-
 void
 Dhcpv4Srv::startD2() {
     D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();

src/bin/dhcp4/dhcp4_srv.h

@@ -118,6 +118,10 @@ public:
         return (cfg_option_list_);
     }
 
+    /// @brief Sets reserved values of siaddr, sname and file in the
+    /// server's response.
+    void setReservedMessageFields();
+
 private:
 
     /// @brief Copies default parameters from client's to server's message
@@ -150,10 +154,6 @@ private:
     /// host-reservation-identifiers
     void setHostIdentifiers();
 
-    /// @brief Sets reserved values of siaddr, sname and file in the
-    /// server's response.
-    void setReservedMessageFields();
-
     /// @brief Pointer to the allocation engine used by the server.
     AllocEnginePtr alloc_engine_;
     /// @brief Pointer to the DHCPv4 message sent by the client.
@@ -517,6 +517,19 @@ protected:
     /// @param ex DHCPv4 exchange holding the client's message to be checked.
     void appendBasicOptions(Dhcpv4Exchange& ex);
 
+    /// @brief Sets fixed fields of the outgoing packet.
+    ///
+    /// If the incoming packets belongs to a class and that class defines
+    /// next-server, server-hostname or boot-file-name, we need to set the
+    /// siaddr, sname or filename fields in the outgoing packet. Also, those
+    /// values can be defined for subnet or in reservations. The values
+    /// defined in reservation takes precedence over class values, which
+    /// in turn take precedence over subnet values.
+    ///
+    /// @param ex DHCPv4 exchange holding the client's message and the server's
+    ///           response to be adjusted.
+    void setFixedFields(Dhcpv4Exchange& ex);
+
     /// @brief Processes Client FQDN and Hostname Options sent by a client.
     ///
     /// Client may send Client FQDN or Hostname option to communicate its name
@@ -756,18 +769,6 @@ protected:
     /// @param pkt packet to be classified
     void classifyPacket(const Pkt4Ptr& pkt);
 
-    /// @brief Performs packet processing specific to a vendor class
-    ///
-    /// If the selected subnet, query or response in the @c ex object is NULL
-    /// this method returns immediately and returns true.
-    ///
-    /// @note This processing is a likely candidate to be pushed into hooks.
-    ///
-    /// @param ex The exchange holding both the client's message and the
-    /// server's response.
-    /// @return true if successful, false otherwise (will prevent sending response)
-    bool vendorClassSpecificProcessing(const Dhcpv4Exchange& ex);
-
     /// @brief Allocation Engine.
     /// Pointer to the allocation engine that we are currently using
     /// It must be a pointer, because we will support changing engines

src/bin/dhcp4/tests/Makefile.am

@@ -78,6 +78,7 @@ dhcp4_unittests_SOURCES += dhcp4_srv_unittest.cc
 dhcp4_unittests_SOURCES += dhcp4_test_utils.cc dhcp4_test_utils.h
 dhcp4_unittests_SOURCES += direct_client_unittest.cc
 dhcp4_unittests_SOURCES += ctrl_dhcp4_srv_unittest.cc
+dhcp4_unittests_SOURCES += classify_unittest.cc
 dhcp4_unittests_SOURCES += config_parser_unittest.cc
 dhcp4_unittests_SOURCES += fqdn_unittest.cc
 dhcp4_unittests_SOURCES += marker_file.cc

src/bin/dhcp4/tests/classify_unittest.cc

@@ -0,0 +1,268 @@
+// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <config.h>
+#include <asiolink/io_address.h>
+#include <dhcp/dhcp4.h>
+#include <dhcp/tests/iface_mgr_test_config.h>
+#include <dhcp4/tests/dhcp4_client.h>
+#include <dhcp/option_int.h>
+#include <vector>
+
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::dhcp;
+using namespace isc::dhcp::test;
+using namespace std;
+
+namespace {
+
+/// @brief Set of JSON configurations used throughout the classify tests.
+///
+/// - Configuration 0:
+///   - Used for testing direct traffic
+///   - 1 subnet: 10.0.0.0/24
+///   - 1 pool: 10.0.0.10-10.0.0.100
+///   - the following classes defined:
+///     option[93].hex == 0x0009, next-server set to 1.2.3.4
+///     option[93].hex == 0x0007, set server-hostname to deneb
+///     option[93].hex == 0x0006, set boot-file-name to pxelinux.0
+///     option[93].hex == 0x0001, set boot-file-name to ipxe.efi
+const char* CONFIGS[] = {
+    // Configuration 0
+    "{ \"interfaces-config\": {"
+        "   \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"valid-lifetime\": 600,"
+        "\"client-classes\": ["
+        "{"
+        "   \"name\": \"pxe1\","
+        "   \"test\": \"option[93].hex == 0x0009\","
+        "   \"next-server\": \"1.2.3.4\""
+        "},"
+        "{"
+        "   \"name\": \"pxe2\","
+        "   \"test\": \"option[93].hex == 0x0007\","
+        "   \"server-hostname\": \"deneb\""
+        "},"
+        "{"
+        "   \"name\": \"pxe3\","
+        "   \"test\": \"option[93].hex == 0x0006\","
+        "   \"boot-file-name\": \"pxelinux.0\""
+        "},"
+        "{"
+        "   \"name\": \"pxe4\","
+        "   \"test\": \"option[93].hex == 0x0001\","
+        "   \"boot-file-name\": \"ipxe.efi\""
+        "},"
+        "],"
+        "\"subnet4\": [ { "
+        "    \"subnet\": \"10.0.0.0/24\", "
+        "    \"id\": 1,"
+        "    \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]"
+        " } ]"
+    "}"
+
+};
+
+/// @brief Test fixture class for testing classification.
+///
+/// For the time being it covers only fixed fields, but it's going to be
+/// expanded to cover other cases.
+class ClassifyTest : public Dhcpv4SrvTest {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Sets up fake interfaces.
+    ClassifyTest()
+        : Dhcpv4SrvTest(),
+          iface_mgr_test_config_(true) {
+        IfaceMgr::instance().openSockets4();
+    }
+
+    /// @brief Desctructor.
+    ///
+    ~ClassifyTest() {
+    }
+
+    /// @brief Does client exchanges and checks if fixed fields have expected values.
+    ///
+    /// Depending on the value of msgtype (allowed types: DHCPDISCOVER, DHCPREQUEST or
+    /// DHCPINFORM), this method sets up the server, then conducts specified exchange
+    /// and then checks if the response contains expected values of next-server, sname
+    /// and filename fields.
+    ///
+    /// @param config server configuration to be used
+    /// @param msgtype DHCPDISCOVER, DHCPREQUEST or DHCPINFORM
+    /// @param extra_opt option to include in client messages (optional)
+    /// @param exp_next_server expected value of the next-server field
+    /// @param exp_sname expected value of the sname field
+    /// @param exp_filename expected value of the filename field
+    void
+    testFixedFields(const char* config, uint8_t msgtype, const OptionPtr& extra_opt,
+                    const std::string& exp_next_server, const std::string& exp_sname,
+                    const std::string& exp_filename) {
+         Dhcp4Client client(Dhcp4Client::SELECTING);
+
+         // Configure DHCP server.
+         configure(config, *client.getServer());
+
+         if (extra_opt) {
+             client.addExtraOption(extra_opt);
+         }
+
+         switch (msgtype) {
+         case DHCPDISCOVER:
+             client.doDiscover();
+             break;
+         case DHCPREQUEST:
+             client.doDORA();
+             break;
+         case DHCPINFORM:
+             // Preconfigure the client with the IP address.
+             client.createLease(IOAddress("10.0.0.56"), 600);
+
+             client.doInform(false);
+             break;
+         }
+
+         ASSERT_TRUE(client.getContext().response_);
+         Pkt4Ptr resp = client.getContext().response_;
+
+         EXPECT_EQ(exp_next_server, resp->getSiaddr().toText());
+
+         // This is bizarre. If I use Pkt4::MAX_SNAME_LEN in the ASSERT_GE macro,
+         // the linker will complain about it being not defined.
+         const size_t max_sname = Pkt4::MAX_SNAME_LEN;
+
+         ASSERT_GE(max_sname, exp_sname.length());
+         vector<uint8_t> sname(max_sname, 0);
+         memcpy(&sname[0], &exp_sname[0], exp_sname.size());
+         EXPECT_EQ(sname, resp->getSname());
+
+         const size_t max_filename = Pkt4::MAX_FILE_LEN;
+         ASSERT_GE(max_filename, exp_filename.length());
+         vector<uint8_t> filename(max_filename, 0);
+         memcpy(&filename[0], &exp_filename[0], exp_filename.size());
+         EXPECT_EQ(filename, resp->getFile());
+    }
+
+    /// @brief Interface Manager's fake configuration control.
+    IfaceMgrTestConfig iface_mgr_test_config_;
+};
+
+
+// This test checks that an incoming DISCOVER that does not match any classes
+// will get the fixed fields empty.
+TEST_F(ClassifyTest, fixedFieldsDiscoverNoClasses) {
+    testFixedFields(CONFIGS[0], DHCPDISCOVER, OptionPtr(), "0.0.0.0", "", "");
+}
+// This test checks that an incoming REQUEST that does not match any classes
+// will get the fixed fields empty.
+TEST_F(ClassifyTest, fixedFieldsRequestNoClasses) {
+    testFixedFields(CONFIGS[0], DHCPREQUEST, OptionPtr(), "0.0.0.0", "", "");
+}
+// This test checks that an incoming INFORM that does not match any classes
+// will get the fixed fields empty.
+TEST_F(ClassifyTest, fixedFieldsInformNoClasses) {
+    testFixedFields(CONFIGS[0], DHCPINFORM, OptionPtr(), "0.0.0.0", "", "");
+}
+
+
+// This test checks that an incoming DISCOVER that does match a class that has
+// next-server specified will result in a response that has the next-server set.
+TEST_F(ClassifyTest, fixedFieldsDiscoverNextServer) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0009));
+
+    testFixedFields(CONFIGS[0], DHCPDISCOVER, pxe, "1.2.3.4", "", "");
+}
+// This test checks that an incoming REQUEST that does match a class that has
+// next-server specified will result in a response that has the next-server set.
+TEST_F(ClassifyTest, fixedFieldsRequestNextServer) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0009));
+
+    testFixedFields(CONFIGS[0], DHCPREQUEST, pxe, "1.2.3.4", "", "");
+}
+// This test checks that an incoming INFORM that does match a class that has
+// next-server specified will result in a response that has the next-server set.
+TEST_F(ClassifyTest, fixedFieldsInformNextServer) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0009));
+
+    testFixedFields(CONFIGS[0], DHCPINFORM, pxe, "1.2.3.4", "", "");
+}
+
+
+// This test checks that an incoming DISCOVER that does match a class that has
+// server-hostname specified will result in a response that has the sname field set.
+TEST_F(ClassifyTest, fixedFieldsDiscoverHostname) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0007));
+
+    testFixedFields(CONFIGS[0], DHCPDISCOVER, pxe, "0.0.0.0", "deneb", "");
+}
+// This test checks that an incoming REQUEST that does match a class that has
+// server-hostname specified will result in a response that has the sname field set.
+TEST_F(ClassifyTest, fixedFieldsRequestHostname) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0007));
+
+    testFixedFields(CONFIGS[0], DHCPREQUEST, pxe, "0.0.0.0", "deneb", "");
+}
+// This test checks that an incoming INFORM that does match a class that has
+// server-hostname specified will result in a response that has the sname field set.
+TEST_F(ClassifyTest, fixedFieldsInformHostname) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0007));
+
+    testFixedFields(CONFIGS[0], DHCPINFORM, pxe, "0.0.0.0", "deneb", "");
+}
+
+
+// This test checks that an incoming DISCOVER that does match a class that has
+// boot-file-name specified will result in a response that has the filename field set.
+TEST_F(ClassifyTest, fixedFieldsDiscoverFile1) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0006));
+
+    testFixedFields(CONFIGS[0], DHCPDISCOVER, pxe, "0.0.0.0", "", "pxelinux.0");
+}
+// This test checks that an incoming REQUEST that does match a class that has
+// boot-file-name specified will result in a response that has the filename field set.
+TEST_F(ClassifyTest, fixedFieldsRequestFile1) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0006));
+
+    testFixedFields(CONFIGS[0], DHCPREQUEST, pxe, "0.0.0.0", "", "pxelinux.0");
+}
+// This test checks that an incoming INFORM that does match a class that has
+// boot-file-name specified will result in a response that has the filename field set.
+TEST_F(ClassifyTest, fixedFieldsInformFile1) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0006));
+
+    testFixedFields(CONFIGS[0], DHCPDISCOVER, pxe, "0.0.0.0", "", "pxelinux.0");
+}
+
+
+// This test checks that an incoming DISCOVER that does match a different class that has
+// boot-file-name specified will result in a response that has the filename field set.
+TEST_F(ClassifyTest, fixedFieldsDiscoverFile2) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0001));
+
+    testFixedFields(CONFIGS[0], DHCPDISCOVER, pxe, "0.0.0.0", "", "ipxe.efi");
+}
+// This test checks that an incoming REQUEST that does match a different class that has
+// boot-file-name specified will result in a response that has the filename field set.
+TEST_F(ClassifyTest, fixedFieldsRequestFile2) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0001));
+
+    testFixedFields(CONFIGS[0], DHCPREQUEST, pxe, "0.0.0.0", "", "ipxe.efi");
+}
+// This test checks that an incoming INFORM that does match a different class that has
+// boot-file-name specified will result in a response that has the filename field set.
+TEST_F(ClassifyTest, fixedFieldsInformFile2) {
+    OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0001));
+
+    testFixedFields(CONFIGS[0], DHCPINFORM, pxe, "0.0.0.0", "", "ipxe.efi");
+}
+
+
+} // end of anonymous namespace

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1486,8 +1486,13 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsDocsisDefinitions) {
     ASSERT_EQ(0, rcode_);
 }
 
-// Checks if DOCSIS client packets are classified properly
-TEST_F(Dhcpv4SrvTest, docsisClientClassification) {
+/// Checks if DOCSIS client packets are classified properly
+///
+/// @todo: With the change in #4626 the vendorClassSpecificProcessing
+/// code was removed and replaced with generic classification. One day
+/// we should rewrite this test to use classes. It would check that the
+/// classification system can be used for docsis packets.
+TEST_F(Dhcpv4SrvTest, DISABLED_docsisClientClassification) {
 
     NakedDhcpv4Srv srv(0);
 

src/lib/dhcpsrv/client_class_def.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -15,7 +15,8 @@ namespace dhcp {
 ClientClassDef::ClientClassDef(const std::string& name,
                                const ExpressionPtr& match_expr,
                                const CfgOptionPtr& cfg_option)
-    : name_(name), match_expr_(match_expr), cfg_option_(cfg_option) {
+    : name_(name), match_expr_(match_expr), cfg_option_(cfg_option),
+      next_server_(asiolink::IOAddress::IPV4_ZERO_ADDRESS()) {
 
     // Name can't be blank
     if (name_.empty()) {
@@ -33,7 +34,8 @@ ClientClassDef::ClientClassDef(const std::string& name,
 
 ClientClassDef::ClientClassDef(const ClientClassDef& rhs)
     : name_(rhs.name_), match_expr_(ExpressionPtr()),
-      cfg_option_(new CfgOption()) {
+      cfg_option_(new CfgOption()),
+      next_server_(asiolink::IOAddress::IPV4_ZERO_ADDRESS()) {
 
     if (rhs.match_expr_) {
         match_expr_.reset(new Expression());
@@ -43,6 +45,10 @@ ClientClassDef::ClientClassDef(const ClientClassDef& rhs)
     if (rhs.cfg_option_) {
         rhs.cfg_option_->copyTo(*cfg_option_);
     }
+
+    next_server_ = rhs.next_server_;
+    sname_ = rhs.sname_;
+    filename_ = rhs.filename_;
 }
 
 ClientClassDef::~ClientClassDef() {
@@ -86,7 +92,10 @@ ClientClassDef::equals(const ClientClassDef& other) const {
          (*match_expr_ == *(other.match_expr_)))) &&
         ((!cfg_option_ && !other.cfg_option_) ||
         (cfg_option_ && other.cfg_option_ &&
-         (*cfg_option_ == *other.cfg_option_))));
+         (*cfg_option_ == *other.cfg_option_))) &&
+            (next_server_ == other.next_server_) &&
+            (sname_ == other.sname_) &&
+            (filename_ == other.filename_));
 }
 
 std::ostream& operator<<(std::ostream& os, const ClientClassDef& x) {
@@ -114,8 +123,14 @@ ClientClassDictionary::~ClientClassDictionary() {
 void
 ClientClassDictionary::addClass(const std::string& name,
                                 const ExpressionPtr& match_expr,
-                                const CfgOptionPtr& cfg_option) {
+                                const CfgOptionPtr& cfg_option,
+                                asiolink::IOAddress next_server,
+                                const std::string& sname,
+                                const std::string& filename) {
     ClientClassDefPtr cclass(new ClientClassDef(name, match_expr, cfg_option));
+    cclass->setNextServer(next_server);
+    cclass->setSname(sname);
+    cclass->setFilename(filename);
     addClass(cclass);
 }
 

src/lib/dhcpsrv/client_class_def.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -106,6 +106,45 @@ public:
     /// @brief Provides a convenient text representation of the class
     friend std::ostream& operator<<(std::ostream& os, const ClientClassDef& x);
 
+    /// @brief returns next-server value
+    /// @return next-server value
+    const asiolink::IOAddress& getNextServer() const {
+        return (next_server_);
+    }
+
+    /// @brief sets the next-server value
+    ///
+    /// @param addr the value to be set
+    void setNextServer(const asiolink::IOAddress& addr) {
+        next_server_ = addr;
+    }
+
+    /// @brief sets the server-name value
+    ///
+    /// @param sname the value to be set
+    void setSname(const std::string& sname) {
+        sname_ = sname;
+    }
+
+    /// @brief returns server-hostname value
+    /// @return the vector that contains server-hostname (may be empty if not defined)
+    const std::string& getSname() const {
+        return (sname_);
+    }
+
+    /// @brief sets the boot-file-name value
+    ///
+    /// @param filename the value to be set
+    void setFilename(const std::string& filename) {
+        filename_ = filename;
+    }
+
+    /// @brief returns boot-file-name value
+    /// @return the vector that contains boot-file-name (may be empty if not defined)
+    const std::string& getFilename() const {
+        return (filename_);
+    }
+
 private:
     /// @brief Unique text identifier by which this class is known.
     std::string name_;
@@ -116,6 +155,24 @@ private:
 
     /// @brief The option data configuration for this class
     CfgOptionPtr cfg_option_;
+
+    /// @brief Next server field
+    /// If set by the next-server parameter, this value will be set
+    /// in the siaddr field of the DHCPv4 packet.
+    asiolink::IOAddress next_server_;
+
+    /// @brief server-hostname
+    /// If set by the server-hostname parameter, this value will be
+    /// set in the sname field of the DHCPv4 packet.
+    /// This can be up to 64 octets long.
+    std::string sname_;
+
+    /// @brief boot-file-name
+    /// If set by the boot-file-name parameter, this value will be
+    /// set in the file field of the DHCPv4 packet.
+    /// This can be up to 128 octets long.
+    std::string filename_;
+
 };
 
 /// @brief a pointer to an ClientClassDef
@@ -147,12 +204,18 @@ public:
     /// @param name Name to assign to this class
     /// @param match_expr Expression the class will use to determine membership
     /// @param options Collection of options members should be given
+    /// @param next_server next-server value for this class (optional)
+    /// @param sname server-name value for this class (optional)
+    /// @param filename boot-file-name value for this class (optional)
     ///
     /// @throw DuplicateClientClassDef if class already exists within the
     /// dictionary.  See @ref dhcp::ClientClassDef::ClientClassDef() for
     /// others.
     void addClass(const std::string& name, const ExpressionPtr& match_expr,
-                  const CfgOptionPtr& options);
+                  const CfgOptionPtr& options,
+                  asiolink::IOAddress next_server = asiolink::IOAddress("0.0.0.0"),
+                  const std::string& sname = std::string(),
+                  const std::string& filename = std::string());
 
     /// @brief Adds a new class to the list
     ///

src/lib/dhcpsrv/parsers/client_class_def_parser.cc

@@ -9,10 +9,14 @@
 #include <dhcpsrv/client_class_def.h>
 #include <dhcpsrv/parsers/client_class_def_parser.h>
 #include <eval/eval_context.h>
+#include <asiolink/io_address.h>
+#include <asiolink/io_error.h>
 
 #include <boost/foreach.hpp>
 
 using namespace isc::data;
+using namespace isc::asiolink;
+using namespace std;
 
 /// @file client_class_def.cc
 ///
@@ -73,6 +77,7 @@ ClientClassDefParser::ClientClassDefParser(const std::string&,
 
 void
 ClientClassDefParser::build(ConstElementPtr class_def_cfg) {
+
     // Parse the elements that make up the option definition.
     BOOST_FOREACH(ConfigPair param, class_def_cfg->mapValue()) {
         std::string entry(param.first);
@@ -92,6 +97,16 @@ ClientClassDefParser::build(ConstElementPtr class_def_cfg) {
 
             opts_parser.reset(new OptionDataListParser(entry, options_, family));
             parser = opts_parser;
+        } else if (entry == "next-server") {
+            StringParserPtr str_parser(new StringParser(entry, string_values_));
+            parser = str_parser;
+        } else if (entry == "server-hostname") {
+            StringParserPtr str_parser(new StringParser(entry, string_values_));
+            parser = str_parser;
+
+        } else if (entry == "boot-file-name") {
+            StringParserPtr str_parser(new StringParser(entry, string_values_));
+            parser = str_parser;
         } else {
             isc_throw(DhcpConfigError, "invalid parameter '" << entry
                       << "' (" << param.second->getPosition() << ")");
@@ -104,14 +119,45 @@ ClientClassDefParser::build(ConstElementPtr class_def_cfg) {
     std::string name;
     try {
         name = string_values_->getParam("name");
-    } catch (const std::exception& ex) {
-        isc_throw(DhcpConfigError, ex.what() << " ("
-                  << class_def_cfg->getPosition() << ")");
-    }
 
-    try {
+        // Let's parse the next-server field
+        IOAddress next_server("0.0.0.0");
+        string next_server_txt = string_values_->getOptionalParam("next-server", "0.0.0.0");
+        try {
+            next_server = IOAddress(next_server_txt);
+        } catch (const IOError& ex) {
+            isc_throw(DhcpConfigError, "Invalid next-server value specified: '"
+                      << next_server_txt);
+        }
+
+        if (next_server.getFamily() != AF_INET) {
+            isc_throw(DhcpConfigError, "Invalid next-server value: '"
+                      << next_server_txt << "', must be IPv4 address");
+        }
+
+        if (next_server.isV4Bcast()) {
+            isc_throw(DhcpConfigError, "Invalid next-server value: '"
+                      << next_server_txt << "', must not be a broadcast");
+        }
+
+        // Let's try to parse sname
+        string sname = string_values_->getOptionalParam("server-hostname", "");
+        if (sname.length() >= Pkt4::MAX_SNAME_LEN) {
+            isc_throw(DhcpConfigError, "server-hostname must be at most "
+                      << Pkt4::MAX_SNAME_LEN - 1 << " bytes long, it is "
+                      << sname.length());
+        }
+
+        string filename = string_values_->getOptionalParam("boot-file-name", "");
+        if (filename.length() > Pkt4::MAX_FILE_LEN) {
+            isc_throw(DhcpConfigError, "boot-file-name must be at most "
+                      << Pkt4::MAX_FILE_LEN - 1 << " bytes long, it is "
+                      << filename.length());
+        }
+
         // an OptionCollectionPtr
-        class_dictionary_->addClass(name, match_expr_, options_);
+        class_dictionary_->addClass(name, match_expr_, options_, next_server,
+                                    sname, filename);
     } catch (const std::exception& ex) {
         isc_throw(DhcpConfigError, ex.what()
                   << " (" << class_def_cfg->getPosition() << ")");

src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -12,6 +12,7 @@
 #include <dhcp/option_string.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/parsers/client_class_def_parser.h>
+#include <asiolink/io_address.h>
 #include <eval/evaluate.h>
 #include <gtest/gtest.h>
 #include <sstream>
@@ -23,6 +24,7 @@
 
 using namespace isc::data;
 using namespace isc::dhcp;
+using namespace isc::asiolink;
 
 namespace {
 
@@ -583,4 +585,190 @@ TEST_F(ClientClassDefListParserTest, invalidClass) {
                  DhcpConfigError);
 }
 
+// Test verifies that without any class specified, the fixed fields have their
+// default, empty value.
+TEST_F(ClientClassDefParserTest, noFixedFields) {
+
+    std::string cfg_text =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+
+    ClientClassDefPtr cclass;
+    ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, Option::V4));
+
+    // We should find our class.
+    ASSERT_TRUE(cclass);
+
+    // And it should not have any fixed fields set
+    EXPECT_EQ(IOAddress("0.0.0.0"), cclass->getNextServer());
+    EXPECT_EQ(0, cclass->getSname().size());
+    EXPECT_EQ(0, cclass->getFilename().size());
+}
+
+// Test verifies that it is possible to define next-server field and it
+// is actually set in the class properly.
+TEST_F(ClientClassDefParserTest, nextServer) {
+
+    std::string cfg_text =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"next-server\": \"192.0.2.254\",\n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+
+    ClientClassDefPtr cclass;
+    ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, Option::V4));
+
+    // We should find our class.
+    ASSERT_TRUE(cclass);
+
+    // And it should have next-server set, but everything else not set.
+    EXPECT_EQ(IOAddress("192.0.2.254"), cclass->getNextServer());
+    EXPECT_EQ(0, cclass->getSname().size());
+    EXPECT_EQ(0, cclass->getFilename().size());
+}
+
+// Test verifies that the parser rejects bogus next-server value.
+TEST_F(ClientClassDefParserTest, nextServerBogus) {
+
+    std::string bogus_v6 =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"next-server\": \"2001:db8::1\",\n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+    std::string bogus_junk =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"next-server\": \"not-an-address\",\n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+
+    EXPECT_THROW(parseClientClassDef(bogus_v6, Option::V4), DhcpConfigError);
+    EXPECT_THROW(parseClientClassDef(bogus_junk, Option::V4), DhcpConfigError);
+}
+
+// Test verifies that it is possible to define server-hostname field and it
+// is actually set in the class properly.
+TEST_F(ClientClassDefParserTest, serverName) {
+
+    std::string cfg_text =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"server-hostname\": \"hal9000\",\n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+
+    ClientClassDefPtr cclass;
+    ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, Option::V4));
+
+    // We should find our class.
+    ASSERT_TRUE(cclass);
+
+    // And it should not have any fixed fields set
+    std::string exp_sname("hal9000");
+
+    EXPECT_EQ(exp_sname, cclass->getSname());
+}
+
+// Test verifies that the parser rejects bogus server-hostname value.
+TEST_F(ClientClassDefParserTest, serverNameInvalid) {
+
+    std::string cfg_too_long =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"server-hostname\": \"1234567890123456789012345678901234567890"
+                                   "1234567890123456789012345\", \n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+
+    EXPECT_THROW(parseClientClassDef(cfg_too_long, Option::V4), DhcpConfigError);
+}
+
+
+// Test verifies that it is possible to define boot-file-name field and it
+// is actually set in the class properly.
+TEST_F(ClientClassDefParserTest, filename) {
+
+    std::string cfg_text =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"boot-file-name\": \"ipxe.efi\", \n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+
+    ClientClassDefPtr cclass;
+    ASSERT_NO_THROW(cclass = parseClientClassDef(cfg_text, Option::V4));
+
+    // We should find our class.
+    ASSERT_TRUE(cclass);
+
+    // And it should not have any fixed fields set
+    std::string exp_filename("ipxe.efi");
+    EXPECT_EQ(exp_filename, cclass->getFilename());
+}
+
+// Test verifies that the parser rejects bogus boot-file-name value.
+TEST_F(ClientClassDefParserTest, filenameBogus) {
+
+    // boot-file-name is allowed up to 128 bytes, this one is 129.
+    std::string cfg_too_long =
+        "{ \n"
+        "    \"name\": \"MICROSOFT\", \n"
+        "    \"boot-file-name\": \"1234567890123456789012345678901234567890"
+                                  "1234567890123456789012345678901234567890"
+                                  "1234567890123456789012345678901234567890"
+                                  "1234567890123456789012345678901234567890"
+                                  "1234567890123456789012345678901234567890"
+                                  "1234567890123456789012345678901234567890"
+                                  "123456789\", \n"
+        "    \"option-data\": [ \n"
+        "        { \n"
+        "           \"name\": \"domain-name-servers\", \n"
+        "           \"data\": \"192.0.2.1, 192.0.2.2\" \n"
+        "        } \n"
+        "      ] \n"
+        "} \n";
+
+    EXPECT_THROW(parseClientClassDef(cfg_too_long, Option::V4), DhcpConfigError);
+}
+
+
 } // end of anonymous namespace

src/lib/dhcpsrv/tests/client_class_def_unittest.cc

@@ -8,6 +8,7 @@
 #include <dhcpsrv/client_class_def.h>
 #include <exceptions/exceptions.h>
 #include <boost/scoped_ptr.hpp>
+#include <asiolink/io_address.h>
 
 #include <gtest/gtest.h>
 
@@ -17,6 +18,7 @@
 using namespace std;
 using namespace isc::dhcp;
 using namespace isc::util;
+using namespace isc::asiolink;
 using namespace isc;
 
 namespace {
@@ -298,4 +300,56 @@ TEST(ClientClassDictionary, copyAndEquality) {
     EXPECT_TRUE(*dictionary != *dictionary2);
 }
 
+// Tests the default constructor regarding fixed fields
+TEST(ClientClassDef, fixedFieldsDefaults) {
+    boost::scoped_ptr<ClientClassDef> cclass;
+
+    std::string name = "class1";
+    ExpressionPtr expr;
+    CfgOptionPtr cfg_option;
+
+    // Classes cannot have blank names
+    ASSERT_THROW(cclass.reset(new ClientClassDef("", expr, cfg_option)),
+                 BadValue);
+
+    // Verify we can create a class with a name, expression, and no cfg_option
+    ASSERT_NO_THROW(cclass.reset(new ClientClassDef(name, expr)));
+
+    // Let's checks that it doesn't return any nonsense
+    string empty;
+    ASSERT_EQ(IOAddress("0.0.0.0"), cclass->getNextServer());
+    EXPECT_EQ(empty, cclass->getSname());
+    EXPECT_EQ(empty, cclass->getFilename());
+}
+
+// Tests basic operations of fixed fields
+TEST(ClientClassDef, fixedFieldsBasics) {
+    boost::scoped_ptr<ClientClassDef> cclass;
+
+    std::string name = "class1";
+    ExpressionPtr expr;
+    CfgOptionPtr cfg_option;
+
+    // Classes cannot have blank names
+    ASSERT_THROW(cclass.reset(new ClientClassDef("", expr, cfg_option)),
+                 BadValue);
+
+    // Verify we can create a class with a name, expression, and no cfg_option
+    ASSERT_NO_THROW(cclass.reset(new ClientClassDef(name, expr)));
+
+
+    string sname = "This is a very long string that can be a server name";
+    string filename = "this-is-a-slightly-longish-name-of-a-file.txt";
+
+    cclass->setNextServer(IOAddress("1.2.3.4"));
+    cclass->setSname(sname);
+    cclass->setFilename(filename);
+
+    // Let's checks that it doesn't return any nonsense
+    ASSERT_EQ(IOAddress("1.2.3.4"), cclass->getNextServer());
+    EXPECT_EQ(sname, cclass->getSname());
+    EXPECT_EQ(filename, cclass->getFilename());
+}
+
+
 } // end of anonymous namespace