Skip to content

Commit

Permalink
Merge pull request #402 from ansao-aci/master
Browse files Browse the repository at this point in the history 
System  security group  changes
  • Loading branch information
@mchalla
mchalla committed Sep 16, 2021
2 parents 65f8637 + 679d6b0 commit 1d7d5b5
Show file tree
Hide file tree
Showing 7 changed files with 341 additions and 59 deletions.
227 changes: 184 additions & 43 deletions agent-ovs/ovs/AccessFlowManager.cpp
View file

Large diffs are not rendered by default.

27 changes: 20 additions & 7 deletions agent-ovs/ovs/FlowUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,17 @@ void add_l2classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
uint16_t priority,
uint32_t flags, uint64_t cookie,
uint32_t svnid, uint32_t dvnid,
bool isSystemRule,
/* out */ FlowEntryList& entries) {
if (clsfr.isProtSet())
return;

ovs_be64 ckbe = ovs_htonll(cookie);
if (isSystemRule){
svnid = 0;
dvnid = 0;
}
FlowBuilder f;

f.cookie(ckbe)
.flags(flags);
flowutils::match_group(f, priority, svnid, dvnid);
Expand Down Expand Up @@ -203,12 +207,18 @@ void add_classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
uint16_t priority,
uint32_t flags, uint64_t cookie,
uint32_t svnid, uint32_t dvnid,
bool isSystemRule,
/* out */ FlowEntryList& entries) {
using modelgbp::l2::EtherTypeEnumT;
using modelgbp::l4::TcpFlagsEnumT;
ovs_be64 ckbe = ovs_htonll(cookie);
MaskList srcPorts;
MaskList dstPorts;

if (isSystemRule){
svnid = 0;
dvnid = 0;
}
if (clsfr.getProt(0) == 1 &&
(clsfr.isIcmpTypeSet() || clsfr.isIcmpCodeSet())) {
if (clsfr.isIcmpTypeSet()) {
Expand Down Expand Up @@ -287,6 +297,7 @@ void add_classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
for (const Mask& dm : dstPorts) {
for (uint32_t flagMask : tcpFlagsVec) {
FlowBuilder f;

f.cookie(ckbe);
f.flags(flags);

Expand Down Expand Up @@ -320,7 +331,7 @@ void add_classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
f.action().dropLog(currentTable,ActionBuilder::CaptureReason::POLICY_DENY,cookie).go(nextTable);
}
else {
f.action().metadata(0, flow::meta::DROP_LOG).go(nextTable);
f.action().metadata(0, flow::meta::DROP_LOG).go(nextTable);
}
break;
case flowutils::CA_ALLOW:
Expand Down Expand Up @@ -354,11 +365,13 @@ void add_classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
FlowBuilder::CT_NEW,
FlowBuilder::CT_TRACKED |
FlowBuilder::CT_NEW);
f.action().conntrack(ActionBuilder::CT_COMMIT,
MFF_REG6);
if(log) {
f.action().permitLog(currentTable, dropTable, cookie);
}
if (!isSystemRule) {
f.action().conntrack(ActionBuilder::CT_COMMIT,
MFF_REG6);
if(log) {
f.action().permitLog(currentTable, dropTable, cookie);
}
}
f.action().go(nextTable);
break;
case CA_REFLEX_FWD_EST:
Expand Down
4 changes: 4 additions & 0 deletions agent-ovs/ovs/IntFlowManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5938,6 +5938,7 @@ void IntFlowManager::addContractRules(FlowEntryList& entryList,
OFPUTIL_FF_SEND_FLOW_REM,
cookie,
cvnid, pvnid,
false,
entryList);
} else {
flowutils::add_classifier_entries(*cls, act, log,
Expand All @@ -5950,6 +5951,7 @@ void IntFlowManager::addContractRules(FlowEntryList& entryList,
OFPUTIL_FF_SEND_FLOW_REM,
cookie,
cvnid, pvnid,
false,
entryList);
}
}
Expand All @@ -5966,6 +5968,7 @@ void IntFlowManager::addContractRules(FlowEntryList& entryList,
OFPUTIL_FF_SEND_FLOW_REM,
cookie,
pvnid, cvnid,
false,
entryList);
} else {
flowutils::add_classifier_entries(*cls, act, log,
Expand All @@ -5978,6 +5981,7 @@ void IntFlowManager::addContractRules(FlowEntryList& entryList,
OFPUTIL_FF_SEND_FLOW_REM,
cookie,
pvnid, cvnid,
false,
entryList);
}
}
Expand Down
13 changes: 12 additions & 1 deletion agent-ovs/ovs/include/AccessFlowManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,21 @@ class AccessFlowManager : public EndpointListener,
* port after applying policy
*/
GROUP_MAP_TABLE_ID,
/**
* Enforece system security group policy on packets coming into
* the endpoint from switch.
*/
SYS_SEC_GRP_IN_TABLE_ID,
/**
* Enforce security group policy on packets coming in to the
* endpoint from the switch
*/
SEC_GROUP_IN_TABLE_ID,
/**
* Enforce system security group policy on packets coming out of
* the endpoints to the switch.
*/
SYS_SEC_GRP_OUT_TABLE_ID,
/**
* Enforce security group policy on packets coming out from
* the endpoint to the switch
Expand Down Expand Up @@ -177,7 +187,8 @@ class AccessFlowManager : public EndpointListener,
void handleSecGrpSetUpdate(const EndpointListener::uri_set_t& secGrps,
const std::string& secGrpsId);
void handleDscpQosUpdate(const string& interface, uint8_t dscp);

bool checkIfSystemSecurityGroup(const string& uri);

Agent& agent;
SwitchManager& switchManager;
IdGenerator& idGen;
Expand Down
2 changes: 2 additions & 0 deletions agent-ovs/ovs/include/FlowUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ void add_classifier_entries(modelgbp::gbpe::L24Classifier& clsfr,
uint16_t priority,
uint32_t flags, uint64_t cookie,
uint32_t svnid, uint32_t dvnid,
bool isSystemRule,
/* out */ FlowEntryList& entries);

