pcn-iptables: fix horus module

Signed-off-by: Matteo Bertrone <m.bertrone@gmail.com>

Author: mbertrone

src/services/pcn-iptables/src/Chain.h

@@ -127,7 +127,7 @@ class Chain : public ChainInterface {
       std::map<struct HorusRule, struct HorusValue> &horus,
       const std::vector<std::shared_ptr<ChainRule>> &rules);
 
-  static void fromRuleToHorusKeyValue(std::shared_ptr<ChainRule> rule,
+  static bool fromRuleToHorusKeyValue(std::shared_ptr<ChainRule> rule,
                                       struct HorusRule &key,
                                       struct HorusValue &value);
 };

src/services/pcn-iptables/src/Utils.cpp

@@ -534,11 +534,16 @@ bool Chain::interfaceFromRulesToMap(
   return brk;
 }
 
-void Chain::fromRuleToHorusKeyValue(std::shared_ptr<ChainRule> rule,
+bool Chain::fromRuleToHorusKeyValue(std::shared_ptr<ChainRule> rule,
                                     struct HorusRule &key,
                                     struct HorusValue &value) {
   key.setFields = 0;
 
+  try {
+    rule->getConntrack();
+    return false;
+  } catch (std::runtime_error) {}
+
   try {
     IpAddr ips;
     ips.fromString(rule->getSrc());
@@ -590,7 +595,7 @@ void Chain::fromRuleToHorusKeyValue(std::shared_ptr<ChainRule> rule,
     value.action = 1;
 
   value.ruleID = rule->getId();
-  return;
+  return true;
 }
 
 void Chain::horusFromRulesToMap(
@@ -599,30 +604,28 @@ void Chain::horusFromRulesToMap(
   struct HorusRule key;
   struct HorusValue value;
 
-  bool first_rule = true;
   uint64_t set_fields = 0;
 
   // find match pattern for first rule (e.g. 01101 means ips proto ports set)
 
   int i = 0;
 
   for (auto const &rule : rules) {
-    i++;
-    fromRuleToHorusKeyValue(rule, key, value);
-
-    if (i == 1) {
+    if (i >= HorusConst::MAX_RULE_SIZE_FOR_HORUS)
+      break;
+    if(!fromRuleToHorusKeyValue(rule, key, value))
+      return;
+    if (i == 0) {
       if (key.setFields == 0) {
         break;
       }
-
       set_fields = key.setFields;
     }
-
     if (key.setFields != set_fields) {
       break;
     }
-
     horus.insert(std::pair<struct HorusRule, struct HorusValue>(key, value));
+    i++;
   }
 }
 

src/services/pcn-iptables/src/datapaths/Iptables_Horus_dp.c

@@ -30,7 +30,7 @@ struct packetHeaders {
   uint32_t seqN;
   uint32_t ackN;
   uint8_t connStatus;
-};
+} __attribute__((packed));
 
 struct horusKey {
 #if _SRCIP
@@ -69,14 +69,13 @@ enum {
 
 BPF_TABLE("extern", int, struct packetHeaders, packet, 1);
 
-// TODO 1024 -> const
-BPF_TABLE("hash", struct horusKey, struct horusValue, horusTable, 1024);
+BPF_TABLE("hash", struct horusKey, struct horusValue, horusTable, _MAX_RULE_SIZE_FOR_HORUS);
 
 BPF_TABLE("extern", int, int, forwardingDecision, 1);
 
 // Per-CPU maps used to keep counter of HORUS rules
-BPF_TABLE("percpu_array", int, u64, pkts_horus, 1024);
-BPF_TABLE("percpu_array", int, u64, bytes_horus, 1024);
+BPF_TABLE("percpu_array", int, u64, pkts_horus, _MAX_RULE_SIZE_FOR_HORUS);
+BPF_TABLE("percpu_array", int, u64, bytes_horus, _MAX_RULE_SIZE_FOR_HORUS);
 
 static __always_inline void incrementHorusCounters(u32 ruleID, u32 bytes) {
   u64 *value;
@@ -133,18 +132,18 @@ static int handle_rx(struct CTXTYPE *ctx, struct pkt_metadata *md) {
   struct horusValue *value;
   value = horusTable.lookup(&key);
 
-  pcn_log(ctx, LOG_DEBUG, "HORUS key: %x. pkt: %x", key.srcIp, pkt->srcIp);
+  pcn_log(ctx, LOG_DEBUG, "Horus srcIp: %I dstIp: %I", pkt->srcIp, pkt->dstIp);
   if (value == NULL) {
     // Miss, goto pipleline
     goto PIPELINE;
   } else {
     // Independently from the final action (ACCEPT or DROP)
     // I have to update the counters
-    if (value->ruleID <= 1024) {
+    if (value->ruleID < _MAX_RULE_SIZE_FOR_HORUS) {
       pcn_log(ctx, LOG_DEBUG, "HORUS RuleID: %d", value->ruleID);
       incrementHorusCounters(value->ruleID, md->packet_len);
     } else {
-      pcn_log(ctx, LOG_DEBUG, "HORUS RuleID is greater than 1024");
+      pcn_log(ctx, LOG_DEBUG, "HORUS RuleID is greater than _MAX_RULE_SIZE_FOR_HORUS");
       goto PIPELINE;
     }
 
@@ -165,4 +164,4 @@ PIPELINE:;
   pcn_log(ctx, LOG_DEBUG, "HORUS Lookup MISS. Goto PIPELINE. ");
   call_bpf_program(ctx, _CHAINSELECTOR);
   return RX_DROP;
-}
+}

src/services/pcn-iptables/src/defines.h

@@ -124,7 +124,7 @@ const uint8_t DSTPORT = 4;
 // apply Horus mitigator optimization if there are at least # rules
 // matching the pattern, at ruleset begin
 const uint8_t MIN_RULE_SIZE_FOR_HORUS = 1;
-const uint8_t MAX_RULE_SIZE_FOR_HORUS = -1;  // not used
+const uint8_t MAX_RULE_SIZE_FOR_HORUS = 2048;
 
 // Enable Horus optimization
 // We want to disable Horus by default while we decide a policy to apply it or

src/services/pcn-iptables/src/modules/Horus.cpp

@@ -131,6 +131,9 @@ std::string Iptables::Horus::getCode() {
   replaceAll(no_macro_code, "_CONNTRACK_LABEL_INGRESS",
              std::to_string(ModulesConstants::CONNTRACKLABEL_INGRESS));
 
+  replaceAll(no_macro_code, "_MAX_RULE_SIZE_FOR_HORUS",
+             std::to_string(HorusConst::MAX_RULE_SIZE_FOR_HORUS));
+
   if (program_type_ == ProgramType::INGRESS) {
     replaceAll(no_macro_code, "call_bpf_program", "call_ingress_program");
   } else if (program_type_ == ProgramType::EGRESS) {
@@ -224,7 +227,7 @@ void Iptables::Horus::updateTableValue(struct HorusRule horus_key,
   }
 
   char buffer[2 * sizeof(uint32_t) +
-              2 * sizeof(uint16_t) /*+ sizeof(uint8_t)*/] = {0};
+              2 * sizeof(uint16_t) + sizeof(uint8_t)] = {0};
   auto table = iptables_.get_raw_table("horusTable", index_);
   table.set(key.toData(buffer), &horus_value);
 }