Skip to content

Commit 25dbc33

Browse files
tomflynntomflynn
committed
Improving NetFlow UTs
Signed-off-by: Tom Flynn <tom.flynn@gmail.com> Change-Id: I7602d029cf301012807c7459bb9206acfbd82d55
1 parent f5ba801 commit 25dbc33

File tree

10 files changed

+55
-67
lines changed

10 files changed

+55
-67
lines changed

‎agent-ovs/lib/NetFlowManager.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,13 @@ namespace opflexagent {
161161
return !exporter_map.empty();
162162
}
163163

164-
void NetFlowManager::NetFlowUniverseListener::processExporterConfig(const shared_ptr<modelgbp::netflow::ExporterConfig>& exporterconfig) {
165-
shared_ptr<ExporterConfigState> exportState;
166-
lock_guard<recursive_mutex> guard(opflexagent::NetFlowManager::exporter_mutex);
167-
auto itr = netflowmanager.exporter_map.find(exporterconfig->getURI());
168-
if (itr != netflowmanager.exporter_map.end()) {
169-
netflowmanager.exporter_map.erase(itr);
164+
void NetFlowManager::updateExporterConfigState(const shared_ptr<modelgbp::netflow::ExporterConfig>& exporterconfig) {
165+
lock_guard<recursive_mutex> guard(NetFlowManager::exporter_mutex);
166+
auto itr = exporter_map.find(exporterconfig->getURI());
167+
if (itr != exporter_map.end()) {
168+
exporter_map.erase(itr);
170169
}
171-
exportState = make_shared<ExporterConfigState>(exporterconfig->getURI(), exporterconfig->getName().get());
170+
shared_ptr<ExporterConfigState> exportState = make_shared<ExporterConfigState>(exporterconfig->getURI(), exporterconfig->getName().get());
172171
boost::optional<const string&> dstAddr = exporterconfig->getDstAddr();
173172
if (dstAddr) {
174173
exportState->setDstAddress(dstAddr.get());
@@ -197,7 +196,11 @@ namespace opflexagent {
197196
if (activeTimeout) {
198197
exportState->setActiveFlowTimeOut(activeTimeout.get());
199198
}
200-
netflowmanager.exporter_map.insert(make_pair(exporterconfig->getURI(), exportState));
199+
exporter_map.insert(make_pair(exporterconfig->getURI(), exportState));
200+
}
201+
202+
void NetFlowManager::NetFlowUniverseListener::processExporterConfig(const shared_ptr<modelgbp::netflow::ExporterConfig>& exporterconfig) {
203+
netflowmanager.updateExporterConfigState(exporterconfig);
201204
}
202205
}
203206

‎agent-ovs/lib/include/opflexagent/ExporterConfigState.h

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,20 @@ class ExporterConfigState
104104
*/
105105
void setVersion(const uint8_t& version_ ) { version = version_; };
106106
/**
107-
* get the version of ExporterConfig
108-
* @return version of the ExporterConfig
107+
* get the dscp of ExporterConfig
108+
* @return dscp of the ExporterConfig
109109
*/
110110
const uint8_t& getDscp() const { return dscp; };
111111
/**
112-
* set the dscp of ExporterConfig
113-
* @param[in] dscp_ dscp of the ExporterConfig
112+
* set the dscp of ExporterConfig
113+
* @param[in] dscp_ dscp of the ExporterConfig
114114
*/
115115
void setDscp(const uint8_t& dscp_ ) { dscp = dscp_; };
116116
/**
117117
* gets the URI, which points to a ExporterConfig object
118118
* @return a URI
119119
*/
120120
const URI& getUri() const { return uri; };
121-
/**
122-
* set the URI to the one passed in
123-
* @param uri_ URI to a ExporterConfig object
124-
*/
125-
void setUri(URI &uri_) { uri = uri_; };
126121
};
127122
} // namespace opflexagent
128123
#endif //OPFLEX_EXPORTERCONFIGSTATE

‎agent-ovs/lib/include/opflexagent/NetFlowManager.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ class NetFlowManager {
7676
*/
7777
boost::optional<shared_ptr<ExporterConfigState>> getExporterConfigState(const URI& uri) const;
7878

79+
/**
80+
* Update the exporter config state
81+
* @param exporterconfig Pointer to updated exporter config
82+
*/
83+
void updateExporterConfigState(const shared_ptr<modelgbp::netflow::ExporterConfig>& exporterconfig);
84+
7985
/**
8086
* Are there any exporters
8187
*

‎agent-ovs/lib/test/NetflowManager_test.cpp

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,17 @@
1111

1212
#include <boost/test/unit_test.hpp>
1313
#include <modelgbp/dmtree/Root.hpp>
14-
#include <opflex/modb/Mutator.h>
1514

1615
#include <opflexagent/logging.h>
1716
#include <opflexagent/test/BaseFixture.h>
1817
#include "Policies.h"
19-
#include <modelgbp/platform/Config.hpp>
20-
#include <modelgbp/netflow/ExporterConfig.hpp>
21-
#include <opflexagent/ExporterConfigState.h>
2218
#include <modelgbp/netflow/CollectorVersionEnumT.hpp>
2319

2420
namespace opflexagent {
2521

2622
using std::shared_ptr;
2723
using namespace modelgbp;
28-
using namespace modelgbp::gbp;
24+
2925
class NetflowFixture : public BaseFixture {
3026

3127
public:
@@ -72,20 +68,20 @@ static bool checkNetFlowDstAddress(boost::optional<shared_ptr<ExporterConfigStat
7268
if (!pExpst)
7369
return false;
7470
const string addr = pExportCfg->getDstAddr("");
75-
LOG(DEBUG) << "checkNetFlowDstAddress" << addr;
71+
LOG(DEBUG) << "checkNetFlowDstAddress " << addr;
7672
return pExpst.get()->getDstAddress() == addr;
7773
}
7874
static bool checkNetFlowDstPort(boost::optional<shared_ptr<ExporterConfigState>> pExpst,
7975
shared_ptr<netflow::ExporterConfig>& pExportCfg) {
8076
if (!pExpst)
8177
return false;
82-
LOG(DEBUG) << "checkNetFlowDstPort" << pExpst.get()->getDestinationPort();
78+
LOG(DEBUG) << "checkNetFlowDstPort " << pExpst.get()->getDestinationPort();
8379
return pExpst.get()->getDestinationPort() == pExportCfg->getDstPort();
8480
}
8581
static bool checkNetFlowVers(boost::optional<shared_ptr<ExporterConfigState>> pExpst,
8682
shared_ptr<netflow::ExporterConfig>& pExportCfg) {
8783
if (!pExpst) return false;
88-
LOG(DEBUG) << "checkNetFlowVers" << pExpst.get()->getVersion();
84+
LOG(DEBUG) << "checkNetFlowVers " << std::to_string(pExpst.get()->getVersion());
8985
return pExpst.get()->getVersion() == pExportCfg->getVersion().get();
9086
}
9187
BOOST_FIXTURE_TEST_CASE( verify_artifacts, NetflowFixture ) {
@@ -96,18 +92,11 @@ BOOST_FIXTURE_TEST_CASE( verify_artifacts, NetflowFixture ) {
9692
WAIT_FOR(checkNetFlowDstPort(agent.getNetFlowManager().getExporterConfigState(exportCfg->getURI()), exportCfg), 500);
9793
WAIT_FOR(checkNetFlowVers(agent.getNetFlowManager().getExporterConfigState(exportCfg->getURI()), exportCfg), 500);
9894

99-
{
100-
// remove exporter
101-
Mutator mutator(framework, "policyreg");
102-
exportCfg->remove();
103-
mutator.commit();
104-
}
105-
{
106-
// remove PlatformConfig
107-
Mutator mutator(framework, "policyreg");
108-
exportCfg->remove();
109-
mutator.commit();
110-
}
95+
// remove exporter
96+
Mutator mutator(framework, "policyreg");
97+
exportCfg->remove();
98+
mutator.commit();
99+
111100
WAIT_FOR(!agent.getNetFlowManager().anyExporters(), 500)
112101
}
113102

‎agent-ovs/lib/test/SimStats_test.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ namespace opflexagent {
1919
class SimStatsFixture : public BaseFixture {
2020
public:
2121
SimStatsFixture() : BaseFixture(), epSource(&agent.getEndpointManager()), simStats(agent) {
22+
boost::property_tree::ptree properties;
23+
agent.setProperties(properties);
2224
simStats.start();
2325
shared_ptr<modelgbp::policy::Universe> pUniverse =
2426
modelgbp::policy::Universe::resolve(framework).get();

‎agent-ovs/ovs/JsonRpcTransactMessage.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -174,19 +174,7 @@ bool JsonRpcTransactMessage::operator()(rapidjson::Writer<T> & writer) {
174174
string mutateRowOperation = toString(rowEntry.second.first);
175175
writer.String(mutateRowOperation.c_str());
176176
const TupleDataSet &tdsPtr = rowEntry.second.second;
177-
if (!tdsPtr.label.empty()) {
178-
writer.StartArray();
179-
writer.String(tdsPtr.label.c_str());
180-
writer.StartArray();
181-
LOG(DEBUG) << "label " << tdsPtr.label;
182-
for (auto &val : tdsPtr.tuples) {
183-
writePair<T>(writer, val, false);
184-
}
185-
writer.EndArray();
186-
writer.EndArray();
187-
} else {
188-
writePair(writer, *(tdsPtr.tuples.begin()), false);
189-
}
177+
writePair(writer, *(tdsPtr.tuples.begin()), false);
190178
writer.EndArray();
191179
}
192180
writer.EndArray();

‎agent-ovs/ovs/NetFlowRenderer.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,13 @@
1414
#include <opflexagent/NetFlowManager.h>
1515

1616
#include <boost/optional.hpp>
17-
18-
#include <boost/range/adaptors.hpp>
1917
#include <boost/format.hpp>
2018

2119

2220
namespace opflexagent {
2321
using boost::optional;
24-
using namespace boost::adaptors;
2522

2623
NetFlowRenderer::NetFlowRenderer(Agent& agent_) : JsonRpcRenderer(agent_) {
27-
2824
}
2925

3026
void NetFlowRenderer::start(const std::string& swName, OvsdbConnection* conn) {
@@ -94,13 +90,9 @@ namespace opflexagent {
9490
LOG(DEBUG) << "netflow/ipfix target " << target.c_str() << " version is " << std::to_string(expSt.get()->getVersion());
9591

9692
if (expSt.get()->getVersion() == CollectorVersionEnumT::CONST_V5) {
97-
LOG(DEBUG) << "creating netflow";
9893
uint32_t timeout = expSt.get()->getActiveFlowTimeOut();
99-
LOG(DEBUG) << "netflow timeout " << timeout;
10094
createNetFlow(target, timeout);
101-
} else if (expSt.get()->getVersion() ==
102-
CollectorVersionEnumT::CONST_V9) {
103-
LOG(DEBUG) << "creating IPFIX";
95+
} else if (expSt.get()->getVersion() == CollectorVersionEnumT::CONST_V9) {
10496
uint32_t sampling = expSt.get()->getSamplingRate();
10597
createIpfix(target, sampling);
10698
}
@@ -165,14 +157,12 @@ namespace opflexagent {
165157
}
166158

167159
void NetFlowRenderer::delConnectCb(const boost::system::error_code& ec,
168-
shared_ptr<ExporterConfigState> expSt) {
160+
shared_ptr<ExporterConfigState>& expSt) {
169161
if (ec) {
170162
connection_timer.reset();
171163
return;
172164
}
173165
LOG(DEBUG) << "timer span del cb";
174166
exporterDeleted(expSt);
175167
}
176-
177-
178168
}

‎agent-ovs/ovs/include/NetFlowRenderer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class NetFlowRenderer : public NetFlowListener,
9494

9595
private:
9696
void updateConnectCb(const boost::system::error_code& ec, const opflex::modb::URI& uri);
97-
void delConnectCb(const boost::system::error_code& ec, shared_ptr<ExporterConfigState> expSt);
97+
void delConnectCb(const boost::system::error_code& ec, shared_ptr<ExporterConfigState>& expSt);
9898
};
9999
}
100100
#endif //OPFLEX_NETFLOWRENDERER_H

‎agent-ovs/ovs/test/NetFlowRenderer_test.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,38 @@ class NetFlowRendererFixture : public BaseFixture {
4242
unique_ptr<OvsdbConnection> conn;
4343
};
4444

45-
static bool verifyCreateDestroy(const shared_ptr<NetFlowRenderer>& nfr) {
45+
bool verifyCreateDestroy(Agent& agent, const shared_ptr<NetFlowRenderer>& nfr) {
4646
nfr->setNextId(2000);
4747

48+
Mutator mutator(agent.getFramework(), "policyreg");
49+
auto root = modelgbp::dmtree::Root::createRootElement(agent.getFramework());
50+
auto pu = root->addPolicyUniverse();
51+
auto platform = pu->addPlatformConfig("platform");
52+
auto exporterConfig = platform->addNetflowExporterConfig("exporter");
53+
URI exporterURI = exporterConfig->getURI();
54+
4855
bool result = nfr->createNetFlow("5.5.5.6", 10);
49-
URI exporterURI("/PolicyUniverse/");
5056
ExporterConfigState state(exporterURI, "test");
5157
state.setVersion(1); // modelgbp::netflow::CollectorVersionEnumT::CONST_V5
5258
shared_ptr<ExporterConfigState> statePtr = make_shared<ExporterConfigState>(state);
5359
nfr->exporterDeleted(statePtr);
5460

5561
result = result && nfr->createIpfix("5.5.5.5", 500);
5662
statePtr->setVersion(2); // modelgbp::netflow::CollectorVersionEnumT::CONST_V9
63+
64+
exporterConfig->setDscp(99);
65+
exporterConfig->setSrcAddr("3.3.3.3");
66+
exporterConfig->setVersion(2);
67+
exporterConfig->setDstAddr("5.5.5.7");
68+
agent.getNetFlowManager().updateExporterConfigState(exporterConfig);
69+
nfr->exporterUpdated(exporterURI);
70+
5771
nfr->exporterDeleted(statePtr);
5872
return result;
5973
}
6074

6175
BOOST_FIXTURE_TEST_CASE( verify_createdestroy, NetFlowRendererFixture ) {
62-
BOOST_CHECK_EQUAL(true, verifyCreateDestroy(nfr));
76+
BOOST_CHECK_EQUAL(true, verifyCreateDestroy(agent, nfr));
6377
}
6478
BOOST_AUTO_TEST_SUITE_END()
6579

‎agent-ovs/ovs/test/include/MockRpcConnection.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class ResponseDict {
9696
/**
9797
* number of netflow responses to send
9898
*/
99-
static const unsigned int no_of_netflow_msgs = 10;
99+
static const unsigned int no_of_netflow_msgs = 14;
100100

101101
/**
102102
* rapidjson Document object array
@@ -234,6 +234,7 @@ class ResponseDict {
234234
selectPortsResp, selectPortsResp, selectPortsResp, createMirrorResp, interfaceInsertResp,
235235
selectPortsResp, updateBridgePortsResp, selectInterfaceResp,
236236
deleteResp, deleteResp, getBridgeUuidResp, createNetflowResp, deleteResp,
237+
deleteResp, deleteResp, getBridgeUuidResp, createIpFixResp,
237238
deleteResp, deleteResp, getBridgeUuidResp, createIpFixResp, deleteResp};
238239

239240
};

0 commit comments

Comments
 (0)