/**
Expand All @@ -145,6 +146,7 @@ void add_l2classifier_entries(modelgbp::gbpe::L24Classifier& clsfr,
uint16_t priority,
uint32_t flags, uint64_t cookie,
uint32_t svnid, uint32_t dvnid,
bool isSystemRule,
/* out */ FlowEntryList& entries);
/**
* Add a match entry for the DHCP v4 and v6 request
Expand Down
124 changes: 117 additions & 7 deletions agent-ovs/ovs/test/AccessFlowManager_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ class AccessFlowManagerFixture : public FlowManagerFixture {
void initExpSecGrp5(std::vector<std::pair<std::string,uint16_t>>& namedAddressSet);
void initExpSecGrp6();

/** Initialize system security group flow entries */
void initExpSysSecGrpSet1();

/** Initialize dscp flow entries */
void addDscpFlows(shared_ptr<Endpoint>& ep);

Expand All @@ -76,6 +79,8 @@ class AccessFlowManagerFixture : public FlowManagerFixture {
shared_ptr<SecGroup> secGrp1;
shared_ptr<SecGroup> secGrp2;
shared_ptr<SecGroup> secGrp3;
shared_ptr<SecGroup> sysSecGrp1;

shared_ptr<L24Classifier> classifier100,classifier101;

shared_ptr<modelgbp::policy::Space> pSpace;
Expand Down Expand Up @@ -235,7 +240,7 @@ BOOST_FIXTURE_TEST_CASE(learningBridge, AccessFlowManagerFixture) {

#define ADDF(flow) addExpFlowEntry(expTables, flow)
enum TABLE {
DROP_LOG=0, SVC_BYPASS = 1, GRP = 2, IN_POL = 3, OUT_POL = 4, TAP=5, OUT = 6, EXP_DROP=7
DROP_LOG=0, SVC_BYPASS = 1, GRP = 2, SYS_IN_POL = 3, IN_POL = 4, SYS_OUT_POL = 5, OUT_POL = 6, TAP=7, OUT = 8, EXP_DROP=9
};

enum CaptureReason {
Expand Down Expand Up @@ -397,6 +402,58 @@ BOOST_FIXTURE_TEST_CASE(secGrp, AccessFlowManagerFixture) {
WAIT_FOR_TABLES("remote-addsubnets", 500);
}

BOOST_FIXTURE_TEST_CASE(syssecgrp, AccessFlowManagerFixture) {
createObjects();
createPolicyObjects();
shared_ptr<modelgbp::gbp::Subnets> rs;
{
Mutator mutator(framework, "policyreg");
rs = space->addGbpSubnets("subnets_rule0");

rs->addGbpSubnet("subnets_rule0_1")
->setAddress("0.0.0.0")
.setPrefixLen(0);
rs->addGbpSubnet("subnets_rule0_2")
->setAddress("0::")
.setPrefixLen(0);

shared_ptr<modelgbp::gbp::SecGroupRule> r1, r2, r3, r4, r5 ;
sysSecGrp1 = space->addGbpSecGroup("ostack_SystemSecurityGroup");

r1 = sysSecGrp1->addGbpSecGroupSubject("2_subject1")
->addGbpSecGroupRule("2_1_rule1");
r1->addGbpRuleToClassifierRSrc(classifier0->getURI().toString());
r1->addGbpSecGroupRuleToRemoteAddressRSrc(rs->getURI().toString());

r2 = sysSecGrp1->addGbpSecGroupSubject("2_subject1")
->addGbpSecGroupRule("2_1_rule2");
r2->setDirection(DirectionEnumT::CONST_BIDIRECTIONAL).setOrder(20)
.addGbpRuleToClassifierRSrc(classifier5->getURI().toString());

r3 = sysSecGrp1->addGbpSecGroupSubject("2_subject1")
->addGbpSecGroupRule("2_1_rule3");
r3->setDirection(DirectionEnumT::CONST_OUT).setOrder(30)
.addGbpRuleToClassifierRSrc(classifier9->getURI().toString());
r3->addGbpSecGroupRuleToRemoteAddressRSrc(rs->getURI().toString());
mutator.commit();

}

ep0.reset(new Endpoint("0-0-0-0"));
epSrc.updateEndpoint(*ep0);

initExpStatic();
WAIT_FOR_TABLES("empty-secgrp", 500);

ep0->addSecurityGroup(sysSecGrp1->getURI());
epSrc.updateEndpoint(*ep0);

clearExpFlowTables();
initExpStatic();
initExpSysSecGrpSet1();
WAIT_FOR_TABLES("syssecgrp", 500);
}

BOOST_FIXTURE_TEST_CASE(denyrule, AccessFlowManagerFixture) {
createObjects();
createPolicyObjects();
Expand Down Expand Up @@ -676,6 +733,8 @@ void AccessFlowManagerFixture::initExpStatic() {
.actions().go(SVC_BYPASS).done());
ADDF(Bldr().table(SVC_BYPASS).priority(1)
.actions().go(GRP).done());
ADDF(Bldr().table(SYS_IN_POL).priority(1).actions().go(IN_POL).done());
ADDF(Bldr().table(SYS_OUT_POL).priority(1).actions().go(OUT_POL).done());
for(int i=SVC_BYPASS; i<=OUT; i++) {
ADDF(Bldr().table(i).priority(0)
.cookie(ovs_ntohll(opflexagent::flow::cookie::TABLE_DROP_FLOW))
Expand Down Expand Up @@ -755,7 +814,7 @@ void AccessFlowManagerFixture::initExpEp(shared_ptr<Endpoint>& ep) {
.meta((opflexagent::flow::meta::access_out::POP_VLAN|
opflexagent::flow::meta::access_meta::EGRESS_DIR),
opflexagent::flow::meta::ACCESS_MASK)
.go(OUT_POL).done());
.go(SYS_OUT_POL).done());
if (ep->isAccessAllowUntagged()) {
ADDF(Bldr().table(GRP).priority(99).in(access)
.isVlanTci("0x0000/0x1fff")
Expand All @@ -764,29 +823,29 @@ void AccessFlowManagerFixture::initExpEp(shared_ptr<Endpoint>& ep) {
.load(OUTPORT, uplink)
.meta(opflexagent::flow::meta::access_meta::EGRESS_DIR,
opflexagent::flow::meta::access_meta::MASK)
.go(OUT_POL).done());
.go(SYS_OUT_POL).done());
}
ADDF(Bldr().table(GRP).priority(100).in(uplink)
.actions().load(RD, zoneId).load(SEPG, 1).load(OUTPORT, access)
.load(FD, ep->getAccessIfaceVlan().get())
.meta((opflexagent::flow::meta::access_out::PUSH_VLAN|
opflexagent::flow::meta::access_meta::INGRESS_DIR),
opflexagent::flow::meta::ACCESS_MASK)
.go(IN_POL).done());
.go(SYS_IN_POL).done());
} else {
ADDF(Bldr().table(GRP).priority(100).in(access)
.noVlan()
.actions().load(RD, zoneId).load(SEPG, 1)
.load(OUTPORT, uplink)
.meta(opflexagent::flow::meta::access_meta::EGRESS_DIR,
opflexagent::flow::meta::access_meta::MASK)
.go(OUT_POL).done());
.go(SYS_OUT_POL).done());
ADDF(Bldr().table(GRP).priority(100).in(uplink)
.actions().load(RD, zoneId).load(SEPG, 1)
.load(OUTPORT, access)
.meta(opflexagent::flow::meta::access_meta::INGRESS_DIR,
opflexagent::flow::meta::access_meta::MASK)
.go(IN_POL).done());
.go(SYS_IN_POL).done());
}
}

