From 83763e2540d5dc785a4809661542389683f54214 Mon Sep 17 00:00:00 2001 From: Adam Kiripolsky Date: Tue, 3 Dec 2024 17:30:22 +0100 Subject: [PATCH] dpdk/rte_flow: add rule error log, globalize free --- src/runmode-dpdk.c | 2 ++ src/source-dpdk.c | 18 ++++++++++---- src/source-dpdk.h | 1 + src/util-dpdk-rte-flow.c | 53 ++++++++++++++++++++++++++++------------ src/util-dpdk-rte-flow.h | 3 ++- 5 files changed, 56 insertions(+), 21 deletions(-) diff --git a/src/runmode-dpdk.c b/src/runmode-dpdk.c index 43048fdda43d..3225d4c41c37 100644 --- 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 index 34e2544cb5bf..1e0b5fb8c375 100644 --- 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 index 6b47894b7593..785e668488f4 100644 --- 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 index 647085bba039..d351be72d889 100644 --- 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 index f631886f5df1..377cd4331f8b 100644 --- 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 */