minor Logaction changes on PolicyManager side (#292)

* minor Logaction changes on PolicyManager side * modified genie model and addresses the review comments Co-authored-by: bashokba <bashokba@cisco.com>
noironetworks · Oct 23, 2020 · bf30e36 · bf30e36
1 parent 39e659f
commit bf30e36
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 20 deletions.
diff --git a/agent-ovs/lib/PolicyManager.cpp b/agent-ovs/lib/PolicyManager.cpp
@@ -1196,10 +1196,10 @@ static bool updatePolicyRules(OFFramework& framework,
                     newRedirGrps.insert(destGrpUri.get());
                 }
                 else if(r->getTargetClass().get() == LogAction::CLASS_ID) {
-                        optional<shared_ptr<modelgbp::gbp::LogAction> > resloveLog = 
-                                       modelgbp::gbp::LogAction::resolve(framework,r->getTargetURI().get());
-                    if (resloveLog) {
-                        ruleLog = resloveLog.get()->getLog(0) != 0 ;
+                        optional<shared_ptr<LogAction> > resolveLog = 
+                           LogAction::resolve(framework,r->getTargetURI().get());
+                    if (resolveLog) {
+                        ruleLog = true;
                     }
                 }
             }
@@ -1208,12 +1208,12 @@ static bool updatePolicyRules(OFFramework& framework,
             uint16_t clsPrio = 0;
             for (const shared_ptr<L24Classifier>& c : classifiers) {
                 newRules.push_back(std::
-                                   make_shared<PolicyRule>(dir,
-                                                           rulePrio - clsPrio,
-                                                           c, ruleAllow,
-                                                           remoteSubnets,
-                                                           ruleRedirect, ruleLog,
-                                                           destGrpUri));
+                            make_shared<PolicyRule>(dir,
+                                                    rulePrio - clsPrio,
+                                                    c, ruleAllow,
+                                                    remoteSubnets,
+                                                    ruleRedirect, ruleLog,
+                                                    destGrpUri));
                 if (clsPrio < 127)
                     clsPrio += 1;
             }

diff --git a/agent-ovs/lib/include/opflexagent/test/ModbFixture.h b/agent-ovs/lib/include/opflexagent/test/ModbFixture.h
@@ -455,12 +455,11 @@ class ModbFixture : public BaseFixture {
         epg0->addGbpEpGroupToProvContractRSrc(con3->getURI().toString());
         epg1->addGbpEpGroupToConsContractRSrc(con3->getURI().toString());
 
-           //action 1      
+           //action 3      
         action3 = space->addGbpAllowDenyAction("action3");
         action3->setAllow(0);
-          //action 2
+          //action 4
         action4 =  space->addGbpLogAction("action4");
-        action4->setLog(1);
 
         con4 = space->addGbpContract("contract4");
         con4->addGbpSubject("4_subject1")->addGbpRule("4_1_rule1")

diff --git a/agent-ovs/ovs/FlowUtils.cpp b/agent-ovs/ovs/FlowUtils.cpp
@@ -142,7 +142,7 @@ void add_l2classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
     if (act != flowutils::CA_DENY)
         f.action().go(nextTable);
     if (act == flowutils::CA_DENY) {
-       if (log != 0) { 
+       if (log) {
           f.action().dropLog(currentTable,ActionBuilder::CaptureReason::POLICY_DENY, cookie).go(nextTable);
         }
        else {
@@ -268,7 +268,7 @@ void add_classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
 
                         switch (act) {
                         case flowutils::CA_DENY:
-                             if (log != 0) {
+                             if (log) {
                                  f.action().dropLog(currentTable,ActionBuilder::CaptureReason::POLICY_DENY,cookie).go(nextTable);
                              }
                              else {

diff --git a/agent-ovs/ovs/test/AccessFlowManager_test.cpp b/agent-ovs/ovs/test/AccessFlowManager_test.cpp
@@ -411,7 +411,6 @@ BOOST_FIXTURE_TEST_CASE(denyrule, AccessFlowManagerFixture) {
        action1->setAllow(0).setOrder(5);
        //action 2
        action2 =  space->addGbpLogAction("action2");
-       action2->setLog(1);
        //security group rule
        r1 = secGrp3->addGbpSecGroupSubject("1_subject1")
                    ->addGbpSecGroupRule("1_1_rule1");

diff --git a/genie/MODEL/SPECIFIC/GBP/COMMON/action.mdl b/genie/MODEL/SPECIFIC/GBP/COMMON/action.mdl
@@ -40,10 +40,6 @@ module[gbp]
     class[LogAction;
           super=gbp/Action;
           concrete]
-    {
-        # Set to nonzero to log, or zero to skip logging
-        member[log; type=scalar/UInt8]
-    }
 
     class[RedirectDest;
           super=gbp/BaseNextHop;