[4097a] Moved specific processing to appendRequestedOptions and renamed classSpecificProcessing

src/bin/dhcp4/dhcp4_srv.cc

@@ -861,6 +861,33 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) {
             }
         }
     }
+
+    // Process each class in the packet
+    const ClientClasses& classes = query->getClasses();
+    for (ClientClasses::const_iterator cclass = classes.begin();
+         cclass != classes.end(); ++cclass) {
+        // Find the client class definition for this class
+        const ClientClassDefPtr& ccdef = CfgMgr::instance().getCurrentCfg()->
+            getClientClassDictionary()->findClass(*cclass);
+        if (!ccdef) {
+            // Not found: the class is not configured
+            LOG_DEBUG(options4_logger, DBG_DHCP4_BASIC, DHCP4_CLASS_UNCONFIGURED)
+                .arg(query->getLabel())
+                .arg(*cclass);
+            continue;
+        }
+        // For each requested option code get the instance of the option
+        // in the class to be returned to the client.
+        for (std::vector<uint8_t>::const_iterator opt = requested_opts.begin();
+             opt != requested_opts.end(); ++opt) {
+            if (!resp->getOption(*opt)) {
+                OptionDescriptor desc = ccdef->getCfgOption()->get("dhcp4", *opt);
+                if (desc.option_) {
+                    resp->addOption(desc.option_);
+                }
+            }
+        }
+    }
 }
 
 void
@@ -1611,7 +1638,7 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 
     /// @todo: decide whether we want to add a new hook point for
     /// doing class specific processing.
-    if (!classSpecificProcessing(ex)) {
+    if (!vendorClassSpecificProcessing(ex)) {
         /// @todo add more verbosity here
         LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL, DHCP4_DISCOVER_CLASS_PROCESSING_FAILED)
             .arg(discover->getLabel());
@@ -1661,7 +1688,7 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 
     /// @todo: decide whether we want to add a new hook point for
     /// doing class specific processing.
-    if (!classSpecificProcessing(ex)) {
+    if (!vendorClassSpecificProcessing(ex)) {
         /// @todo add more verbosity here
         LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL, DHCP4_REQUEST_CLASS_PROCESSING_FAILED)
             .arg(ex.getQuery()->getLabel());
@@ -1949,7 +1976,7 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
 
     /// @todo: decide whether we want to add a new hook point for
     /// doing class specific processing.
-    if (!classSpecificProcessing(ex)) {
+    if (!vendorClassSpecificProcessing(ex)) {
         LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL,
                   DHCP4_INFORM_CLASS_PROCESSING_FAILED)
             .arg(inform->getLabel());
@@ -2334,7 +2361,7 @@ void Dhcpv4Srv::classifyPacket(const Pkt4Ptr& pkt) {
 }
 
 bool
-Dhcpv4Srv::classSpecificProcessing(const Dhcpv4Exchange& ex) {
+Dhcpv4Srv::vendorClassSpecificProcessing(const Dhcpv4Exchange& ex) {
 
     Subnet4Ptr subnet = ex.getContext()->subnet_;
     Pkt4Ptr query = ex.getQuery();
@@ -2345,7 +2372,6 @@ Dhcpv4Srv::classSpecificProcessing(const Dhcpv4Exchange& ex) {
         return (true);
     }
 
-    // DOCSIS3 class modem specific processing
     if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_MODEM)) {
 
         // Set next-server. This is TFTP server address. Cable modems will
@@ -2367,52 +2393,12 @@ Dhcpv4Srv::classSpecificProcessing(const Dhcpv4Exchange& ex) {
         }
     }
 
-    // DOCSIS3 class erouter specific processing
     if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER)) {
 
         // Do not set TFTP server address for eRouter devices.
         rsp->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS());
     }
 
-    // try to get the 'Parameter Request List' option which holds the
-    // codes of requested options.
-    OptionUint8ArrayPtr option_prl = boost::dynamic_pointer_cast<
-        OptionUint8Array>(query->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
-    // If there is no PRL option in the message from the client then
-    // there is nothing to do.
-    if (!option_prl) {
-        return (true);
-    }
-    // Get the codes of requested options.
-    const std::vector<uint8_t>& requested_opts = option_prl->getValues();
-
-    // Process each class in the packet
-    const ClientClasses& classes = query->getClasses();
-    for (ClientClasses::const_iterator cclass = classes.begin();
-         cclass != classes.end(); ++cclass) {
-        // Find the client class definition for this class
-        const ClientClassDefPtr& ccdef = CfgMgr::instance().getCurrentCfg()->
-            getClientClassDictionary()->findClass(*cclass);
-        if (!ccdef) {
-            // Not found: the class is not configured
-	    LOG_DEBUG(options4_logger, DBG_DHCP4_BASIC, DHCP4_CLASS_UNCONFIGURED)
-		.arg(query->getLabel())
-		.arg(*cclass);
-            continue;
-        }
-	// For each requested option code get the instance of the option
-	// in the class to be returned to the client.
-	for (std::vector<uint8_t>::const_iterator opt = requested_opts.begin();
-	     opt != requested_opts.end(); ++opt) {
-	    if (!rsp->getOption(*opt)) {
-		OptionDescriptor desc = ccdef->getCfgOption()->get("dhcp4", *opt);
-		if (desc.option_) {
-		    rsp->addOption(desc.option_);
-		}
-	    }
-	}
-    }
-
     return (true);
 }
 