Expand Down Expand Up @@ -815,6 +874,57 @@ void AccessFlowManagerFixture::initExpSecGrpSet12(bool second,
initExpSecGrp2(setId);
}

void AccessFlowManagerFixture::initExpSysSecGrpSet1(){
uint16_t prio = PolicyManager::MAX_POLICY_RULE_PRIORITY;
PolicyManager::rule_list_t rules;
agent.getPolicyManager().getSecGroupRules(sysSecGrp1->getURI(), rules);
uint32_t ruleId;

ADDF(Bldr().table(SYS_IN_POL).cookie(ovs_ntohll(opflexagent::flow::cookie::TABLE_DROP_FLOW))
.flags(OFPUTIL_FF_SEND_FLOW_REM).priority(2)
.actions().dropLog(SYS_IN_POL).go(EXP_DROP).done());

ADDF(Bldr().table(SYS_OUT_POL).cookie(ovs_ntohll(opflexagent::flow::cookie::TABLE_DROP_FLOW))
.flags(OFPUTIL_FF_SEND_FLOW_REM).priority(2)
.actions().dropLog(SYS_OUT_POL).go(EXP_DROP).done());

/* classifier 5 */
ruleId = idGen.getId("l24classifierRule",
classifier5->getURI().toString());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_IN_POL).priority(prio).cookie(ruleId)
.isEth(0x8906).actions().go(IN_POL).done());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_OUT_POL).priority(prio).cookie(ruleId)
.isEth(0x8906).actions().go(OUT_POL).done());

