On 2/11/2026 1:52 PM, Anatoly Burakov wrote:
These filters are mashed together even though they almost do not share any
code at all between each other. Separate security filter from ntuple filter
and parse it separately.
While we're at it, we're making checks more stringent (such as checking for
NULL conf), and more type safe.
Signed-off-by: Anatoly Burakov <[email protected]>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 136 ++++++++++++++++++---------
1 file changed, 91 insertions(+), 45 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c
b/drivers/net/intel/ixgbe/ixgbe_flow.c
index c17c6c4bf6..cd8d46019f 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -214,41 +214,6 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
memset(ð_null, 0, sizeof(struct rte_flow_item_eth));
memset(&vlan_null, 0, sizeof(struct rte_flow_item_vlan));
- /**
- * Special case for flow action type RTE_FLOW_ACTION_TYPE_SECURITY
- */
- act = next_no_void_action(actions, NULL);
- if (act->type == RTE_FLOW_ACTION_TYPE_SECURITY) {
- const void *conf = act->conf;
- /* check if the next not void item is END */
- act = next_no_void_action(actions, act);
- if (act->type != RTE_FLOW_ACTION_TYPE_END) {
- memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- /* get the IP pattern*/
- item = next_no_void_pattern(pattern, NULL);
- while (item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
- item->type != RTE_FLOW_ITEM_TYPE_IPV6) {
- if (item->last ||
- item->type == RTE_FLOW_ITEM_TYPE_END) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- item, "IP pattern missing.");
- return -rte_errno;
- }
- item = next_no_void_pattern(pattern, item);
- }
-
- filter->proto = IPPROTO_ESP;
- return ixgbe_crypto_add_ingress_sa_from_flow(conf, item->spec,
- item->type == RTE_FLOW_ITEM_TYPE_IPV6);
- }
-
/* the first not void item can be MAC or IPv4 */
item = next_no_void_pattern(pattern, NULL);
@@ -607,6 +572,81 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
return 0;
}
+static int
+ixgbe_parse_security_filter(struct rte_eth_dev *dev, const struct
rte_flow_attr *attr,
+ const struct rte_flow_item pattern[], const struct
rte_flow_action actions[],
+ struct rte_flow_error *error)
+{
+ struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ const struct rte_flow_action_security *security;
+ const struct rte_flow_item *item;
+ const struct rte_flow_action *act;
+
+ if (hw->mac.type != ixgbe_mac_82599EB &&
+ hw->mac.type != ixgbe_mac_X540 &&
+ hw->mac.type != ixgbe_mac_X550 &&
+ hw->mac.type != ixgbe_mac_X550EM_x &&
+ hw->mac.type != ixgbe_mac_X550EM_a &&
+ hw->mac.type != ixgbe_mac_E610)
+ return -ENOTSUP;
+
+ if (pattern == NULL) {
+ rte_flow_error_set(error,
+ EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_NUM,
+ NULL, "NULL pattern.");
+ return -rte_errno;
+ }
+ if (actions == NULL) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_NUM,
+ NULL, "NULL action.");
+ return -rte_errno;
+ }
+ if (attr == NULL) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR,
+ NULL, "NULL attribute.");
+ return -rte_errno;
+ }
+
+ /* check if next non-void action is security */
+ act = next_no_void_action(actions, NULL);
+ if (act->type != RTE_FLOW_ACTION_TYPE_SECURITY) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ act, "Not supported action.");
+ }
+ security = act->conf;
+ if (security == NULL) {
this looks like a bug fix. In a previous implementation it didn't check
if act->conf is NULL and consequent calling for
ixgbe_crypto_add_ingress_sa_from_flow() immediately dereference it.
Probably it would be great to backport this as a fix.
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, act,
+ "NULL security action config.");
+ }
+ /* check if the next not void item is END */
+ act = next_no_void_action(actions, act);
+ if (act->type != RTE_FLOW_ACTION_TYPE_END) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ act, "Not supported action.");
+ }
+
+ /* get the IP pattern*/
+ item = next_no_void_pattern(pattern, NULL);
+ while (item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
+ item->type != RTE_FLOW_ITEM_TYPE_IPV6) {
+ if (item->last || item->type == RTE_FLOW_ITEM_TYPE_END) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item, "IP pattern missing.");
+ return -rte_errno;
+ }
+ item = next_no_void_pattern(pattern, item);
+ }
+
+ return ixgbe_crypto_add_ingress_sa_from_flow(security->security_session,
did you mean &security->security_session here?
+ item->spec, item->type == RTE_FLOW_ITEM_TYPE_IPV6);
probably need to add check if item->spec is NULL
+}
+
/* a specific function for ixgbe because the flags is specific */
static int
ixgbe_parse_ntuple_filter(struct rte_eth_dev *dev,
<snip>
--
Regards,
Vladimir
