From 30aedc9edf919a529642fa090da665526cbeceba Mon Sep 17 00:00:00 2001 From: Nicholas Brown Date: Tue, 22 Jan 2019 17:23:21 +0000 Subject: [PATCH 1/4] Simplify ConfigManager parsing loop --- src/core/MainSignalHandler.h | 2 +- src/modules/ConfigManager.cpp | 14 ++++++-------- src/modules/{ConfigManager.h => ConfigManager.hpp} | 0 src/tests/vermonttest/ReconfTest.cpp | 2 +- src/vermont.cc | 2 +- 5 files changed, 9 insertions(+), 11 deletions(-) rename src/modules/{ConfigManager.h => ConfigManager.hpp} (100%) diff --git a/src/core/MainSignalHandler.h b/src/core/MainSignalHandler.h index 639a29b..1dfe118 100644 --- a/src/core/MainSignalHandler.h +++ b/src/core/MainSignalHandler.h @@ -5,7 +5,7 @@ #include "common/SignalHandler.h" #include "common/VermontControl.h" -#include "modules/ConfigManager.h" +#include "modules/ConfigManager.hpp" class MainSignalHandler : public SignalInterface { diff --git a/src/modules/ConfigManager.cpp b/src/modules/ConfigManager.cpp index d68f5e5..88b5c66 100644 --- a/src/modules/ConfigManager.cpp +++ b/src/modules/ConfigManager.cpp @@ -18,7 +18,7 @@ * */ -#include "modules/ConfigManager.h" +#include "ConfigManager.hpp" #include "core/Connector.h" #include "core/CfgNode.h" #include "common/defs.h" @@ -156,13 +156,11 @@ void ConfigManager::parseConfig(std::string fileName) * attached to the node) to the graph */ XMLNode::XMLSet rootElements = root->getElementChildren(); - for (XMLNode::XMLSet::const_iterator it = rootElements.begin(); - it != rootElements.end(); - it++) { + for (const auto& element : rootElements) { bool found = false; - for (unsigned int i = 0; i < ARRAY_SIZE(configModules); i++) { - if ((*it)->getName() == configModules[i]->getName()) { - Cfg* cfg = configModules[i]->create(*it); + for (auto& module : configModules) { + if (element->getName() == module->getName()) { + Cfg* cfg = module->create(element); // handle special modules SensorManagerCfg* smcfg = dynamic_cast(cfg); @@ -179,7 +177,7 @@ void ConfigManager::parseConfig(std::string fileName) } if (!found) { - msg(LOG_ERR, "Unknown cfg entry %s found", (*it)->getName().c_str()); + msg(LOG_ERR, "Unknown cfg entry %s found", element->getName().c_str()); } } diff --git a/src/modules/ConfigManager.h b/src/modules/ConfigManager.hpp similarity index 100% rename from src/modules/ConfigManager.h rename to src/modules/ConfigManager.hpp diff --git a/src/tests/vermonttest/ReconfTest.cpp b/src/tests/vermonttest/ReconfTest.cpp index bcf042e..2e2d282 100644 --- a/src/tests/vermonttest/ReconfTest.cpp +++ b/src/tests/vermonttest/ReconfTest.cpp @@ -4,7 +4,7 @@ #include "modules/packet/filter//SystematicSampler.h" #include "common/msg.h" #include "CounterDestination.h" -#include "modules/ConfigManager.h" +#include "modules/ConfigManager.hpp" #include "core/ConnectionSplitter.h" #include "PrinterModule.h" diff --git a/src/vermont.cc b/src/vermont.cc index a129f26..e716600 100644 --- a/src/vermont.cc +++ b/src/vermont.cc @@ -41,7 +41,7 @@ #include "common/defs.h" #include "core/MainSignalHandler.h" -#include "modules/ConfigManager.h" +#include "modules/ConfigManager.hpp" using namespace std; From 0f98329a025e145f9365972ea6223bce982b54c9 Mon Sep 17 00:00:00 2001 From: Nicholas Brown Date: Thu, 24 Jan 2019 13:25:00 +0000 Subject: [PATCH 2/4] Don't log in the fast data path Should not be generating logs per ipfix frame. Use DPRINT_ functions in the critical datapath so that they disabled by default and don't impact performance. LOG_* should be used for interesting state that's not per packets/frame, and DPRINTF_ used for per packet/frame logging, and then explicitly enabled and compile time. --- src/modules/ipfix/IpfixParser.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/modules/ipfix/IpfixParser.cpp b/src/modules/ipfix/IpfixParser.cpp index 7516712..835bf95 100644 --- a/src/modules/ipfix/IpfixParser.cpp +++ b/src/modules/ipfix/IpfixParser.cpp @@ -642,7 +642,7 @@ int IpfixParser::processIpfixPacket(boost::shared_array message, uint16 } } - msg(LOG_DEBUG, "IPFIX message from %s contained %u Data Records and %u Template Records. Sequence number was %lu.", + DPRINTF_DEBUG("IPFIX message from %s contained %u Data Records and %u Template Records. Sequence number was %lu.", (sourceId->toString()).c_str(), numberOfDataRecords, numberOfTemplateRecords, (unsigned long) sequenceNumber); // Update statistics @@ -834,4 +834,3 @@ void IpfixParser::withdrawBufferedTemplates() bt = bt->next; } } - From fd82f2d2800181e669854a4898685469036304c8 Mon Sep 17 00:00:00 2001 From: Nicholas Brown Date: Wed, 20 Feb 2019 18:11:22 +0000 Subject: [PATCH 3/4] Don't log normal behaviour in the aggregator data path --- src/modules/ipfix/aggregator/Rule.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/ipfix/aggregator/Rule.cpp b/src/modules/ipfix/aggregator/Rule.cpp index 416f97a..55f1602 100644 --- a/src/modules/ipfix/aggregator/Rule.cpp +++ b/src/modules/ipfix/aggregator/Rule.cpp @@ -435,7 +435,7 @@ int Rule::dataRecordMatches(IpfixDataRecord* record) { */ /* no corresponding data field or fixed data field found, this flow cannot match */ - msg(LOG_DEBUG, "No corresponding DataDataRecord field for RuleField of type %s", ruleField->type.toString().c_str()); + DPRINTF_INFO("No corresponding DataDataRecord field for RuleField of type %s", ruleField->type.toString().c_str()); return 0; } /* if a non-discarding rule field specifies no pattern, check at least if the data field exists */ @@ -454,7 +454,7 @@ int Rule::dataRecordMatches(IpfixDataRecord* record) { if (ruleField->type==InformationElement::IeInfo(IPFIX_ETYPEID_anonymisationType, IPFIX_PEN_vermont)) continue; - msg(LOG_NOTICE, "No corresponding DataRecord field for RuleField of type %s", ruleField->type.toString().c_str()); + DPRINTF_INFO("No corresponding DataRecord field for RuleField of type %s", ruleField->type.toString().c_str()); return 0; } } From bba706fe8c2f13f7c422ae7299ffe2f6111951b6 Mon Sep 17 00:00:00 2001 From: Nicholas Brown Date: Wed, 20 Feb 2019 18:13:21 +0000 Subject: [PATCH 4/4] Removed unused code from Rule.cpp --- src/modules/ipfix/aggregator/Rule.cpp | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/modules/ipfix/aggregator/Rule.cpp b/src/modules/ipfix/aggregator/Rule.cpp index 55f1602..7c27bb8 100644 --- a/src/modules/ipfix/aggregator/Rule.cpp +++ b/src/modules/ipfix/aggregator/Rule.cpp @@ -412,28 +412,6 @@ int Rule::dataRecordMatches(IpfixDataRecord* record) { continue; } - /* Probably, this does not lead to the desired result: - if (biflowAggregation) { - // check if the rule field as a corresponding type in opposite direction - InformationElement::IeId oppDirIeId = InformationElement::oppositeDirectionIeId(ruleField->type); - if(oppDirIeId) { - recordField = dataTemplateInfo->getFieldInfo(oppDirIeId, ruleField->type.enterprise); - if (recordField) { - // corresponding data field found, check if it matches. If it doesn't the whole rule cannot be matched - if (!matchesPattern(&recordField->type, (recordData + recordField->offset), &ruleField->type, ruleField->pattern)) return 0; - if ((ruleField->type.enterprise == 0) && (ruleField->type.length == 5)) { - if (oppDirIeId == IPFIX_TYPEID_sourceIPv4Address) { - if(!checkMask(dataTemplateInfo->getFieldInfo(IPFIX_TYPEID_sourceIPv4PrefixLength, 0), recordData, ruleField)) return 0; - } else if (oppDirIeId == IPFIX_TYPEID_destinationIPv4Address) { - if(!checkMask(dataTemplateInfo->getFieldInfo(IPFIX_TYPEID_destinationIPv4PrefixLength, 0), recordData, ruleField)) return 0; - } - } - continue; - } - } - } - */ - /* no corresponding data field or fixed data field found, this flow cannot match */ DPRINTF_INFO("No corresponding DataDataRecord field for RuleField of type %s", ruleField->type.toString().c_str()); return 0;