Skip to content

Commit c4969b7

Browse files
tomflynntomflynn
committed
Use mutate to update arrays to avoid having to rewrite entire list each time
Avoids a bunch of unneeded JSONRPC calls. Also limit columns returned where possible Signed-off-by: Tom Flynn <tom.flynn@gmail.com> Change-Id: I357055f5319ad5dd01c3b87d3a38b3e144a490fa
1 parent 1686bcc commit c4969b7

File tree

11 files changed

+119
-233
lines changed

11 files changed

+119
-233
lines changed

‎agent-ovs/lib/SpanManager.cpp

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
#include <opflexagent/SpanManager.h>
1313
#include <opflexagent/logging.h>
1414
#include <modelgbp/span/Universe.hpp>
15-
#include <modelgbp/gbp/EpGroup.hpp>
16-
#include <modelgbp/span/LocalEp.hpp>
1715
#include <modelgbp/epr/L2Universe.hpp>
1816
#include <modelgbp/epdr/EndPointToGroupRSrc.hpp>
1917

@@ -167,13 +165,12 @@ namespace opflexagent {
167165
}
168166

169167
void SpanManager::SpanUniverseListener::processSession(const shared_ptr<Session>& sess) {
170-
LOG(DEBUG) << "Process Session " << sess->getURI();
171-
shared_ptr<SessionState> sessState;
172168
auto itr = spanmanager.sess_map.find(sess->getURI());
173169
if (itr != spanmanager.sess_map.end()) {
174170
spanmanager.sess_map.erase(itr);
175171
}
176-
sessState = make_shared<SessionState>(sess->getURI(), sess->getName().get());
172+
shared_ptr<SessionState> sessState =
173+
make_shared<SessionState>(sess->getURI(), sess->getName().get());
177174
spanmanager.sess_map.insert(make_pair(sess->getURI(), sessState));
178175
sessState->setAdminState(sess->getState(1));
179176

@@ -245,7 +242,7 @@ namespace opflexagent {
245242
}
246243

247244
void SessionState::addSrcEndpoint(const SourceEndpoint& srcEp) {
248-
LOG(DEBUG) << "Adding src end point" << srcEp.getName();
245+
LOG(DEBUG) << "Adding src end point " << srcEp.getName();
249246
lock_guard<recursive_mutex> guard(opflexagent::SpanManager::updates);
250247
srcEndpoints.emplace(srcEp);
251248
}
@@ -331,7 +328,6 @@ namespace opflexagent {
331328
void SpanManager::SpanUniverseListener::addEndpoint(
332329
const shared_ptr<LocalEp>& lEp, const shared_ptr<L2Ep>& l2Ep,
333330
const URI& srcMemberUri, const unsigned char dir) {
334-
LOG(DEBUG) << "get parent lEp " << (lEp ? "set" : "null") << " l2Ep " << (l2Ep ? "set" : "null");
335331
optional<URI> parent = SpanManager::getSession(lEp);
336332
if (parent) {
337333
spanmanager.notifyUpdate.insert(parent.get());
@@ -372,14 +368,11 @@ namespace opflexagent {
372368
break;
373369
}
374370
}
375-
if (uriStr.empty()) {
376-
optional<URI> uri;
377-
return uri;
378-
} else {
379-
LOG(DEBUG) << "uri " << uriStr;
380-
optional<URI> uri(uriStr);
381-
return uri;
371+
optional<URI> uri;
372+
if (!uriStr.empty()) {
373+
uri = URI(uriStr);
382374
}
375+
return uri;
383376
}
384377

385378
optional<shared_ptr<SrcMember>> SpanManager::findSrcMem(const URI& sessUri, const URI& uri) {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include <opflex/modb/ObjectListener.h>
1818
#include <opflexagent/PolicyListener.h>
1919
#include <opflexagent/SpanListener.h>
20-
#include <modelgbp/span/Universe.hpp>
2120
#include <modelgbp/span/Session.hpp>
2221
#include <modelgbp/span/SrcGrp.hpp>
2322
#include <modelgbp/span/SrcMember.hpp>

‎agent-ovs/ovs/JsonRpc.cpp

Lines changed: 39 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,10 @@
1414

1515
#include <set>
1616
#include <map>
17-
#include <tuple>
1817
#include <regex>
19-
#include <string>
20-
#include <rapidjson/document.h>
2118
#include <rapidjson/stringbuffer.h>
2219

2320
#include "JsonRpc.h"
24-
#include <opflexagent/logging.h>
25-
#include "OvsdbConnection.h"
2621

2722
using namespace std::chrono;
2823

@@ -59,8 +54,8 @@ bool JsonRpc::createNetFlow(const string& brUuid, const string& target, const in
5954
msg1.kvPairs.emplace_back("uuid-name", uuid_name);
6055

6156
JsonRpcTransactMessage msg2(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
62-
set<tuple<string, string, string>> condSet;
63-
condSet.emplace("_uuid", "==", brUuid);
57+
set<tuple<string, OvsdbFunction, string>> condSet;
58+
condSet.emplace("_uuid", OvsdbFunction::EQ, brUuid);
6459
msg2.conditions = condSet;
6560

6661
tuples.clear();
@@ -92,8 +87,8 @@ bool JsonRpc::createIpfix(const string& brUuid, const string& target, const int&
9287
msg1.kvPairs.emplace_back("uuid-name", uuid_name);
9388

9489
JsonRpcTransactMessage msg2(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
95-
set<tuple<string, string, string>> condSet;
96-
condSet.emplace("_uuid", "==", brUuid);
90+
set<tuple<string, OvsdbFunction, string>> condSet;
91+
condSet.emplace("_uuid", OvsdbFunction::EQ, brUuid);
9792
msg2.conditions = condSet;
9893

9994
tuples.clear();
@@ -111,8 +106,8 @@ bool JsonRpc::createIpfix(const string& brUuid, const string& target, const int&
111106

112107
bool JsonRpc::deleteNetFlow(const string& brName) {
113108
JsonRpcTransactMessage msg1(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
114-
set<tuple<string, string, string>> condSet;
115-
condSet.emplace("name", "==", brName);
109+
set<tuple<string, OvsdbFunction, string>> condSet;
110+
condSet.emplace("name", OvsdbFunction::EQ, brName);
116111
msg1.conditions = condSet;
117112

118113
vector<TupleData> tuples;
@@ -129,8 +124,8 @@ bool JsonRpc::deleteNetFlow(const string& brName) {
129124

130125
bool JsonRpc::deleteIpfix(const string& brName) {
131126
JsonRpcTransactMessage msg1(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
132-
set<tuple<string, string, string>> condSet;
133-
condSet.emplace("name", "==", brName);
127+
set<tuple<string, OvsdbFunction, string>> condSet;
128+
condSet.emplace("name", OvsdbFunction::EQ, brName);
134129
msg1.conditions = condSet;
135130

136131
vector<TupleData> tuples;
@@ -161,26 +156,16 @@ bool JsonRpc::handleGetPortUuidResp(uint64_t reqId,
161156
return false;
162157
}
163158

164-
bool JsonRpc::updateBridgePorts(
165-
const string& brUuid, set<string>& brPorts, const string& port, bool action) {
166-
if (action) {
167-
// add port to list
168-
brPorts.emplace(port);
169-
} else {
170-
// remove port from list
171-
brPorts.erase(port);
172-
}
173-
JsonRpcTransactMessage msg1(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
174-
set<tuple<string, string, string>> condSet;
175-
condSet.emplace("_uuid", "==", brUuid);
159+
bool JsonRpc::updateBridgePorts(const string& brName, const string& portUuid, bool addToList) {
160+
JsonRpcTransactMessage msg1(OvsdbOperation::MUTATE, OvsdbTable::BRIDGE);
161+
set<tuple<string, OvsdbFunction, string>> condSet;
162+
condSet.emplace("name", OvsdbFunction::EQ, brName);
176163
msg1.conditions = condSet;
164+
177165
vector<TupleData> tuples;
178-
tuples.reserve(brPorts.size());
179-
for (auto& elem : brPorts) {
180-
tuples.emplace_back("uuid", elem);
181-
}
182-
TupleDataSet tdSet(tuples, "set");
183-
msg1.rowData.emplace("ports", tdSet);
166+
tuples.emplace_back("uuid", portUuid);
167+
TupleDataSet tdSet = TupleDataSet(tuples);
168+
msg1.mutateRowData.emplace("ports", std::make_pair(addToList ? OvsdbOperation::INSERT : OvsdbOperation::DELETE, tdSet));
184169

185170
const list<JsonRpcTransactMessage> requests = {msg1};
186171
if (!sendRequestAndAwaitResponse(requests)) {
@@ -190,41 +175,6 @@ bool JsonRpc::updateBridgePorts(
190175
return true;
191176
}
192177

193-
bool JsonRpc::handleGetBridgePortList(uint64_t reqId, const Document& payload, BrPortResult& result) {
194-
set<string> brPortSet;
195-
list<string> ids = {"0","rows","0","ports","0"};
196-
Value val;
197-
opflexagent::getValue(payload, ids, val);
198-
if (!val.IsNull() && val.IsString()) {
199-
const string valStr(val.GetString());
200-
ids = {"0", "rows", "0", "ports", "1"};
201-
val.SetObject();
202-
opflexagent::getValue(payload, ids, val);
203-
if (valStr == "uuid") {
204-
brPortSet.emplace(val.GetString());
205-
}
206-
} else {
207-
LOG(WARNING) << "Error getting port uuid";
208-
return false;
209-
}
210-
211-
if (val.IsArray()) {
212-
for (Value::ConstValueIterator itr1 = val.Begin();
213-
itr1 != val.End(); itr1++) {
214-
brPortSet.emplace((*itr1)[1].GetString());
215-
}
216-
}
217-
ids = {"0", "rows", "0", "_uuid", "1"};
218-
Value val3;
219-
opflexagent::getValue(payload, ids, val3);
220-
string brPortUuid = val3.GetString();
221-
tuple<string, set<string>> brPorts = make_tuple(brPortUuid, brPortSet);
222-
223-
result.brUuid = brPortUuid;
224-
result.portUuids.insert(brPortSet.begin(), brPortSet.end());
225-
return true;
226-
}
227-
228178
void JsonRpc::getUuidsFromVal(set<string>& uuidSet, const Document& payload, const string& index) {
229179
LOG(INFO) << "getUuidsFromVal with index " << index;
230180
list<string> ids = {"0","rows","0",index,"0"};
@@ -250,24 +200,11 @@ void JsonRpc::getUuidsFromVal(set<string>& uuidSet, const Document& payload, con
250200
}
251201
}
252202

253-
bool JsonRpc::getBridgePortList(const string& bridge, BrPortResult& res) {
254-
JsonRpcTransactMessage msg1(OvsdbOperation::SELECT, OvsdbTable::BRIDGE);
255-
set<tuple<string, string, string>> condSet;
256-
condSet.emplace("name", "==", bridge);
257-
msg1.conditions = condSet;
258-
msg1.columns.emplace("ports");
259-
msg1.columns.emplace("_uuid");
260-
const list<JsonRpcTransactMessage> requests = {msg1};
261-
if (!sendRequestAndAwaitResponse(requests)) {
262-
LOG(DEBUG) << "Error sending message";
263-
return false;
264-
}
265-
266-
return handleGetBridgePortList(pResp->reqId, pResp->payload, res);
267-
}
268-
269-
bool JsonRpc::getOvsdbMirrorConfig(mirror& mir) {
203+
bool JsonRpc::getOvsdbMirrorConfig(const string &brName, mirror &mir) {
270204
JsonRpcTransactMessage msg1(OvsdbOperation::SELECT, OvsdbTable::MIRROR);
205+
set<tuple<string, OvsdbFunction, string>> condSet;
206+
condSet.emplace("name", OvsdbFunction::EQ, brName);
207+
msg1.conditions = condSet;
271208
const list<JsonRpcTransactMessage> requests1 = {msg1};
272209
if (!sendRequestAndAwaitResponse(requests1)) {
273210
LOG(DEBUG) << "Error sending message";
@@ -310,9 +247,8 @@ bool JsonRpc::getOvsdbMirrorConfig(mirror& mir) {
310247
bool JsonRpc::getCurrentErspanParams(const string& portName, ErspanParams& params) {
311248
// for ERSPAN port get IP address
312249
JsonRpcTransactMessage msg1(OvsdbOperation::SELECT, OvsdbTable::INTERFACE);
313-
tuple<string, string, string> cond1("name", "==", portName);
314-
set<tuple<string, string, string>> condSet;
315-
condSet.emplace(cond1);
250+
set<tuple<string, OvsdbFunction, string>> condSet;
251+
condSet.emplace("name", OvsdbFunction::EQ, portName);
316252
msg1.conditions = condSet;
317253
msg1.columns.emplace("options");
318254

@@ -421,10 +357,10 @@ bool JsonRpc::getErspanOptions(const uint64_t reqId, const Document& payload, Er
421357

422358
void JsonRpc::getPortUuid(const string& name, string& uuid) {
423359
JsonRpcTransactMessage msg1(OvsdbOperation::SELECT, OvsdbTable::PORT);
424-
tuple<string, string, string> cond1("name", "==", name);
425-
set<tuple<string, string, string>> condSet;
426-
condSet.emplace(cond1);
360+
set<tuple<string, OvsdbFunction, string>> condSet;
361+
condSet.emplace("name", OvsdbFunction::EQ, name);
427362
msg1.conditions = condSet;
363+
msg1.columns.emplace("_uuid");
428364

429365
const list<JsonRpcTransactMessage> requests{msg1};
430366
if (!sendRequestAndAwaitResponse(requests)) {
@@ -449,8 +385,8 @@ void JsonRpc::getPortUuids(map<string, string>& ports) {
449385

450386
void JsonRpc::getBridgeUuid(const string& name, string& uuid) {
451387
JsonRpcTransactMessage msg1(OvsdbOperation::SELECT, OvsdbTable::BRIDGE);
452-
set<tuple<string, string, string>> condSet;
453-
condSet.emplace("name", "==", name);
388+
set<tuple<string, OvsdbFunction, string>> condSet;
389+
condSet.emplace("name", OvsdbFunction::EQ, name);
454390
msg1.conditions = condSet;
455391
msg1.columns.emplace("_uuid");
456392

@@ -533,13 +469,13 @@ bool JsonRpc::createMirror(const string& brUuid, const string& name, const set<s
533469

534470
// msg2
535471
tuples.clear();
536-
JsonRpcTransactMessage msg2(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
537-
set<tuple<string, string, string>> condSet;
538-
condSet.emplace("_uuid", "==", brUuid);
472+
JsonRpcTransactMessage msg2(OvsdbOperation::MUTATE, OvsdbTable::BRIDGE);
473+
set<tuple<string, OvsdbFunction, string>> condSet;
474+
condSet.emplace("_uuid", OvsdbFunction::EQ, brUuid);
539475
msg2.conditions = condSet;
540476
tuples.emplace_back("named-uuid", uuid_name);
541477
tdSet = TupleDataSet(tuples);
542-
msg2.rowData.emplace("mirrors", tdSet);
478+
msg2.mutateRowData.emplace("mirrors", std::make_pair(OvsdbOperation::INSERT, tdSet));
543479

544480
const list<JsonRpcTransactMessage> requests = {msg1, msg2};
545481
if (!sendRequestAndAwaitResponse(requests)) {
@@ -549,7 +485,7 @@ bool JsonRpc::createMirror(const string& brUuid, const string& name, const set<s
549485
return handleCreateMirrorResp(pResp->reqId, pResp->payload);
550486
}
551487

552-
bool JsonRpc::addErspanPort(const string& bridge, ErspanParams& params) {
488+
bool JsonRpc::addErspanPort(const string& bridgeName, ErspanParams& params) {
553489
JsonRpcTransactMessage msg1(OvsdbOperation::INSERT, OvsdbTable::PORT);
554490
vector<TupleData> tuples;
555491
tuples.emplace_back("", params.getPortName());
@@ -592,22 +528,13 @@ bool JsonRpc::addErspanPort(const string& bridge, ErspanParams& params) {
592528
tdSet = TupleDataSet(tuples, "map");
593529
msg2.rowData.emplace("options", tdSet);
594530

595-
// get bridge port list and add erspan port to it.
596-
BrPortResult res;
597-
if (!getBridgePortList(bridge, res)) {
598-
return false;
599-
}
600-
601-
JsonRpcTransactMessage msg3(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
531+
JsonRpcTransactMessage msg3(OvsdbOperation::MUTATE, OvsdbTable::BRIDGE);
602532
tuples.clear();
603-
for (const auto& elem : res.portUuids) {
604-
tuples.emplace_back("uuid", elem);
605-
}
606533
tuples.emplace_back("named-uuid", uuid_name);
607-
tdSet = TupleDataSet(tuples, "set");
608-
msg3.rowData.emplace("ports", tdSet);
609-
set<tuple<string, string, string>> condSet;
610-
condSet.emplace("_uuid", "==", res.brUuid);
534+
tdSet = TupleDataSet(tuples);
535+
msg3.mutateRowData.emplace("ports", std::make_pair(OvsdbOperation::INSERT, tdSet));
536+
set<tuple<string, OvsdbFunction, string>> condSet;
537+
condSet.emplace("name", OvsdbFunction::EQ, bridgeName);
611538
msg3.conditions = condSet;
612539

613540
const list<JsonRpcTransactMessage> requests = {msg1, msg2, msg3};
@@ -642,8 +569,8 @@ bool JsonRpc::handleCreateMirrorResp(uint64_t reqId, const Document& payload) {
642569

643570
bool JsonRpc::deleteMirror(const string& brName) {
644571
JsonRpcTransactMessage msg(OvsdbOperation::UPDATE, OvsdbTable::BRIDGE);
645-
set<tuple<string, string, string>> condSet;
646-
condSet.emplace("name", "==", brName);
572+
set<tuple<string, OvsdbFunction, string>> condSet;
573+
condSet.emplace("name", OvsdbFunction::EQ, brName);
647574
msg.conditions = condSet;
648575

649576
vector<TupleData> tuples;

0 commit comments

Comments
 (0)