From 34f85e2545ba3e169722c9e89e1d0ab86a685561 Mon Sep 17 00:00:00 2001
From: Nicholas Brown <nick.brown@att.com>
Date: Wed, 29 Aug 2018 18:40:36 +0100
Subject: [PATCH] Error message when key/nonkey configuration doesn't match
 actual behaviour

Using a 'traditional' flow definition like that below will result in:

vermont[4729]: Field bgpDestinationAsNumber configured as nonFlowKey will not be Aggregated
vermont[4729]: Field bgpSourceAsNumber configured as nonFlowKey will not be Aggregated
vermont[4729]: Field destinationIPv4PrefixLength configured as nonFlowKey will not be Aggregated
vermont[4729]: Field egressInterface configured as nonFlowKey will not be Aggregated
vermont[4729]: Field ipNextHopIPv4Address configured as nonFlowKey will not be Aggregated
vermont[4729]: Field sourceIPv4PrefixLength configured as nonFlowKey will not be Aggregated

<rule>
        <flowKey>
               <ieName>destinationIPv4Address</ieName>
               <autoAddV4PrefixLength>false</autoAddV4PrefixLength>
        </flowKey>
        <flowKey>
               <ieName>destinationTransportPort</ieName>
        </flowKey>
        <flowKey>
               <ieName>ingressInterface</ieName>
        </flowKey>
        <flowKey>
               <ieName>ipClassOfService</ieName>
        </flowKey>
        <flowKey>
               <ieName>protocolIdentifier</ieName>
        </flowKey>
        <flowKey>
               <ieName>sourceIPv4Address</ieName>
               <autoAddV4PrefixLength>false</autoAddV4PrefixLength>
        </flowKey>
        <flowKey>
               <ieName>sourceTransportPort</ieName>
        </flowKey>
        <nonFlowKey>
               <ieName>bgpDestinationAsNumber</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>bgpSourceAsNumber</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>destinationIPv4PrefixLength</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>egressInterface</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>flowEndMilliseconds</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>flowStartMilliseconds</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>ipNextHopIPv4Address</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>octetDeltaCount</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>packetDeltaCount</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>sourceIPv4PrefixLength</ieName>
        </nonFlowKey>
        <nonFlowKey>
               <ieName>tcpControlBits</ieName>
        </nonFlowKey>
</rule>
---
 src/modules/ipfix/aggregator/AggregatorBaseCfg.cpp | 9 ++++++++-
 src/modules/ipfix/aggregator/BaseHashtable.h       | 3 ++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/modules/ipfix/aggregator/AggregatorBaseCfg.cpp b/src/modules/ipfix/aggregator/AggregatorBaseCfg.cpp
index bedec837..5b535fd5 100644
--- a/src/modules/ipfix/aggregator/AggregatorBaseCfg.cpp
+++ b/src/modules/ipfix/aggregator/AggregatorBaseCfg.cpp
@@ -23,6 +23,7 @@
 #include "Rules.hpp"
 #include "core/XMLElement.h"
 #include "core/InfoElementCfg.h"
+#include "BaseHashtable.h"
 
 AggregatorBaseCfg::AggregatorBaseCfg(XMLElement* elem)
 	: CfgBase(elem), pollInterval(0)
@@ -46,7 +47,7 @@ AggregatorBaseCfg::AggregatorBaseCfg(XMLElement* elem)
 				} else {
 					msg(LOG_CRIT, "Too many rules: %ul\n", MAX_RULES);
 				}
-				
+
 			}
 		} else if (e->matches("expiration")) {
 			// get the time values or set them to '0' if they are not specified
@@ -140,6 +141,9 @@ Rule::Field* AggregatorBaseCfg::readNonFlowKeyRule(XMLElement* e)
 	ruleField->semantic = ie.getSemantic();
 	ruleField->fieldIe = ie.getFieldIe();
 
+	if (!BaseHashtable::isToBeAggregated(ruleField->type))
+		msg(LOG_ERR, "Field %s configured as nonFlowKey will not be Aggregated", ie.getIeName().c_str());
+
 	if (ie.getAutoAddV4PrefixLength() &&
 			(ruleField->type == InformationElement::IeInfo(IPFIX_TYPEID_sourceIPv4Address, 0) ||
 			ruleField->type == InformationElement::IeInfo(IPFIX_TYPEID_destinationIPv4Address, 0))) {
@@ -178,6 +182,9 @@ Rule::Field* AggregatorBaseCfg::readFlowKeyRule(XMLElement* e) {
 		ruleField->type.enterprise = ie.getEnterpriseNumber();
 		ruleField->type.length = ie.getIeLength();
 
+		if (BaseHashtable::isToBeAggregated(ruleField->type))
+			msg(LOG_ERR, "Field %s configured as FlowKey will be Aggregated", ie.getIeName().c_str());
+
 		if (ie.getAutoAddV4PrefixLength() &&
 				(ruleField->type.id == IPFIX_TYPEID_sourceIPv4Address || ruleField->type.id == IPFIX_TYPEID_destinationIPv4Address)) {
 			ruleField->type.length++; // for additional mask field
diff --git a/src/modules/ipfix/aggregator/BaseHashtable.h b/src/modules/ipfix/aggregator/BaseHashtable.h
index 55bc8162..ff1530ff 100644
--- a/src/modules/ipfix/aggregator/BaseHashtable.h
+++ b/src/modules/ipfix/aggregator/BaseHashtable.h
@@ -92,6 +92,8 @@ class BaseHashtable : public Sensor
 	 */
 	void postReconfiguration();
 
+	static int isToBeAggregated(InformationElement::IeInfo& type);
+
 protected:
 	/**
 	 * contains needed data elements when FPA or DPA is performed for PacketHashtable
@@ -153,7 +155,6 @@ class BaseHashtable : public Sensor
 
 	alock_t aggInProgress; /** indicates if currently an element is aggregated in the hashtable, used for atomic lock for preReconfiguration */
 
-	int isToBeAggregated(InformationElement::IeInfo& type);
 	HashtableBucket* createBucket(boost::shared_array<IpfixRecord::Data> data, uint32_t obsdomainid,
 		HashtableBucket* next, HashtableBucket* prev, uint32_t hash, time_t now);
 	void exportBucket(HashtableBucket* bucket);