/* classifier 9 */
ruleId = idGen.getId("l24classifierRule",
classifier9->getURI().toString());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_IN_POL).priority(prio - 128).cookie(ruleId)
.isCtState("-new+est-rel+rpl-inv+trk").tcp()
.actions().go(IN_POL).done());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_IN_POL).priority(prio - 128).cookie(ruleId)
.isCtState("-new-est+rel+rpl-inv+trk").ip()
.actions().go(IN_POL).done());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_IN_POL).priority(prio - 128)
.isCtState("-trk").tcp()
.actions().ct("table=2,zone=NXM_NX_REG6[0..15]").done());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_OUT_POL).priority(prio - 128).cookie(ruleId)
.isCtState("-trk")
.tcp().isTpDst(22)
.actions().ct("table=2,zone=NXM_NX_REG6[0..15]").done());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_OUT_POL).priority(prio - 128).cookie(ruleId)
.isCtState("+est+trk")
.tcp().isTpDst(22)
.actions()
.go(OUT_POL).done());
ADDF(Bldr(SEND_FLOW_REM).table(SYS_OUT_POL).priority(prio - 128).cookie(ruleId)
.isCtState("+new+trk")
.tcp().isTpDst(22)
.actions()
.go(OUT_POL).done());

}

uint16_t AccessFlowManagerFixture::initExpSecGrp1(uint32_t setId,
int remoteAddress) {
uint16_t prio = PolicyManager::MAX_POLICY_RULE_PRIORITY;
Expand Down Expand Up @@ -886,6 +996,7 @@ uint16_t AccessFlowManagerFixture::initExpSecGrp1(uint32_t setId,
return 512;
}


uint16_t AccessFlowManagerFixture::initExpSecGrp2(uint32_t setId) {
uint16_t prio = PolicyManager::MAX_POLICY_RULE_PRIORITY;
PolicyManager::rule_list_t rules;
Expand Down Expand Up @@ -1010,6 +1121,5 @@ void AccessFlowManagerFixture::initExpSecGrp6() {
ADDF(Bldr(SEND_FLOW_REM).table(OUT_POL).priority(prio).cookie(ruleId)
.tcp().reg(SEPG, setId).isIpDst("10.0.0.0/16").isTpDst(80).actions()
.permitLog(OUT_POL,EXP_DROP,ruleId).go(TAP).done());

}
BOOST_AUTO_TEST_SUITE_END()
3 changes: 2 additions & 1 deletion agent-ovs/ovs/test/TableDropStatsManager_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ void TableDropStatsManagerFixture::testOneStaticDropFlow (
{
uint64_t expected_pkt_count = INITIAL_PACKET_COUNT,
expected_byte_count = INITIAL_PACKET_COUNT * PACKET_SIZE;
int ctr = 1;
FlowEntryList dropLogFlows;
if(portConn.getSwitchName()=="int_conn") {
createIntBridgeDropFlowList(table_id,
Expand All @@ -299,10 +300,10 @@ void TableDropStatsManagerFixture::testOneStaticDropFlow (
expected_byte_count *=4;
}
} else {
ctr = 2;
createAccBridgeDropFlowList(table_id,
dropLogFlows);
}
int ctr = 1;
if(refresh_aged_flow) {
ctr++;
}
Expand Down

0 comments on commit 1d7d5b5

Please sign in to comment.