dpdk/rte_flow: add rule error log, globalize free

adaki4 · Dec 3, 2024 · 83763e2 · 83763e2
1 parent 8c99dbc
commit 83763e2
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 21 deletions.
diff --git a/src/runmode-dpdk.c b/src/runmode-dpdk.c
@@ -316,6 +316,7 @@ static void DPDKDerefConfig(void *conf)
 {
     SCEnter();
     DPDKIfaceConfig *iconf = (DPDKIfaceConfig *)conf;
+    iconf->RTERulesFree(&iconf->drop_filter);
 
     if (SC_ATOMIC_SUB(iconf->ref, 1) == 1) {
         if (iconf->pkt_mempool != NULL) {
@@ -340,6 +341,7 @@ static void ConfigInit(DPDKIfaceConfig **iconf)
     SC_ATOMIC_INIT(ptr->ref);
     (void)SC_ATOMIC_ADD(ptr->ref, 1);
     ptr->DerefFunc = DPDKDerefConfig;
+    ptr->RTERulesFree = RuleStorageFree;
     ptr->flags = 0;
 
     *iconf = ptr;

diff --git a/src/source-dpdk.c b/src/source-dpdk.c
@@ -635,14 +635,13 @@ static TmEcode ReceiveDPDKThreadInit(ThreadVars *tv, const void *initdata, void
             goto fail;
         }
 
-        // some PMDs requires additional actions only after the device has started
-        DevicePostStartPMDSpecificActions(ptv, dev_info.driver_name);
-        retval = CreateRules(dpdk_config->iface, dpdk_config->port_id, &dpdk_config->drop_filter);
+        retval = CreateRules(dpdk_config->iface, dpdk_config->port_id, &dpdk_config->drop_filter, dev_info.driver_name);
         if (retval != 0) {
-            SCLogError("%s: error (%s) when creating rte_flow rules", dpdk_config->iface,
-                    rte_strerror(-retval));
+            SCLogError("%s: error when creating rte_flow rules", dpdk_config->iface);
             goto fail;
         }
+        // some PMDs requires additional actions only after the device has started
+        DevicePostStartPMDSpecificActions(ptv, dev_info.driver_name);
 
         uint16_t inconsistent_numa_cnt = SC_ATOMIC_GET(dpdk_config->inconsistent_numa_cnt);
         if (inconsistent_numa_cnt > 0 && ptv->port_socket_id != SOCKET_ID_ANY) {
@@ -658,8 +657,17 @@ static TmEcode ReceiveDPDKThreadInit(ThreadVars *tv, const void *initdata, void
         }
     }
 
+    // Save rte_flow rules from being destroyed
+    char **tmp = dpdk_config->drop_filter.rules;
+    dpdk_config->drop_filter.rules = NULL;
+
     *data = (void *)ptv;
     dpdk_config->DerefFunc(dpdk_config);
+
+    // Restore rte_flow rules
+    dpdk_config->drop_filter.rules = tmp;
+    tmp = NULL;
+
     SCReturnInt(TM_ECODE_OK);
 
 fail:

diff --git a/src/source-dpdk.h b/src/source-dpdk.h
@@ -87,6 +87,7 @@ typedef struct DPDKIfaceConfig_ {
     SC_ATOMIC_DECLARE(uint16_t, inconsistent_numa_cnt);
     DPDKWorkerSync *workers_sync;
     void (*DerefFunc)(void *);
+    void (*RTERulesFree)(RuleStorage *);
 
     struct rte_flow *flow[100];
 #endif

diff --git a/src/util-dpdk-rte-flow.c b/src/util-dpdk-rte-flow.c
@@ -44,7 +44,6 @@
 static int RuleStorageSetup(RuleStorage *);
 static int RuleStorageAddRule(RuleStorage *, const char *);
 static int RuleStorageExtendCapacity(RuleStorage *);
-static void RuleStorageFree(RuleStorage *);
 
 static int RuleStorageSetup(RuleStorage *rule_storage)
 {
@@ -101,7 +100,7 @@ static int RuleStorageExtendCapacity(RuleStorage *rule_storage)
     SCReturnInt(0);
 }
 
-static void RuleStorageFree(RuleStorage *rule_storage)
+void RuleStorageFree(RuleStorage *rule_storage)
 {
     if (rule_storage->rules == NULL) {
         SCReturn;
@@ -141,10 +140,31 @@ int ConfigLoadRTEFlowRules(
     SCReturnInt(0);
 }
 
-int CreateRules(char *port_name, int port_id, RuleStorage *rule_storage)
+/**
+ * \brief Check and log whether pattern is broad / not-specific
+ *        as ice does not accept them                  */
+static void iceDeviceError(struct rte_flow_item *items) {
+    int i = 0;
+    while (items[i].type != RTE_FLOW_ITEM_TYPE_END) {
+        if (items[i].spec != NULL) {
+            SCReturn;
+        }
+        ++i;
+    }
+    SCLogError("ice driver does not support broad patterns");
+
+}
+
+static void DriverSpecificErrorMessage(char *driver_name, struct rte_flow_item *items) {
+    if (strcmp(driver_name, "net_ice") == 0) {
+        iceDeviceError(items);
+    }
+}
+
+int CreateRules(char *port_name, int port_id, RuleStorage *rule_storage, char *driver_name)
 {
     SCEnter();
-    int retval = 0;
+    bool failed = false;
     struct rte_flow_error flush_error = { 0 };
     struct rte_flow_attr attr = { 0 };
     struct rte_flow_action action[] = { { 0 }, { 0 } };
@@ -161,34 +181,37 @@ int CreateRules(char *port_name, int port_id, RuleStorage *rule_storage)
 
         int ret = ParsePattern(rule_storage->rules[i], data, sizeof(data), &items);
         if (ret != 0) {
-            retval |= ret;
+            failed = true;
             SCLogError("Error when parsing rte_flow rule: %s", rule_storage->rules[i]);
             continue;
         }
-
+        ret = rte_flow_validate(port_id, &attr, items, action, &flow_error);
+        if (ret != 0) {
+            failed = true;
+            SCLogError("Error when validating rte_flow rule with pattern %s for port %s: %s errmsg: %s", rule_storage->rules[i], port_name,
+                rte_strerror(-ret), flow_error.message);
+            DriverSpecificErrorMessage(driver_name, items);
+            continue;
+        }
         flow = rte_flow_create(port_id, &attr, items, action, &flow_error);
         if (flow == NULL) {
+            failed = true;
             SCLogError("Error when creating rte_flow rule with pattern %s on %s: %s",
-                    rule_storage->rules[i], port_name, flow_error.message);
-            ret = rte_flow_validate(port_id, &attr, items, action, &flow_error);
-            retval |= ret;
-            SCLogError("Error on rte_flow validation for port %s: %s errmsg: %s", port_name,
-                    rte_strerror(-retval), flow_error.message);
+                rule_storage->rules[i], port_name, flow_error.message);
         } else {
             SCLogInfo("rte_flow rule with pattern: %s created for port %s", rule_storage->rules[i],
                     port_name);
         }
-    }
 
+    }
     RuleStorageFree(rule_storage);
 
-    if (retval != 0) {
+    if (failed) {
         SCLogError("Error parsing/creating rte_flow rule(s), flushing rules on port %s", port_name);
         int ret = rte_flow_flush(port_id, &flush_error);
         if (ret != 0) {
             SCLogError("Unable to flush rte_flow rules of %s: %s Flush error msg: %s", port_name,
-                    rte_strerror(-retval), flush_error.message);
-            retval = ret;
+                    rte_strerror(-ret), flush_error.message);
         }
         SCReturn(-1);
     }

diff --git a/src/util-dpdk-rte-flow.h b/src/util-dpdk-rte-flow.h
@@ -36,8 +36,9 @@
 #ifndef SURICATA_RTE_FLOW_RULES_H
 #define SURICATA_RTE_FLOW_RULES_H
 
+void RuleStorageFree(RuleStorage *rule_storage);
 int ConfigLoadRTEFlowRules(ConfNode *if_root, ConfNode *if_default, const char *filter_type,
         RuleStorage *rule_storage);
-int CreateRules(char *port_name, int port_id, RuleStorage *rule_storage);
+int CreateRules(char *port_name, int port_id, RuleStorage *rule_storage, char *driver_name);
 
 #endif /* SURICATA_RTE_FLOW_RULES_H */