From: Francis Dupont Date: Sat, 21 Nov 2015 12:22:57 +0000 (+0100) Subject: [4097a] Moved specific processing to appendRequestedOptions and renamed classSpecific... X-Git-Tag: trac4204fd_base~2^2~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=102820fc3526cc4f6422ba0c81918c1a0527fdb6;p=thirdparty%2Fkea.git [4097a] Moved specific processing to appendRequestedOptions and renamed classSpecificProcessing --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index d017ca0f90..41a046d81c 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/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::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& 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::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); } diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 8537ad4ae6..544205b69b 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/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 diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 0a772d262d..18108bafe9 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/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); } diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h index 908621a660..91e2ab59f1 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/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;