src/bin/dhcp4/dhcp4_srv.h

@@ -716,7 +716,7 @@ protected:
     /// @param pkt packet to be classified
     void classifyPacket(const Pkt4Ptr& pkt);
 
-    /// @brief Performs packet processing specific to a class
+    /// @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.
@@ -726,7 +726,7 @@ protected:
     /// @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 classSpecificProcessing(const Dhcpv4Exchange& ex);
+    bool vendorClassSpecificProcessing(const Dhcpv4Exchange& ex);
 
     /// @brief Allocation Engine.
     /// Pointer to the allocation engine that we are currently using

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1676,6 +1676,9 @@ TEST_F(Dhcpv4SrvTest, docsisClientClassification) {
 
 // Checks if client packets are classified properly using match expressions.
 TEST_F(Dhcpv4SrvTest, matchClassification) {
+    IfaceMgrTestConfig test_config(true);
+    IfaceMgr::instance().openSockets4();
+
     NakedDhcpv4Srv srv(0);
 
     // The router class matches incoming packets with foo in a host-name
@@ -1707,12 +1710,19 @@ TEST_F(Dhcpv4SrvTest, matchClassification) {
     CfgMgr::instance().commit();
 
     // Create packets with enough to select the subnet
+    OptionPtr clientid = generateClientId();
     Pkt4Ptr query1(new Pkt4(DHCPDISCOVER, 1234));
     query1->setRemoteAddr(IOAddress("192.0.2.1"));
+    query1->addOption(clientid);
+    query1->setIface("eth1");
     Pkt4Ptr query2(new Pkt4(DHCPDISCOVER, 1234));
     query2->setRemoteAddr(IOAddress("192.0.2.1"));
+    query2->addOption(clientid);
+    query2->setIface("eth1");
     Pkt4Ptr query3(new Pkt4(DHCPDISCOVER, 1234));
     query3->setRemoteAddr(IOAddress("192.0.2.1"));
+    query3->addOption(clientid);
+    query3->setIface("eth1");
 
     // Create and add a PRL option to the first 2 queries
     OptionUint8ArrayPtr prl(new OptionUint8Array(Option::V4,
@@ -1738,25 +1748,20 @@ TEST_F(Dhcpv4SrvTest, matchClassification) {
     EXPECT_FALSE(query2->inClass("router"));
     EXPECT_TRUE(query3->inClass("router"));
 
-    Dhcpv4Exchange ex1 = createExchange(query1);
-    Pkt4Ptr response1 = ex1.getResponse();
-    Dhcpv4Exchange ex2 = createExchange(query2);
-    Pkt4Ptr response2 = ex2.getResponse();
-    Dhcpv4Exchange ex3 = createExchange(query3);
-    Pkt4Ptr response3 = ex3.getResponse();
+    // Process queries
+    Pkt4Ptr response1 = srv.processDiscover(query1);
+    Pkt4Ptr response2 = srv.processDiscover(query2);
+    Pkt4Ptr response3 = srv.processDiscover(query3);
 
     // Classification processing should add an ip-forwarding option
-    srv.classSpecificProcessing(ex1);
     OptionPtr opt1 = response1->getOption(DHO_IP_FORWARDING);
     EXPECT_TRUE(opt1);
 
-    // But only for the first exchange
-    srv.classSpecificProcessing(ex2);
+    // But only for the first exchange: second was not classified
     OptionPtr opt2 = response2->getOption(DHO_IP_FORWARDING);
     EXPECT_FALSE(opt2);
 
-    // But only for the first exchange
-    srv.classSpecificProcessing(ex3);
+    // But only for the first exchange: third has no PRL
     OptionPtr opt3 = response3->getOption(DHO_IP_FORWARDING);
     EXPECT_FALSE(opt3);
 }

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -204,7 +204,6 @@ public:
     using Dhcpv4Srv::srvidToString;
     using Dhcpv4Srv::unpackOptions;
     using Dhcpv4Srv::classifyPacket;
-    using Dhcpv4Srv::classSpecificProcessing;
     using Dhcpv4Srv::accept;
     using Dhcpv4Srv::acceptMessageType;
     using Dhcpv4Srv::selectSubnet;