* [PATCH v7 16/27] net/i40e: use common action checks for tunnel
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Bruce Richardson
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common flow action checking parsing infrastructure for checking
flow actions for flow director tunnel filter. As a result, more stringent
checks are performed against parametersm specifically the following:
- reject NULL conf for VF action (instead of attempt at NULL dereference)
- second action was expected to be QUEUE but if it wasn't it was ignored
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/i40e/i40e_flow.c | 104 +++++++++++++----------------
1 file changed, 48 insertions(+), 56 deletions(-)
diff --git a/drivers/net/intel/i40e/i40e_flow.c b/drivers/net/intel/i40e/i40e_flow.c
index 1ca70ff71c..1051c99fba 100644
--- a/drivers/net/intel/i40e/i40e_flow.c
+++ b/drivers/net/intel/i40e/i40e_flow.c
@@ -1107,15 +1107,6 @@ static struct i40e_valid_pattern i40e_supported_patterns[] = {
{ pattern_fdir_ipv6_sctp, i40e_flow_parse_l4_cloud_filter },
};
-#define NEXT_ITEM_OF_ACTION(act, actions, index) \
- do { \
- act = actions + index; \
- while (act->type == RTE_FLOW_ACTION_TYPE_VOID) { \
- index++; \
- act = actions + index; \
- } \
- } while (0)
-
/* Find the first VOID or non-VOID item pointer */
static const struct rte_flow_item *
i40e_find_first_item(const struct rte_flow_item *item, bool is_void)
@@ -2629,61 +2620,62 @@ i40e_flow_parse_tunnel_action(struct rte_eth_dev *dev,
struct i40e_tunnel_filter_conf *filter)
{
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- const struct rte_flow_action *act;
const struct rte_flow_action_queue *act_q;
- const struct rte_flow_action_vf *act_vf;
- uint32_t index = 0;
+ struct ci_flow_actions parsed_actions = {0};
+ struct ci_flow_actions_check_param ac_param = {
+ .allowed_types = (enum rte_flow_action_type[]) {
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_PF,
+ RTE_FLOW_ACTION_TYPE_VF,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .max_actions = 2,
+ };
+ const struct rte_flow_action *first, *second;
+ int ret;
- /* Check if the first non-void action is PF or VF. */
- NEXT_ITEM_OF_ACTION(act, actions, index);
- if (act->type != RTE_FLOW_ACTION_TYPE_PF &&
- act->type != RTE_FLOW_ACTION_TYPE_VF) {
- rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
+ ret = ci_flow_check_actions(actions, &ac_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ first = parsed_actions.actions[0];
+ /* can be NULL */
+ second = parsed_actions.actions[1];
- if (act->type == RTE_FLOW_ACTION_TYPE_VF) {
- act_vf = act->conf;
- filter->vf_id = act_vf->id;
+ /* first action must be PF or VF */
+ if (first->type == RTE_FLOW_ACTION_TYPE_VF) {
+ const struct rte_flow_action_vf *vf = first->conf;
+ if (vf->id >= pf->vf_num) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, first,
+ "Invalid VF ID for tunnel filter");
+ return -rte_errno;
+ }
+ filter->vf_id = vf->id;
filter->is_to_vf = 1;
- if (filter->vf_id >= pf->vf_num) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Invalid VF ID for tunnel filter");
- return -rte_errno;
- }
+ } else if (first->type != RTE_FLOW_ACTION_TYPE_PF) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, first,
+ "Unsupported action");
}
- /* Check if the next non-void item is QUEUE */
- index++;
- NEXT_ITEM_OF_ACTION(act, actions, index);
- if (act->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
- act_q = act->conf;
- filter->queue_id = act_q->index;
- if ((!filter->is_to_vf) &&
- (filter->queue_id >= pf->dev_data->nb_rx_queues)) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Invalid queue ID for tunnel filter");
- return -rte_errno;
- } else if (filter->is_to_vf &&
- (filter->queue_id >= pf->vf_nb_qps)) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Invalid queue ID for tunnel filter");
- return -rte_errno;
- }
- }
+ /* check if second action is QUEUE */
+ if (second == NULL)
+ return 0;
- /* Check if the next non-void item is END */
- index++;
- NEXT_ITEM_OF_ACTION(act, actions, index);
- if (act->type != RTE_FLOW_ACTION_TYPE_END) {
- rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
+ act_q = second->conf;
+ /* check queue ID for PF flow */
+ if (!filter->is_to_vf && act_q->index >= pf->dev_data->nb_rx_queues) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, act_q,
+ "Invalid queue ID for tunnel filter");
+ }
+ /* check queue ID for VF flow */
+ if (filter->is_to_vf && act_q->index >= pf->vf_nb_qps) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, act_q,
+ "Invalid queue ID for tunnel filter");
}
+ filter->queue_id = act_q->index;
return 0;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v7 15/27] net/i40e: use common action checks for FDIR
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Bruce Richardson
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common flow action checking parsing infrastructure for checking
flow actions for flow director filter.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/i40e/i40e_flow.c | 139 ++++++++++++++++-------------
1 file changed, 76 insertions(+), 63 deletions(-)
diff --git a/drivers/net/intel/i40e/i40e_flow.c b/drivers/net/intel/i40e/i40e_flow.c
index 57d83479f1..1ca70ff71c 100644
--- a/drivers/net/intel/i40e/i40e_flow.c
+++ b/drivers/net/intel/i40e/i40e_flow.c
@@ -2486,28 +2486,49 @@ i40e_flow_parse_fdir_action(struct rte_eth_dev *dev,
struct i40e_fdir_filter_conf *filter)
{
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- const struct rte_flow_action *act;
- const struct rte_flow_action_queue *act_q;
- const struct rte_flow_action_mark *mark_spec = NULL;
- uint32_t index = 0;
+ struct ci_flow_actions parsed_actions = {0};
+ struct ci_flow_actions_check_param ac_param = {
+ .allowed_types = (enum rte_flow_action_type[]) {
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_DROP,
+ RTE_FLOW_ACTION_TYPE_PASSTHRU,
+ RTE_FLOW_ACTION_TYPE_MARK,
+ RTE_FLOW_ACTION_TYPE_FLAG,
+ RTE_FLOW_ACTION_TYPE_RSS,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .max_actions = 2,
+ };
+ const struct rte_flow_action *first, *second;
+ int ret;
- /* Check if the first non-void action is QUEUE or DROP or PASSTHRU. */
- NEXT_ITEM_OF_ACTION(act, actions, index);
- switch (act->type) {
+ ret = ci_flow_check_actions(actions, &ac_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ first = parsed_actions.actions[0];
+ /* can be NULL */
+ second = parsed_actions.actions[1];
+
+ switch (first->type) {
case RTE_FLOW_ACTION_TYPE_QUEUE:
- act_q = act->conf;
+ {
+ const struct rte_flow_action_queue *act_q = first->conf;
+ /* check against PF constraints */
+ if (!filter->input.flow_ext.is_vf && act_q->index >= pf->dev_data->nb_rx_queues) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, first,
+ "Invalid queue ID for FDIR");
+ }
+ /* check against VF constraints */
+ if (filter->input.flow_ext.is_vf && act_q->index >= pf->vf_nb_qps) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, first,
+ "Invalid queue ID for FDIR");
+ }
filter->action.rx_queue = act_q->index;
- if ((!filter->input.flow_ext.is_vf &&
- filter->action.rx_queue >= pf->dev_data->nb_rx_queues) ||
- (filter->input.flow_ext.is_vf &&
- filter->action.rx_queue >= pf->vf_nb_qps)) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION, act,
- "Invalid queue ID for FDIR.");
- return -rte_errno;
- }
filter->action.behavior = I40E_FDIR_ACCEPT;
break;
+ }
case RTE_FLOW_ACTION_TYPE_DROP:
filter->action.behavior = I40E_FDIR_REJECT;
break;
@@ -2515,69 +2536,61 @@ i40e_flow_parse_fdir_action(struct rte_eth_dev *dev,
filter->action.behavior = I40E_FDIR_PASSTHRU;
break;
case RTE_FLOW_ACTION_TYPE_MARK:
+ {
+ const struct rte_flow_action_mark *act_m = first->conf;
filter->action.behavior = I40E_FDIR_PASSTHRU;
- mark_spec = act->conf;
filter->action.report_status = I40E_FDIR_REPORT_ID;
- filter->soft_id = mark_spec->id;
- break;
+ filter->soft_id = act_m->id;
+ break;
+ }
default:
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION, act,
- "Invalid action.");
- return -rte_errno;
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, first,
+ "Invalid first action for FDIR");
}
- /* Check if the next non-void item is MARK or FLAG or END. */
- index++;
- NEXT_ITEM_OF_ACTION(act, actions, index);
- switch (act->type) {
+ /* do we have another? */
+ if (second == NULL)
+ return 0;
+
+ switch (second->type) {
case RTE_FLOW_ACTION_TYPE_MARK:
- if (mark_spec) {
- /* Double MARK actions requested */
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION, act,
- "Invalid action.");
- return -rte_errno;
+ {
+ const struct rte_flow_action_mark *act_m = second->conf;
+ /* only one mark action can be specified */
+ if (first->type == RTE_FLOW_ACTION_TYPE_MARK) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, second,
+ "Invalid second action for FDIR");
}
- mark_spec = act->conf;
filter->action.report_status = I40E_FDIR_REPORT_ID;
- filter->soft_id = mark_spec->id;
+ filter->soft_id = act_m->id;
break;
+ }
case RTE_FLOW_ACTION_TYPE_FLAG:
- if (mark_spec) {
- /* MARK + FLAG not supported */
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION, act,
- "Invalid action.");
- return -rte_errno;
+ {
+ /* mark + flag is unsupported */
+ if (first->type == RTE_FLOW_ACTION_TYPE_MARK) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, second,
+ "Invalid second action for FDIR");
}
filter->action.report_status = I40E_FDIR_NO_REPORT_STATUS;
break;
+ }
case RTE_FLOW_ACTION_TYPE_RSS:
- if (filter->action.behavior != I40E_FDIR_PASSTHRU) {
- /* RSS filter won't be next if FDIR did not pass thru */
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION, act,
- "Invalid action.");
- return -rte_errno;
+ /* RSS filter only can be after passthru or mark */
+ if (first->type != RTE_FLOW_ACTION_TYPE_PASSTHRU &&
+ first->type != RTE_FLOW_ACTION_TYPE_MARK) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, second,
+ "Invalid second action for FDIR");
}
break;
- case RTE_FLOW_ACTION_TYPE_END:
- return 0;
default:
- rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Invalid action.");
- return -rte_errno;
- }
-
- /* Check if the next non-void item is END */
- index++;
- NEXT_ITEM_OF_ACTION(act, actions, index);
- if (act->type != RTE_FLOW_ACTION_TYPE_END) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Invalid action.");
- return -rte_errno;
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, second,
+ "Invalid second action for FDIR");
}
return 0;
--
2.47.3
^ permalink raw reply related
* [PATCH v7 14/27] net/i40e: use common action checks for ethertype
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Bruce Richardson
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common flow action checking parsing infrastructure for checking
flow actions for ethertype filter.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/i40e/i40e_flow.c | 56 +++++++++++++-----------------
1 file changed, 24 insertions(+), 32 deletions(-)
diff --git a/drivers/net/intel/i40e/i40e_flow.c b/drivers/net/intel/i40e/i40e_flow.c
index 9f1fb7bbc7..57d83479f1 100644
--- a/drivers/net/intel/i40e/i40e_flow.c
+++ b/drivers/net/intel/i40e/i40e_flow.c
@@ -1448,43 +1448,35 @@ i40e_flow_parse_ethertype_action(struct rte_eth_dev *dev,
struct rte_flow_error *error,
struct rte_eth_ethertype_filter *filter)
{
- struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- const struct rte_flow_action *act;
- const struct rte_flow_action_queue *act_q;
- uint32_t index = 0;
+ struct ci_flow_actions parsed_actions = {0};
+ struct ci_flow_actions_check_param ac_param = {
+ .allowed_types = (enum rte_flow_action_type[]) {
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_DROP,
+ RTE_FLOW_ACTION_TYPE_END,
+ },
+ .max_actions = 1,
+ };
+ const struct rte_flow_action *action;
+ int ret;
- /* Check if the first non-void action is QUEUE or DROP. */
- NEXT_ITEM_OF_ACTION(act, actions, index);
- if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
- act->type != RTE_FLOW_ACTION_TYPE_DROP) {
- rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
+ ret = ci_flow_check_actions(actions, &ac_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ action = parsed_actions.actions[0];
- if (act->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
- act_q = act->conf;
+ if (action->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
+ const struct rte_flow_action_queue *act_q = action->conf;
+ /* check queue index */
+ if (act_q->index >= dev->data->nb_rx_queues) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, action,
+ "Invalid queue index");
+ }
filter->queue = act_q->index;
- if (filter->queue >= pf->dev_data->nb_rx_queues) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Invalid queue ID for"
- " ethertype_filter.");
- return -rte_errno;
- }
- } else {
+ } else if (action->type == RTE_FLOW_ACTION_TYPE_DROP) {
filter->flags |= RTE_ETHTYPE_FLAGS_DROP;
}
-
- /* Check if the next non-void item is END */
- index++;
- NEXT_ITEM_OF_ACTION(act, actions, index);
- if (act->type != RTE_FLOW_ACTION_TYPE_END) {
- rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
return 0;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v7 13/27] net/i40e: refactor RSS flow parameter checks
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Bruce Richardson
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Currently, the hash parser parameter checks are somewhat confusing as they
have multiple mutually exclusive code paths and requirements, and it's
difficult to reason about them because RSS flow parsing is interspersed
with validation checks.
To address that, refactor hash engine error checking to perform almost all
validation at the beginning, with only happy paths being implemented in
actual parsing functions.
Some parameter combinations that were previously ignored (and perhaps
produced a warning) are now explicitly rejected:
- if no pattern is specified, RSS types are rejected
- for queue lists and regions, RSS key is rejected
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/i40e/i40e_flow.c | 13 +-
drivers/net/intel/i40e/i40e_hash.c | 438 ++++++++++++++++++-----------
drivers/net/intel/i40e/i40e_hash.h | 2 +-
3 files changed, 275 insertions(+), 178 deletions(-)
diff --git a/drivers/net/intel/i40e/i40e_flow.c b/drivers/net/intel/i40e/i40e_flow.c
index 36e816758c..9f1fb7bbc7 100644
--- a/drivers/net/intel/i40e/i40e_flow.c
+++ b/drivers/net/intel/i40e/i40e_flow.c
@@ -3816,6 +3816,7 @@ i40e_flow_check(struct rte_eth_dev *dev,
if (ret) {
return ret;
}
+ /* action and pattern validation will happen in each respective engine */
if (!pattern) {
rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_NUM,
@@ -3830,13 +3831,11 @@ i40e_flow_check(struct rte_eth_dev *dev,
return -rte_errno;
}
- /* Get the non-void item of action */
- while ((actions + i)->type == RTE_FLOW_ACTION_TYPE_VOID)
- i++;
-
- if ((actions + i)->type == RTE_FLOW_ACTION_TYPE_RSS) {
- filter_ctx->type = RTE_ETH_FILTER_HASH;
- return i40e_hash_parse(dev, pattern, actions + i, &filter_ctx->rss_conf, error);
+ /* try parsing as RSS */
+ filter_ctx->type = RTE_ETH_FILTER_HASH;
+ ret = i40e_hash_parse(dev, pattern, actions, &filter_ctx->rss_conf, error);
+ if (!ret) {
+ return ret;
}
i = 0;
diff --git a/drivers/net/intel/i40e/i40e_hash.c b/drivers/net/intel/i40e/i40e_hash.c
index 5756ebf255..3c1302469c 100644
--- a/drivers/net/intel/i40e/i40e_hash.c
+++ b/drivers/net/intel/i40e/i40e_hash.c
@@ -16,6 +16,8 @@
#include "i40e_ethdev.h"
#include "i40e_hash.h"
+#include "../common/flow_check.h"
+
#ifndef BIT
#define BIT(n) (1UL << (n))
#endif
@@ -925,12 +927,7 @@ i40e_hash_parse_key(const struct rte_flow_action_rss *rss_act,
{
const uint8_t *key = rss_act->key;
- if (!key || rss_act->key_len != sizeof(rss_conf->key)) {
- if (rss_act->key_len != sizeof(rss_conf->key))
- PMD_DRV_LOG(WARNING,
- "RSS key length invalid, must be %u bytes, now set key to default",
- (uint32_t)sizeof(rss_conf->key));
-
+ if (key == NULL) {
memcpy(rss_conf->key, i40e_rss_key_default, sizeof(rss_conf->key));
} else {
memcpy(rss_conf->key, key, sizeof(rss_conf->key));
@@ -941,45 +938,29 @@ i40e_hash_parse_key(const struct rte_flow_action_rss *rss_act,
}
static int
-i40e_hash_parse_queues(const struct rte_eth_dev *dev,
- const struct rte_flow_action_rss *rss_act,
- struct i40e_rte_flow_rss_conf *rss_conf,
- struct rte_flow_error *error)
+i40e_hash_parse_pattern_act(const struct rte_eth_dev *dev,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action_rss *rss_act,
+ struct i40e_rte_flow_rss_conf *rss_conf,
+ struct rte_flow_error *error)
{
- struct i40e_pf *pf;
- struct i40e_hw *hw;
- uint16_t i;
- uint16_t max_queue;
-
- hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- if (!rss_act->queue_num ||
- rss_act->queue_num > hw->func_caps.rss_table_size)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL, "Invalid RSS queue number");
+ rss_conf->symmetric_enable = rss_act->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ;
if (rss_act->key_len)
- PMD_DRV_LOG(WARNING,
- "RSS key is ignored when queues specified");
+ i40e_hash_parse_key(rss_act, rss_conf);
- pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- if (pf->dev_data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_VMDQ_FLAG)
- max_queue = i40e_pf_calc_configured_queues_num(pf);
- else
- max_queue = pf->dev_data->nb_rx_queues;
+ rss_conf->conf.func = rss_act->func;
+ rss_conf->conf.types = rss_act->types;
+ rss_conf->inset = i40e_hash_get_inset(rss_act->types, rss_conf->symmetric_enable);
- max_queue = RTE_MIN(max_queue, I40E_MAX_Q_PER_TC);
-
- for (i = 0; i < rss_act->queue_num; i++) {
- if (rss_act->queue[i] >= max_queue)
- break;
- }
-
- if (i < rss_act->queue_num)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL, "Invalid RSS queues");
+ return i40e_hash_get_pattern_pctypes(dev, pattern, rss_act,
+ rss_conf, error);
+}
+static int
+i40e_hash_parse_queues(const struct rte_flow_action_rss *rss_act,
+ struct i40e_rte_flow_rss_conf *rss_conf)
+{
memcpy(rss_conf->queue, rss_act->queue,
rss_act->queue_num * sizeof(rss_conf->queue[0]));
rss_conf->conf.queue = rss_conf->queue;
@@ -988,113 +969,38 @@ i40e_hash_parse_queues(const struct rte_eth_dev *dev,
}
static int
-i40e_hash_parse_queue_region(const struct rte_eth_dev *dev,
- const struct rte_flow_item pattern[],
+i40e_hash_parse_queue_region(const struct rte_flow_item pattern[],
const struct rte_flow_action_rss *rss_act,
struct i40e_rte_flow_rss_conf *rss_conf,
struct rte_flow_error *error)
{
- struct i40e_pf *pf;
const struct rte_flow_item_vlan *vlan_spec, *vlan_mask;
- uint64_t hash_queues;
- uint32_t i;
-
- if (pattern[1].type != RTE_FLOW_ITEM_TYPE_END)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ITEM_NUM,
- &pattern[1],
- "Pattern not supported.");
vlan_spec = pattern->spec;
vlan_mask = pattern->mask;
- if (!vlan_spec || !vlan_mask ||
- (rte_be_to_cpu_16(vlan_mask->hdr.vlan_tci) >> 13) != 7)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM, pattern,
- "Pattern error.");
- if (!rss_act->queue)
+ /* VLAN must have spec and mask */
+ if (vlan_spec == NULL || vlan_mask == NULL) {
return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL, "Queues not specified");
-
- if (rss_act->key_len)
- PMD_DRV_LOG(WARNING,
- "RSS key is ignored when configure queue region");
+ RTE_FLOW_ERROR_TYPE_ITEM, &pattern[0],
+ "VLAN pattern spec and mask required");
+ }
+ /* for mask, VLAN/TCI must be masked appropriately */
+ if ((rte_be_to_cpu_16(vlan_mask->hdr.vlan_tci) >> 13) != 0x7) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM, &pattern[0],
+ "VLAN pattern mask invalid");
+ }
/* Use a 64 bit variable to represent all queues in a region. */
RTE_BUILD_BUG_ON(I40E_MAX_Q_PER_TC > 64);
- if (!rss_act->queue_num ||
- !rte_is_power_of_2(rss_act->queue_num) ||
- rss_act->queue_num + rss_act->queue[0] > I40E_MAX_Q_PER_TC)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL, "Queue number error");
-
- for (i = 1; i < rss_act->queue_num; i++) {
- if (rss_act->queue[i - 1] + 1 != rss_act->queue[i])
- break;
- }
-
- if (i < rss_act->queue_num)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL,
- "Queues must be incremented continuously");
-
- /* Map all queues to bits of uint64_t */
- hash_queues = (BIT_ULL(rss_act->queue[0] + rss_act->queue_num) - 1) &
- ~(BIT_ULL(rss_act->queue[0]) - 1);
-
- pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- if (hash_queues & ~pf->hash_enabled_queues)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL, "Some queues are not in LUT");
-
rss_conf->region_queue_num = (uint8_t)rss_act->queue_num;
rss_conf->region_queue_start = rss_act->queue[0];
rss_conf->region_priority = rte_be_to_cpu_16(vlan_spec->hdr.vlan_tci) >> 13;
return 0;
}
-static int
-i40e_hash_parse_global_conf(const struct rte_eth_dev *dev,
- const struct rte_flow_item pattern[],
- const struct rte_flow_action_rss *rss_act,
- struct i40e_rte_flow_rss_conf *rss_conf,
- struct rte_flow_error *error)
-{
- if (rss_act->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL,
- "Symmetric function should be set with pattern types");
-
- rss_conf->conf.func = rss_act->func;
-
- if (rss_act->types)
- PMD_DRV_LOG(WARNING,
- "RSS types are ignored when no pattern specified");
-
- if (pattern[0].type == RTE_FLOW_ITEM_TYPE_VLAN)
- return i40e_hash_parse_queue_region(dev, pattern, rss_act,
- rss_conf, error);
-
- if (rss_act->queue)
- return i40e_hash_parse_queues(dev, rss_act, rss_conf, error);
-
- if (rss_act->key_len) {
- i40e_hash_parse_key(rss_act, rss_conf);
- return 0;
- }
-
- if (rss_act->func == RTE_ETH_HASH_FUNCTION_DEFAULT)
- PMD_DRV_LOG(WARNING, "Nothing change");
- return 0;
-}
-
static bool
i40e_hash_validate_rss_types(uint64_t rss_types)
{
@@ -1124,83 +1030,275 @@ i40e_hash_validate_rss_types(uint64_t rss_types)
}
static int
-i40e_hash_parse_pattern_act(const struct rte_eth_dev *dev,
- const struct rte_flow_item pattern[],
- const struct rte_flow_action_rss *rss_act,
- struct i40e_rte_flow_rss_conf *rss_conf,
- struct rte_flow_error *error)
+i40e_hash_validate_rss_pattern(const struct ci_flow_actions *actions,
+ const struct ci_flow_actions_check_param *param __rte_unused,
+ struct rte_flow_error *error)
{
- if (rss_act->queue)
+ const struct rte_flow_action_rss *rss_act = actions->actions[0]->conf;
+
+ /* queue list is not supported */
+ if (rss_act->queue_num != 0) {
return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL,
- "RSS Queues not supported when pattern specified");
- rss_conf->symmetric_enable = false; /* by default, symmetric is disabled */
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS queues not supported when pattern specified");
+ }
+ /* disallow unsupported hash functions */
switch (rss_act->func) {
case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
- rss_conf->symmetric_enable = true;
- break;
case RTE_ETH_HASH_FUNCTION_DEFAULT:
case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
break;
default:
return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL,
- "RSS hash function not supported "
- "when pattern specified");
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS hash function not supported when pattern specified");
}
if (!i40e_hash_validate_rss_types(rss_act->types))
return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- NULL, "RSS types are invalid");
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+ rss_act, "RSS types are invalid");
- if (rss_act->key_len)
- i40e_hash_parse_key(rss_act, rss_conf);
+ /* check RSS key length if it is specified */
+ if (rss_act->key_len != 0 && rss_act->key_len != I40E_RSS_KEY_LEN) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS key length must be 52 bytes");
+ }
- rss_conf->conf.func = rss_act->func;
- rss_conf->conf.types = rss_act->types;
- rss_conf->inset = i40e_hash_get_inset(rss_act->types, rss_conf->symmetric_enable);
+ return 0;
+}
- return i40e_hash_get_pattern_pctypes(dev, pattern, rss_act,
- rss_conf, error);
+static int
+i40e_hash_validate_rss_common(const struct rte_flow_action_rss *rss_act,
+ struct rte_flow_error *error)
+{
+ /* RSS level is not supported */
+ if (rss_act->level != 0) {
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS level is not supported");
+ }
+
+ /* for empty patterns, symmetric toeplitz is not supported */
+ if (rss_act->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "Symmetric hash function not supported without specific patterns");
+ }
+
+ /* hash types are not supported for global RSS configuration */
+ if (rss_act->types != 0) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS types not supported without a pattern");
+ }
+
+ /* check RSS key length if it is specified */
+ if (rss_act->key_len != 0 && rss_act->key_len != I40E_RSS_KEY_LEN) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS key length must be 52 bytes");
+ }
+
+ return 0;
+}
+
+static int
+i40e_hash_validate_queue_region(const struct ci_flow_actions *actions,
+ const struct ci_flow_actions_check_param *param,
+ struct rte_flow_error *error)
+{
+ const struct rte_flow_action_rss *rss_act = actions->actions[0]->conf;
+ struct i40e_adapter *adapter = I40E_DEV_PRIVATE_TO_ADAPTER(param->driver_ctx);
+ struct i40e_pf *pf = &adapter->pf;
+ uint64_t hash_queues;
+ int ret;
+
+ ret = i40e_hash_validate_rss_common(rss_act, error);
+ if (ret)
+ return ret;
+
+ RTE_BUILD_BUG_ON(sizeof(hash_queues) != sizeof(pf->hash_enabled_queues));
+
+ /* having RSS key is not supported */
+ if (rss_act->key != NULL) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS key not supported");
+ }
+
+ /* queue region must be specified */
+ if (rss_act->queue_num == 0) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS queues missing");
+ }
+
+ /* queue region must be power of two */
+ if (!rte_is_power_of_2(rss_act->queue_num)) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS queue number must be power of two");
+ }
+
+ /* generic checks already filtered out discontiguous/non-unique RSS queues */
+
+ /* queues must not exceed maximum queues per traffic class */
+ if (rss_act->queue[rss_act->queue_num - 1] >= I40E_MAX_Q_PER_TC) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "Invalid RSS queue index");
+ }
+
+ /* queues must be in LUT */
+ hash_queues = (BIT_ULL(rss_act->queue[0] + rss_act->queue_num) - 1) &
+ ~(BIT_ULL(rss_act->queue[0]) - 1);
+
+ if (hash_queues & ~pf->hash_enabled_queues) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+ rss_act, "Some queues are not in LUT");
+ }
+
+ return 0;
+}
+
+static int
+i40e_hash_validate_queue_list(const struct ci_flow_actions *actions,
+ const struct ci_flow_actions_check_param *param,
+ struct rte_flow_error *error)
+{
+ const struct rte_flow_action_rss *rss_act = actions->actions[0]->conf;
+ struct i40e_adapter *adapter = I40E_DEV_PRIVATE_TO_ADAPTER(param->driver_ctx);
+ struct i40e_pf *pf;
+ struct i40e_hw *hw;
+ uint16_t max_queue;
+ bool has_queue, has_key;
+ int ret;
+
+ ret = i40e_hash_validate_rss_common(rss_act, error);
+ if (ret)
+ return ret;
+
+ has_queue = rss_act->queue != NULL;
+ has_key = rss_act->key != NULL;
+
+ /* if we have queues, we must not have key */
+ if (has_queue && has_key) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS key for queue region is not supported");
+ }
+
+ /* if there are no queues, no further checks needed */
+ if (!has_queue)
+ return 0;
+
+ /* check queue number limits */
+ hw = &adapter->hw;
+ if (rss_act->queue_num > hw->func_caps.rss_table_size) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+ rss_act, "Too many RSS queues");
+ }
+
+ pf = &adapter->pf;
+ if (pf->dev_data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_VMDQ_FLAG)
+ max_queue = i40e_pf_calc_configured_queues_num(pf);
+ else
+ max_queue = pf->dev_data->nb_rx_queues;
+
+ max_queue = RTE_MIN(max_queue, I40E_MAX_Q_PER_TC);
+
+ /* we know RSS queues are contiguous so we only need to check last queue */
+ if (rss_act->queue[rss_act->queue_num - 1] >= max_queue) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "Invalid RSS queue");
+ }
+
+ return 0;
}
int
-i40e_hash_parse(const struct rte_eth_dev *dev,
+i40e_hash_parse(struct rte_eth_dev *dev,
const struct rte_flow_item pattern[],
const struct rte_flow_action actions[],
struct i40e_rte_flow_rss_conf *rss_conf,
struct rte_flow_error *error)
{
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ac_param = {
+ .allowed_types = (enum rte_flow_action_type[]) {
+ RTE_FLOW_ACTION_TYPE_RSS,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .max_actions = 1,
+ .driver_ctx = dev->data->dev_private,
+ .rss_queues_contig = true,
+ /* each pattern type will add specific check function */
+ };
const struct rte_flow_action_rss *rss_act;
+ int ret;
- if (actions[1].type != RTE_FLOW_ACTION_TYPE_END)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- &actions[1],
- "Only support one action for RSS.");
-
- rss_act = (const struct rte_flow_action_rss *)actions[0].conf;
- if (rss_act->level)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION_CONF,
- actions,
- "RSS level is not supported");
+ /*
+ * We have two possible paths: global RSS configuration, and an RSS pattern action.
+ *
+ * For global patterns, we act on two types of flows:
+ * - Empty pattern ([END])
+ * - VLAN pattern ([VLAN] -> [END])
+ *
+ * Everything else is handled by pattern action parser.
+ */
+ bool is_empty, is_vlan;
while (pattern->type == RTE_FLOW_ITEM_TYPE_VOID)
pattern++;
- if (pattern[0].type == RTE_FLOW_ITEM_TYPE_END ||
- pattern[0].type == RTE_FLOW_ITEM_TYPE_VLAN)
- return i40e_hash_parse_global_conf(dev, pattern, rss_act,
- rss_conf, error);
+ is_empty = pattern[0].type == RTE_FLOW_ITEM_TYPE_END;
+ is_vlan = pattern[0].type == RTE_FLOW_ITEM_TYPE_VLAN &&
+ pattern[1].type == RTE_FLOW_ITEM_TYPE_END;
- return i40e_hash_parse_pattern_act(dev, pattern, rss_act,
- rss_conf, error);
+ /* VLAN path */
+ if (is_vlan) {
+ ac_param.check = i40e_hash_validate_queue_region;
+ ret = ci_flow_check_actions(actions, &ac_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ rss_act = parsed_actions.actions[0]->conf;
+ /* set up RSS functions */
+ rss_conf->conf.func = rss_act->func;
+ return i40e_hash_parse_queue_region(pattern, rss_act, rss_conf, error);
+ }
+ /* Empty pattern path */
+ if (is_empty) {
+ ac_param.check = i40e_hash_validate_queue_list;
+ ret = ci_flow_check_actions(actions, &ac_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ rss_act = parsed_actions.actions[0]->conf;
+ rss_conf->conf.func = rss_act->func;
+ /* if there is a queue list, take that path */
+ if (rss_act->queue != NULL) {
+ return i40e_hash_parse_queues(rss_act, rss_conf);
+ }
+ /* otherwise just parse RSS key */
+ if (rss_act->key != NULL) {
+ i40e_hash_parse_key(rss_act, rss_conf);
+ }
+ return 0;
+ }
+ ac_param.check = i40e_hash_validate_rss_pattern;
+ ret = ci_flow_check_actions(actions, &ac_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ rss_act = parsed_actions.actions[0]->conf;
+
+ /* pattern case */
+ return i40e_hash_parse_pattern_act(dev, pattern, rss_act, rss_conf, error);
}
static void
diff --git a/drivers/net/intel/i40e/i40e_hash.h b/drivers/net/intel/i40e/i40e_hash.h
index 2513d84565..99df4bccd0 100644
--- a/drivers/net/intel/i40e/i40e_hash.h
+++ b/drivers/net/intel/i40e/i40e_hash.h
@@ -13,7 +13,7 @@
extern "C" {
#endif
-int i40e_hash_parse(const struct rte_eth_dev *dev,
+int i40e_hash_parse(struct rte_eth_dev *dev,
const struct rte_flow_item pattern[],
const struct rte_flow_action actions[],
struct i40e_rte_flow_rss_conf *rss_conf,
--
2.47.3
^ permalink raw reply related
* [PATCH v7 12/27] net/i40e: use common flow attribute checks
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Bruce Richardson
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
There is no need to call the same attribute checks in multiple places
when there are no cases where we do otherwise. Therefore, remove all the
attribute check calls from all filter call paths and instead check the
attributes once at the very beginning of flow validation, and use common
flow attribute checks instead of driver-local ones.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/i40e/i40e_ethdev.h | 1 -
drivers/net/intel/i40e/i40e_flow.c | 121 +++------------------------
2 files changed, 10 insertions(+), 112 deletions(-)
diff --git a/drivers/net/intel/i40e/i40e_ethdev.h b/drivers/net/intel/i40e/i40e_ethdev.h
index c39a5a8802..420669af75 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.h
+++ b/drivers/net/intel/i40e/i40e_ethdev.h
@@ -1315,7 +1315,6 @@ struct i40e_filter_ctx {
};
typedef int (*parse_filter_t)(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,
diff --git a/drivers/net/intel/i40e/i40e_flow.c b/drivers/net/intel/i40e/i40e_flow.c
index c8eaa2b2e5..36e816758c 100644
--- a/drivers/net/intel/i40e/i40e_flow.c
+++ b/drivers/net/intel/i40e/i40e_flow.c
@@ -27,6 +27,8 @@
#include "i40e_ethdev.h"
#include "i40e_hash.h"
+#include "../common/flow_check.h"
+
#define I40E_IPV6_TC_MASK (0xFF << I40E_FDIR_IPv6_TC_OFFSET)
#define I40E_IPV6_FRAG_HEADER 44
#define I40E_TENANT_ARRAY_NUM 3
@@ -76,40 +78,32 @@ static int i40e_flow_parse_tunnel_action(struct rte_eth_dev *dev,
const struct rte_flow_action *actions,
struct rte_flow_error *error,
struct i40e_tunnel_filter_conf *filter);
-static int i40e_flow_parse_attr(const struct rte_flow_attr *attr,
- struct rte_flow_error *error);
static int i40e_flow_parse_ethertype_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 i40e_filter_ctx *filter);
static int i40e_flow_parse_fdir_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 i40e_filter_ctx *filter);
static int i40e_flow_parse_vxlan_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 i40e_filter_ctx *filter);
static int i40e_flow_parse_nvgre_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 i40e_filter_ctx *filter);
static int i40e_flow_parse_mpls_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 i40e_filter_ctx *filter);
static int i40e_flow_parse_gtp_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,
@@ -123,7 +117,6 @@ static int i40e_flow_flush_ethertype_filter(struct i40e_pf *pf);
static int i40e_flow_flush_tunnel_filter(struct i40e_pf *pf);
static int
i40e_flow_parse_qinq_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,
@@ -135,7 +128,6 @@ i40e_flow_parse_qinq_pattern(struct rte_eth_dev *dev,
struct i40e_tunnel_filter_conf *filter);
static int i40e_flow_parse_l4_cloud_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,
@@ -1214,54 +1206,6 @@ i40e_find_parse_filter_func(struct rte_flow_item *pattern, uint32_t *idx)
return parse_filter;
}
-/* Parse attributes */
-static int
-i40e_flow_parse_attr(const struct rte_flow_attr *attr,
- struct rte_flow_error *error)
-{
- /* Must be input direction */
- if (!attr->ingress) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- attr, "Only support ingress.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->egress) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- attr, "Not support egress.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->transfer) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- attr, "Not support transfer.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->priority) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- attr, "Not support priority.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->group) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
- attr, "Not support group.");
- return -rte_errno;
- }
-
- return 0;
-}
-
#define I40E_FLOW_DUMP_CHUNK_BYTES 32
static const char *
@@ -1546,7 +1490,6 @@ i40e_flow_parse_ethertype_action(struct rte_eth_dev *dev,
static int
i40e_flow_parse_ethertype_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,
@@ -1565,10 +1508,6 @@ i40e_flow_parse_ethertype_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_ETHERTYPE;
return ret;
@@ -2654,7 +2593,6 @@ i40e_flow_parse_fdir_action(struct rte_eth_dev *dev,
static int
i40e_flow_parse_fdir_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,
@@ -2671,10 +2609,6 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_FDIR;
return 0;
@@ -2939,7 +2873,6 @@ i40e_flow_parse_l4_pattern(const struct rte_flow_item *pattern,
static int
i40e_flow_parse_l4_cloud_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,
@@ -2956,10 +2889,6 @@ i40e_flow_parse_l4_cloud_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_TUNNEL;
return ret;
@@ -3190,7 +3119,6 @@ i40e_flow_parse_vxlan_pattern(__rte_unused struct rte_eth_dev *dev,
static int
i40e_flow_parse_vxlan_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,
@@ -3208,10 +3136,6 @@ i40e_flow_parse_vxlan_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_TUNNEL;
return ret;
@@ -3441,7 +3365,6 @@ i40e_flow_parse_nvgre_pattern(__rte_unused struct rte_eth_dev *dev,
static int
i40e_flow_parse_nvgre_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,
@@ -3459,10 +3382,6 @@ i40e_flow_parse_nvgre_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_TUNNEL;
return ret;
@@ -3597,7 +3516,6 @@ i40e_flow_parse_mpls_pattern(__rte_unused struct rte_eth_dev *dev,
static int
i40e_flow_parse_mpls_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,
@@ -3615,10 +3533,6 @@ i40e_flow_parse_mpls_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_TUNNEL;
return ret;
@@ -3749,7 +3663,6 @@ i40e_flow_parse_gtp_pattern(struct rte_eth_dev *dev,
static int
i40e_flow_parse_gtp_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,
@@ -3767,10 +3680,6 @@ i40e_flow_parse_gtp_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_TUNNEL;
return ret;
@@ -3866,7 +3775,6 @@ i40e_flow_parse_qinq_pattern(__rte_unused struct rte_eth_dev *dev,
static int
i40e_flow_parse_qinq_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,
@@ -3884,10 +3792,6 @@ i40e_flow_parse_qinq_filter(struct rte_eth_dev *dev,
if (ret)
return ret;
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter->type = RTE_ETH_FILTER_TUNNEL;
return ret;
@@ -3906,7 +3810,12 @@ i40e_flow_check(struct rte_eth_dev *dev,
uint32_t item_num = 0; /* non-void item number of pattern*/
uint32_t i = 0;
bool flag = false;
- int ret = I40E_NOT_SUPPORTED;
+ int ret;
+
+ ret = ci_flow_check_attr(attr, NULL, error);
+ if (ret) {
+ return ret;
+ }
if (!pattern) {
rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_NUM,
@@ -3921,22 +3830,11 @@ i40e_flow_check(struct rte_eth_dev *dev,
return -rte_errno;
}
- if (!attr) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR,
- NULL, "NULL attribute.");
- return -rte_errno;
- }
-
/* Get the non-void item of action */
while ((actions + i)->type == RTE_FLOW_ACTION_TYPE_VOID)
i++;
if ((actions + i)->type == RTE_FLOW_ACTION_TYPE_RSS) {
- ret = i40e_flow_parse_attr(attr, error);
- if (ret)
- return ret;
-
filter_ctx->type = RTE_ETH_FILTER_HASH;
return i40e_hash_parse(dev, pattern, actions + i, &filter_ctx->rss_conf, error);
}
@@ -3961,6 +3859,7 @@ i40e_flow_check(struct rte_eth_dev *dev,
i40e_pattern_skip_void_item(items, pattern);
i = 0;
+ ret = I40E_NOT_SUPPORTED;
do {
parse_filter = i40e_find_parse_filter_func(items, &i);
if (!parse_filter && !flag) {
@@ -3973,7 +3872,7 @@ i40e_flow_check(struct rte_eth_dev *dev,
}
if (parse_filter)
- ret = parse_filter(dev, attr, items, actions, error, filter_ctx);
+ ret = parse_filter(dev, items, actions, error, filter_ctx);
flag = true;
} while ((ret < 0) && (i < RTE_DIM(i40e_supported_patterns)));
--
2.47.3
^ permalink raw reply related
* [PATCH v7 11/27] net/ixgbe: use common checks in RSS filter
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Vladimir Medvedkin
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common attr and action parsing infrastructure in RSS filter. As a
result, some checks have become more stringent, in particular:
- the group attribute is now explicitly rejected instead of being ignored
- the priority attribute was previously allowed but rejected values bigger
than 0xFFFF despite not using priority anywhere - it is now rejected
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 196 +++++++++++----------------
1 file changed, 79 insertions(+), 117 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index 81dd1cb5fd..19d7f93d93 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -112,20 +112,6 @@ const struct rte_flow_item *next_no_void_pattern(
}
}
-static inline
-const struct rte_flow_action *next_no_void_action(
- const struct rte_flow_action actions[],
- const struct rte_flow_action *cur)
-{
- const struct rte_flow_action *next =
- cur ? cur + 1 : &actions[0];
- while (1) {
- if (next->type != RTE_FLOW_ACTION_TYPE_VOID)
- return next;
- next++;
- }
-}
-
/*
* All ixgbe engines mostly check the same stuff, so use a common check.
*/
@@ -2680,6 +2666,56 @@ ixgbe_fdir_flow_program(struct rte_eth_dev *dev,
return 0;
}
+/* Flow actions check specific to RSS filter */
+static int
+ixgbe_flow_actions_check_rss(const struct ci_flow_actions *parsed_actions,
+ const struct ci_flow_actions_check_param *param,
+ struct rte_flow_error *error)
+{
+ const struct rte_flow_action *action = parsed_actions->actions[0];
+ const struct rte_flow_action_rss *rss_act = action->conf;
+ struct rte_eth_dev_data *dev_data = param->driver_ctx;
+ const size_t rss_key_len = sizeof(((struct ixgbe_rte_flow_rss_conf *)0)->key);
+ size_t q_idx, q;
+
+ /* check if queue list is not empty */
+ if (rss_act->queue_num == 0) {
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS queue list is empty");
+ }
+
+ /* check if each RSS queue is valid */
+ for (q_idx = 0; q_idx < rss_act->queue_num; q_idx++) {
+ q = rss_act->queue[q_idx];
+ if (q >= dev_data->nb_rx_queues) {
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "Invalid RSS queue specified");
+ }
+ }
+
+ /* only support default hash function */
+ if (rss_act->func != RTE_ETH_HASH_FUNCTION_DEFAULT) {
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "Non-default RSS hash functions are not supported");
+ }
+ /* levels aren't supported */
+ if (rss_act->level) {
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "A nonzero RSS encapsulation level is not supported");
+ }
+ /* check key length */
+ if (rss_act->key_len != 0 && rss_act->key_len != rss_key_len) {
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss_act,
+ "RSS key must be exactly 40 bytes long");
+ }
+ return 0;
+}
+
static int
ixgbe_parse_rss_filter(struct rte_eth_dev *dev,
const struct rte_flow_attr *attr,
@@ -2687,109 +2723,35 @@ ixgbe_parse_rss_filter(struct rte_eth_dev *dev,
struct ixgbe_rte_flow_rss_conf *rss_conf,
struct rte_flow_error *error)
{
- const struct rte_flow_action *act;
- const struct rte_flow_action_rss *rss;
- uint16_t n;
-
- /**
- * rss only supports forwarding,
- * check if the first not void action is RSS.
- */
- act = next_no_void_action(actions, NULL);
- if (act->type != RTE_FLOW_ACTION_TYPE_RSS) {
- memset(rss_conf, 0, sizeof(struct ixgbe_rte_flow_rss_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- rss = (const struct rte_flow_action_rss *)act->conf;
-
- if (!rss || !rss->queue_num) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act,
- "no valid queues");
- return -rte_errno;
- }
-
- for (n = 0; n < rss->queue_num; n++) {
- if (rss->queue[n] >= dev->data->nb_rx_queues) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act,
- "queue id > max number of queues");
- return -rte_errno;
- }
- }
-
- if (rss->func != RTE_ETH_HASH_FUNCTION_DEFAULT)
- return rte_flow_error_set
- (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, act,
- "non-default RSS hash functions are not supported");
- if (rss->level)
- return rte_flow_error_set
- (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, act,
- "a nonzero RSS encapsulation level is not supported");
- if (rss->key_len && rss->key_len != RTE_DIM(rss_conf->key))
- return rte_flow_error_set
- (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, act,
- "RSS hash key must be exactly 40 bytes");
- if (rss->queue_num > RTE_DIM(rss_conf->queue))
- return rte_flow_error_set
- (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, act,
- "too many queues for RSS context");
- if (ixgbe_rss_conf_init(rss_conf, rss))
- return rte_flow_error_set
- (error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION, act,
- "RSS context initialization failure");
-
- /* 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(rss_conf, 0, sizeof(struct ixgbe_rte_flow_rss_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- /* parse attr */
- /* must be input direction */
- if (!attr->ingress) {
- memset(rss_conf, 0, sizeof(struct ixgbe_rte_flow_rss_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- attr, "Only support ingress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->egress) {
- memset(rss_conf, 0, sizeof(struct ixgbe_rte_flow_rss_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- attr, "Not support egress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->transfer) {
- memset(rss_conf, 0, sizeof(struct ixgbe_rte_flow_rss_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- attr, "No support for transfer.");
- return -rte_errno;
- }
-
- if (attr->priority > 0xFFFF) {
- memset(rss_conf, 0, sizeof(struct ixgbe_rte_flow_rss_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- attr, "Error priority.");
- return -rte_errno;
- }
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ap_param = {
+ .allowed_types = (const enum rte_flow_action_type[]){
+ /* only rss allowed here */
+ RTE_FLOW_ACTION_TYPE_RSS,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .driver_ctx = dev->data,
+ .check = ixgbe_flow_actions_check_rss,
+ .max_actions = 1,
+ };
+ int ret;
+ const struct rte_flow_action *action;
+
+ /* validate attributes */
+ ret = ci_flow_check_attr(attr, NULL, error);
+ if (ret)
+ return ret;
+
+ /* parse requested actions */
+ ret = ci_flow_check_actions(actions, &ap_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ action = parsed_actions.actions[0];
+
+ if (ixgbe_rss_conf_init(rss_conf, action->conf))
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+ "RSS context initialization failure");
return 0;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v7 10/27] net/ixgbe: use common checks in FDIR filters
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Vladimir Medvedkin
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common attr and action parsing infrastructure in flow director
filters (both tunnel and normal). As a result, some checks have become
more stringent, in particular group attribute is now explicitly rejected
instead of being ignored.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 319 ++++++++++++---------------
1 file changed, 138 insertions(+), 181 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index cb7dde5a44..81dd1cb5fd 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -1195,111 +1195,6 @@ ixgbe_parse_l2_tn_filter(struct rte_eth_dev *dev,
return ret;
}
-/* Parse to get the attr and action info of flow director rule. */
-static int
-ixgbe_parse_fdir_act_attr(const struct rte_flow_attr *attr,
- const struct rte_flow_action actions[],
- struct ixgbe_fdir_rule *rule,
- struct rte_flow_error *error)
-{
- const struct rte_flow_action *act;
- const struct rte_flow_action_queue *act_q;
- const struct rte_flow_action_mark *mark;
-
- /* parse attr */
- /* must be input direction */
- if (!attr->ingress) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- attr, "Only support ingress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->egress) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- attr, "Not support egress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->transfer) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- attr, "No support for transfer.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->priority) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- attr, "Not support priority.");
- return -rte_errno;
- }
-
- /* check if the first not void action is QUEUE or DROP. */
- act = next_no_void_action(actions, NULL);
- if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
- act->type != RTE_FLOW_ACTION_TYPE_DROP) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- if (act->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
- act_q = (const struct rte_flow_action_queue *)act->conf;
- rule->queue = act_q->index;
- } else { /* drop */
- /* signature mode does not support drop action. */
- if (rule->mode == RTE_FDIR_MODE_SIGNATURE) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
- rule->fdirflags = IXGBE_FDIRCMD_DROP;
- }
-
- /* check if the next not void item is MARK */
- act = next_no_void_action(actions, act);
- if ((act->type != RTE_FLOW_ACTION_TYPE_MARK) &&
- (act->type != RTE_FLOW_ACTION_TYPE_END)) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- rule->soft_id = 0;
-
- if (act->type == RTE_FLOW_ACTION_TYPE_MARK) {
- mark = (const struct rte_flow_action_mark *)act->conf;
- rule->soft_id = mark->id;
- act = next_no_void_action(actions, act);
- }
-
- /* check if the next not void item is END */
- if (act->type != RTE_FLOW_ACTION_TYPE_END) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- return 0;
-}
-
/* search next no void pattern and skip fuzzy */
static inline
const struct rte_flow_item *next_no_fuzzy_pattern(
@@ -1406,9 +1301,8 @@ static inline uint8_t signature_match(const struct rte_flow_item pattern[])
*/
static int
ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
- const struct rte_flow_attr *attr,
const struct rte_flow_item pattern[],
- const struct rte_flow_action actions[],
+ const struct ci_flow_actions *parsed_actions,
struct ixgbe_fdir_rule *rule,
struct rte_flow_error *error)
{
@@ -1429,30 +1323,13 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
const struct rte_flow_item_vlan *vlan_mask;
const struct rte_flow_item_raw *raw_mask;
const struct rte_flow_item_raw *raw_spec;
+ const struct rte_flow_action *fwd_action, *aux_action;
+ struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
uint8_t j;
- struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-
- if (!pattern) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM_NUM,
- NULL, "NULL pattern.");
- return -rte_errno;
- }
-
- if (!actions) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_NUM,
- NULL, "NULL action.");
- return -rte_errno;
- }
-
- if (!attr) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR,
- NULL, "NULL attribute.");
- return -rte_errno;
- }
+ fwd_action = parsed_actions->actions[0];
+ /* can be NULL */
+ aux_action = parsed_actions->actions[1];
/**
* Some fields may not be provided. Set spec to 0 and mask to default
@@ -1466,29 +1343,51 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
rule->mask.dst_port_mask = 0;
rule->mask.src_port_mask = 0;
- /**
- * The first not void item should be
- * MAC or IPv4 or TCP or UDP or SCTP.
- */
- item = next_no_fuzzy_pattern(pattern, NULL);
- if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
- item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
- item->type != RTE_FLOW_ITEM_TYPE_IPV6 &&
- item->type != RTE_FLOW_ITEM_TYPE_TCP &&
- item->type != RTE_FLOW_ITEM_TYPE_UDP &&
- item->type != RTE_FLOW_ITEM_TYPE_SCTP) {
- memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- item, "Not supported by fdir filter");
- return -rte_errno;
- }
-
+ /* check if this is a signature match */
if (signature_match(pattern))
rule->mode = RTE_FDIR_MODE_SIGNATURE;
else
rule->mode = RTE_FDIR_MODE_PERFECT;
+ /* set up action */
+ if (fwd_action->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
+ const struct rte_flow_action_queue *q_act = fwd_action->conf;
+ rule->queue = q_act->index;
+ } else {
+ /* signature mode does not support drop action. */
+ if (rule->mode == RTE_FDIR_MODE_SIGNATURE) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, fwd_action,
+ "Signature mode does not support drop action.");
+ return -rte_errno;
+ }
+ rule->fdirflags = IXGBE_FDIRCMD_DROP;
+ }
+
+ /* set up mark action */
+ if (aux_action != NULL && aux_action->type == RTE_FLOW_ACTION_TYPE_MARK) {
+ const struct rte_flow_action_mark *m_act = aux_action->conf;
+ rule->soft_id = m_act->id;
+ }
+
+ /**
+ * The first not void item should be
+ * MAC or IPv4 or TCP or UDP or SCTP.
+ */
+ item = next_no_fuzzy_pattern(pattern, NULL);
+ if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
+ item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
+ item->type != RTE_FLOW_ITEM_TYPE_IPV6 &&
+ item->type != RTE_FLOW_ITEM_TYPE_TCP &&
+ item->type != RTE_FLOW_ITEM_TYPE_UDP &&
+ item->type != RTE_FLOW_ITEM_TYPE_SCTP) {
+ memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item, "Not supported by fdir filter");
+ return -rte_errno;
+ }
+
/*Not supported last point for range*/
if (item->last) {
rte_flow_error_set(error, EINVAL,
@@ -2083,7 +1982,7 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
rule->mask.l4_proto_match =
(rule->ixgbe_fdir.formatted.flow_type & IXGBE_ATR_L4TYPE_MASK) != 0;
- return ixgbe_parse_fdir_act_attr(attr, actions, rule, error);
+ return 0;
}
#define NVGRE_PROTOCOL 0x6558
@@ -2126,9 +2025,8 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
* item->last should be NULL.
*/
static int
-ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
- const struct rte_flow_item pattern[],
- const struct rte_flow_action actions[],
+ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_item pattern[],
+ const struct ci_flow_actions *parsed_actions,
struct ixgbe_fdir_rule *rule,
struct rte_flow_error *error)
{
@@ -2141,28 +2039,12 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
const struct rte_flow_item_eth *eth_mask;
const struct rte_flow_item_vlan *vlan_spec;
const struct rte_flow_item_vlan *vlan_mask;
+ const struct rte_flow_action *fwd_action, *aux_action;
uint32_t j;
- if (!pattern) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM_NUM,
- NULL, "NULL pattern.");
- return -rte_errno;
- }
-
- if (!actions) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_NUM,
- NULL, "NULL action.");
- return -rte_errno;
- }
-
- if (!attr) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR,
- NULL, "NULL attribute.");
- return -rte_errno;
- }
+ fwd_action = parsed_actions->actions[0];
+ /* can be NULL */
+ aux_action = parsed_actions->actions[1];
/**
* Some fields may not be provided. Set spec to 0 and mask to default
@@ -2172,6 +2054,20 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
memset(&rule->mask, 0xFF, sizeof(struct ixgbe_hw_fdir_mask));
rule->mask.vlan_tci_mask = 0;
+ /* set up queue/drop action */
+ if (fwd_action->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
+ const struct rte_flow_action_queue *q_act = fwd_action->conf;
+ rule->queue = q_act->index;
+ } else {
+ rule->fdirflags = IXGBE_FDIRCMD_DROP;
+ }
+
+ /* set up mark action */
+ if (aux_action != NULL && aux_action->type == RTE_FLOW_ACTION_TYPE_MARK) {
+ const struct rte_flow_action_mark *mark = aux_action->conf;
+ rule->soft_id = mark->id;
+ }
+
/**
* The first not void item should be
* MAC or IPv4 or IPv6 or UDP or VxLAN.
@@ -2572,7 +2468,43 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
* Do nothing.
*/
- return ixgbe_parse_fdir_act_attr(attr, actions, rule, error);
+ return 0;
+}
+
+/*
+ * Check flow director actions
+ */
+static int
+ixgbe_fdir_actions_check(const struct ci_flow_actions *parsed_actions,
+ const struct ci_flow_actions_check_param *param __rte_unused,
+ struct rte_flow_error *error)
+{
+ const enum rte_flow_action_type fwd_actions[] = {
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_DROP,
+ RTE_FLOW_ACTION_TYPE_END
+ };
+ const struct rte_flow_action *action;
+
+ /* do the generic checks first */
+ int ret = ixgbe_flow_actions_check(parsed_actions, param, error);
+ if (ret)
+ return ret;
+
+ /* first action must be a forwarding action */
+ action = parsed_actions->actions[0];
+ if (!ci_flow_action_type_in_list(action->type, fwd_actions)) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ action, "First action must be QUEUE or DROP");
+ }
+
+ /* second action, if specified, must not be a forwarding action */
+ action = parsed_actions->actions[1];
+ if (action != NULL && ci_flow_action_type_in_list(action->type, fwd_actions)) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ action, "Conflicting actions");
+ }
+ return 0;
}
static int
@@ -2585,22 +2517,47 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
{
int ret;
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ struct ixgbe_adapter *adapter = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_PRIVATE_TO_FDIR_CONF(adapter);
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ap_param = {
+ .allowed_types = (const enum rte_flow_action_type[]){
+ /* queue/mark/drop allowed here */
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_DROP,
+ RTE_FLOW_ACTION_TYPE_MARK,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .driver_ctx = dev->data,
+ .check = ixgbe_fdir_actions_check
+ };
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)
+ 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;
- ret = ixgbe_parse_fdir_filter_normal(dev, attr, pattern,
- actions, rule, error);
+ /* validate attributes */
+ ret = ci_flow_check_attr(attr, NULL, error);
+ if (ret)
+ return ret;
+
+ /* parse requested actions */
+ ret = ci_flow_check_actions(actions, &ap_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+
+ fdir_conf->drop_queue = IXGBE_FDIR_DROP_QUEUE;
+
+ ret = ixgbe_parse_fdir_filter_normal(dev, pattern, &parsed_actions, rule, error);
if (!ret)
return 0;
- return ixgbe_parse_fdir_filter_tunnel(attr, pattern,
- actions, rule, error);
+ return ixgbe_parse_fdir_filter_tunnel(pattern, &parsed_actions,
+ rule, error);
}
static int
--
2.47.3
^ permalink raw reply related
* [PATCH v7 09/27] net/ixgbe: use common checks in security filter
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Vladimir Medvedkin
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common attr and action parsing infrastructure in security filter.
As a result, some checks have become more stringent. In particular, group
attribute is now explicitly rejected instead of being ignored.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 62 ++++++++++------------------
1 file changed, 22 insertions(+), 40 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index 9b879f718b..cb7dde5a44 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -539,7 +539,16 @@ ixgbe_parse_security_filter(struct rte_eth_dev *dev, const struct rte_flow_attr
const struct rte_flow_action_security *security;
struct rte_security_session *session;
const struct rte_flow_item *item;
- const struct rte_flow_action *act;
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ap_param = {
+ .allowed_types = (const enum rte_flow_action_type[]){
+ /* only security is allowed here */
+ RTE_FLOW_ACTION_TYPE_SECURITY,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .max_actions = 1,
+ };
+ const struct rte_flow_action *action;
struct ip_spec spec;
int ret;
@@ -551,45 +560,18 @@ ixgbe_parse_security_filter(struct rte_eth_dev *dev, const struct rte_flow_attr
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;
- }
+ /* validate attributes */
+ ret = ci_flow_check_attr(attr, NULL, error);
+ if (ret)
+ return ret;
- /* 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) {
- 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.");
- }
+ /* parse requested actions */
+ ret = ci_flow_check_actions(actions, &ap_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+
+ action = parsed_actions.actions[0];
+ security = action->conf;
/* get the IP pattern*/
item = next_no_void_pattern(pattern, NULL);
@@ -629,7 +611,7 @@ ixgbe_parse_security_filter(struct rte_eth_dev *dev, const struct rte_flow_attr
ret = ixgbe_crypto_add_ingress_sa_from_flow(session, &spec);
if (ret) {
rte_flow_error_set(error, -ret,
- RTE_FLOW_ERROR_TYPE_ACTION, act,
+ RTE_FLOW_ERROR_TYPE_ACTION, action,
"Failed to add security session.");
return -rte_errno;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v7 08/27] net/ixgbe: use common checks in ntuple filter
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Vladimir Medvedkin
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common attr and action parsing infrastructure in ntuple filter.
As a result, some checks have become more stringent, in particular the
group attribute is now explicitly rejected instead of being ignored.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 133 ++++++++-------------------
1 file changed, 36 insertions(+), 97 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index 1d93717ce8..9b879f718b 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -210,12 +210,11 @@ ixgbe_flow_actions_check(const struct ci_flow_actions *actions,
static int
cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
const struct rte_flow_item pattern[],
- const struct rte_flow_action actions[],
+ const struct rte_flow_action_queue *q_act,
struct rte_eth_ntuple_filter *filter,
struct rte_flow_error *error)
{
const struct rte_flow_item *item;
- const struct rte_flow_action *act;
const struct rte_flow_item_ipv4 *ipv4_spec;
const struct rte_flow_item_ipv4 *ipv4_mask;
const struct rte_flow_item_tcp *tcp_spec;
@@ -231,24 +230,11 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
struct rte_flow_item_eth eth_null;
struct rte_flow_item_vlan vlan_null;
- if (!pattern) {
- rte_flow_error_set(error,
- EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_NUM,
- NULL, "NULL pattern.");
- return -rte_errno;
- }
-
- if (!actions) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_NUM,
- NULL, "NULL action.");
- return -rte_errno;
- }
- if (!attr) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR,
- NULL, "NULL attribute.");
- return -rte_errno;
+ /* Priority must be 16-bit */
+ if (attr->priority > UINT16_MAX) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, attr,
+ "Priority must be 16-bit");
}
memset(ð_null, 0, sizeof(struct rte_flow_item_eth));
@@ -535,70 +521,11 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
action:
- /**
- * n-tuple only supports forwarding,
- * check if the first not void action is QUEUE.
- */
- act = next_no_void_action(actions, NULL);
- if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE) {
- memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- item, "Not supported action.");
- return -rte_errno;
- }
- filter->queue =
- ((const struct rte_flow_action_queue *)act->conf)->index;
+ filter->queue = q_act->index;
- /* 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;
- }
-
- /* parse attr */
- /* must be input direction */
- if (!attr->ingress) {
- memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- attr, "Only support ingress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->egress) {
- memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- attr, "Not support egress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->transfer) {
- memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- attr, "No support for transfer.");
- return -rte_errno;
- }
-
- if (attr->priority > 0xFFFF) {
- memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- attr, "Error priority.");
- return -rte_errno;
- }
filter->priority = (uint16_t)attr->priority;
- if (attr->priority < IXGBE_MIN_N_TUPLE_PRIO ||
- attr->priority > IXGBE_MAX_N_TUPLE_PRIO)
- filter->priority = 1;
+ if (attr->priority < IXGBE_MIN_N_TUPLE_PRIO || attr->priority > IXGBE_MAX_N_TUPLE_PRIO)
+ filter->priority = 1;
return 0;
}
@@ -718,15 +645,40 @@ ixgbe_parse_ntuple_filter(struct rte_eth_dev *dev,
struct rte_eth_ntuple_filter *filter,
struct rte_flow_error *error)
{
- int ret;
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ struct ci_flow_attr_check_param attr_param = {
+ .allow_priority = true,
+ };
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ap_param = {
+ .allowed_types = (const enum rte_flow_action_type[]){
+ /* only queue is allowed here */
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .driver_ctx = dev->data,
+ .check = ixgbe_flow_actions_check,
+ .max_actions = 1,
+ };
+ const struct rte_flow_action *action;
+ int ret;
if (hw->mac.type != ixgbe_mac_82599EB &&
hw->mac.type != ixgbe_mac_X540)
return -ENOTSUP;
- ret = cons_parse_ntuple_filter(attr, pattern, actions, filter, error);
+ /* validate attributes */
+ ret = ci_flow_check_attr(attr, &attr_param, error);
+ if (ret)
+ return ret;
+ /* parse requested actions */
+ ret = ci_flow_check_actions(actions, &ap_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+ action = parsed_actions.actions[0];
+
+ ret = cons_parse_ntuple_filter(attr, pattern, action->conf, filter, error);
if (ret)
return ret;
@@ -739,19 +691,6 @@ ixgbe_parse_ntuple_filter(struct rte_eth_dev *dev,
return -rte_errno;
}
- /* Ixgbe doesn't support many priorities. */
- if (filter->priority < IXGBE_MIN_N_TUPLE_PRIO ||
- filter->priority > IXGBE_MAX_N_TUPLE_PRIO) {
- memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- NULL, "Priority not supported by ntuple filter");
- return -rte_errno;
- }
-
- if (filter->queue >= dev->data->nb_rx_queues)
- return -rte_errno;
-
/* fixed value for ixgbe */
filter->flags = RTE_5TUPLE_FLAGS;
return 0;
--
2.47.3
^ permalink raw reply related
* [PATCH v7 07/27] net/ixgbe: use common checks in L2 tunnel filter
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Vladimir Medvedkin
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common attr and action parsing infrastructure in L2 tunnel filter.
Some checks have become more stringent as a result, in particular, group
attribute is now explicitly rejected instead of being ignored.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 149 +++++++++------------------
1 file changed, 48 insertions(+), 101 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index fe4ef84dda..1d93717ce8 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -136,6 +136,7 @@ ixgbe_flow_actions_check(const struct ci_flow_actions *actions,
{
const struct rte_flow_action *action;
struct rte_eth_dev_data *dev_data = param->driver_ctx;
+ struct ixgbe_adapter *ad = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev_data->dev_private);
size_t idx;
for (idx = 0; idx < actions->count; idx++) {
@@ -153,6 +154,17 @@ ixgbe_flow_actions_check(const struct ci_flow_actions *actions,
}
break;
}
+ case RTE_FLOW_ACTION_TYPE_VF:
+ {
+ const struct rte_flow_action_vf *vf = action->conf;
+ if (vf->id >= ad->max_vfs) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ action,
+ "VF id out of range");
+ }
+ break;
+ }
default:
/* no specific validation */
break;
@@ -882,12 +894,6 @@ ixgbe_parse_ethertype_filter(struct rte_eth_dev *dev, const struct rte_flow_attr
if (ret)
return ret;
- /* only one action is supported */
- if (parsed_actions.count > 1) {
- return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
- parsed_actions.actions[1],
- "Only one action can be specified at a time");
- }
action = parsed_actions.actions[0];
ret = cons_parse_ethertype_filter(pattern, action, filter, error);
@@ -1133,40 +1139,16 @@ ixgbe_parse_syn_filter(struct rte_eth_dev *dev, const struct rte_flow_attr *attr
*/
static int
cons_parse_l2_tn_filter(struct rte_eth_dev *dev,
- const struct rte_flow_attr *attr,
const struct rte_flow_item pattern[],
- const struct rte_flow_action actions[],
+ const struct rte_flow_action *action,
struct ixgbe_l2_tunnel_conf *filter,
struct rte_flow_error *error)
{
const struct rte_flow_item *item;
const struct rte_flow_item_e_tag *e_tag_spec;
const struct rte_flow_item_e_tag *e_tag_mask;
- const struct rte_flow_action *act;
- const struct rte_flow_action_vf *act_vf;
struct ixgbe_adapter *ad = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
- if (!pattern) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM_NUM,
- NULL, "NULL pattern.");
- return -rte_errno;
- }
-
- if (!actions) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_NUM,
- NULL, "NULL action.");
- return -rte_errno;
- }
-
- if (!attr) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR,
- NULL, "NULL attribute.");
- return -rte_errno;
- }
-
/* The first not void item should be e-tag. */
item = next_no_void_pattern(pattern, NULL);
if (item->type != RTE_FLOW_ITEM_TYPE_E_TAG) {
@@ -1224,71 +1206,13 @@ cons_parse_l2_tn_filter(struct rte_eth_dev *dev,
return -rte_errno;
}
- /* parse attr */
- /* must be input direction */
- if (!attr->ingress) {
- memset(filter, 0, sizeof(struct ixgbe_l2_tunnel_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- attr, "Only support ingress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->egress) {
- memset(filter, 0, sizeof(struct ixgbe_l2_tunnel_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- attr, "Not support egress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->transfer) {
- memset(filter, 0, sizeof(struct ixgbe_l2_tunnel_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- attr, "No support for transfer.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->priority) {
- memset(filter, 0, sizeof(struct ixgbe_l2_tunnel_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- attr, "Not support priority.");
- return -rte_errno;
- }
-
- /* check if the first not void action is VF or PF. */
- act = next_no_void_action(actions, NULL);
- if (act->type != RTE_FLOW_ACTION_TYPE_VF &&
- act->type != RTE_FLOW_ACTION_TYPE_PF) {
- memset(filter, 0, sizeof(struct ixgbe_l2_tunnel_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- if (act->type == RTE_FLOW_ACTION_TYPE_VF) {
- act_vf = (const struct rte_flow_action_vf *)act->conf;
+ if (action->type == RTE_FLOW_ACTION_TYPE_VF) {
+ const struct rte_flow_action_vf *act_vf = action->conf;
filter->pool = act_vf->id;
} else {
filter->pool = ad->max_vfs;
}
- /* 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 ixgbe_l2_tunnel_conf));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
return 0;
}
@@ -1300,29 +1224,52 @@ ixgbe_parse_l2_tn_filter(struct rte_eth_dev *dev,
struct ixgbe_l2_tunnel_conf *l2_tn_filter,
struct rte_flow_error *error)
{
+ struct rte_eth_dev_data *dev_data = dev->data;
+ struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev_data->dev_private);
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ap_param = {
+ .allowed_types = (const enum rte_flow_action_type[]){
+ /* only vf/pf is allowed here */
+ RTE_FLOW_ACTION_TYPE_VF,
+ RTE_FLOW_ACTION_TYPE_PF,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .driver_ctx = dev_data,
+ .check = ixgbe_flow_actions_check,
+ .max_actions = 1,
+ };
int ret = 0;
- struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- struct ixgbe_adapter *ad = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
- uint16_t vf_num;
-
- ret = cons_parse_l2_tn_filter(dev, attr, pattern,
- actions, l2_tn_filter, error);
+ const struct rte_flow_action *action;
if (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) {
- memset(l2_tn_filter, 0, sizeof(struct ixgbe_l2_tunnel_conf));
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ITEM,
NULL, "Not supported by L2 tunnel filter");
return -rte_errno;
}
- vf_num = ad->max_vfs;
+ /* validate attributes */
+ ret = ci_flow_check_attr(attr, NULL, error);
+ if (ret)
+ return ret;
- if (l2_tn_filter->pool > vf_num)
- return -rte_errno;
+ /* parse requested actions */
+ ret = ci_flow_check_actions(actions, &ap_param, &parsed_actions, error);
+ if (ret)
+ return ret;
+
+ /* only one action is supported */
+ if (parsed_actions.count > 1) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ parsed_actions.actions[1],
+ "Only one action can be specified at a time");
+ }
+ action = parsed_actions.actions[0];
+
+ ret = cons_parse_l2_tn_filter(dev, pattern, action, l2_tn_filter, error);
return ret;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v7 06/27] net/ixgbe: use common checks in syn filter
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Vladimir Medvedkin
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common attr and action parsing infrastructure in syn filter. Some
checks have been rearranged or become more stringent due to using common
infrastructure. In particular, group attr was ignored previously but is
now explicitly rejected.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 130 +++++++--------------------
1 file changed, 32 insertions(+), 98 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index 5816b35f55..fe4ef84dda 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -935,38 +935,13 @@ ixgbe_parse_ethertype_filter(struct rte_eth_dev *dev, const struct rte_flow_attr
* item->last should be NULL.
*/
static int
-cons_parse_syn_filter(const struct rte_flow_attr *attr,
- const struct rte_flow_item pattern[],
- const struct rte_flow_action actions[],
- struct rte_eth_syn_filter *filter,
- struct rte_flow_error *error)
+cons_parse_syn_filter(const struct rte_flow_attr *attr, const struct rte_flow_item pattern[],
+ const struct rte_flow_action_queue *q_act, struct rte_eth_syn_filter *filter,
+ struct rte_flow_error *error)
{
const struct rte_flow_item *item;
- const struct rte_flow_action *act;
const struct rte_flow_item_tcp *tcp_spec;
const struct rte_flow_item_tcp *tcp_mask;
- const struct rte_flow_action_queue *act_q;
-
- if (!pattern) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM_NUM,
- NULL, "NULL pattern.");
- return -rte_errno;
- }
-
- if (!actions) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_NUM,
- NULL, "NULL action.");
- return -rte_errno;
- }
-
- if (!attr) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR,
- NULL, "NULL attribute.");
- return -rte_errno;
- }
/* the first not void item should be MAC or IPv4 or IPv6 or TCP */
@@ -1074,63 +1049,7 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
return -rte_errno;
}
- /* check if the first not void action is QUEUE. */
- act = next_no_void_action(actions, NULL);
- if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE) {
- memset(filter, 0, sizeof(struct rte_eth_syn_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- act_q = (const struct rte_flow_action_queue *)act->conf;
- filter->queue = act_q->index;
- if (filter->queue >= IXGBE_MAX_RX_QUEUE_NUM) {
- memset(filter, 0, sizeof(struct rte_eth_syn_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- /* 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_syn_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- /* parse attr */
- /* must be input direction */
- if (!attr->ingress) {
- memset(filter, 0, sizeof(struct rte_eth_syn_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- attr, "Only support ingress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->egress) {
- memset(filter, 0, sizeof(struct rte_eth_syn_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- attr, "Not support egress.");
- return -rte_errno;
- }
-
- /* not supported */
- if (attr->transfer) {
- memset(filter, 0, sizeof(struct rte_eth_syn_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- attr, "No support for transfer.");
- return -rte_errno;
- }
+ filter->queue = q_act->index;
/* Support 2 priorities, the lowest or highest. */
if (!attr->priority) {
@@ -1141,7 +1060,7 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
memset(filter, 0, sizeof(struct rte_eth_syn_filter));
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- attr, "Not support priority.");
+ attr, "Priority can be 0 or 0xFFFFFFFF");
return -rte_errno;
}
@@ -1149,15 +1068,27 @@ cons_parse_syn_filter(const struct rte_flow_attr *attr,
}
static int
-ixgbe_parse_syn_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_eth_syn_filter *filter,
- struct rte_flow_error *error)
+ixgbe_parse_syn_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_eth_syn_filter *filter, struct rte_flow_error *error)
{
int ret;
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ap_param = {
+ .allowed_types = (const enum rte_flow_action_type[]){
+ /* only queue is allowed here */
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .driver_ctx = dev->data,
+ .check = ixgbe_flow_actions_check,
+ .max_actions = 1,
+ };
+ struct ci_flow_attr_check_param attr_param = {
+ .allow_priority = true,
+ };
+ const struct rte_flow_action *action;
if (hw->mac.type != ixgbe_mac_82599EB &&
hw->mac.type != ixgbe_mac_X540 &&
@@ -1167,16 +1098,19 @@ ixgbe_parse_syn_filter(struct rte_eth_dev *dev,
hw->mac.type != ixgbe_mac_E610)
return -ENOTSUP;
- ret = cons_parse_syn_filter(attr, pattern,
- actions, filter, error);
-
- if (filter->queue >= dev->data->nb_rx_queues)
- return -rte_errno;
+ /* validate attributes */
+ ret = ci_flow_check_attr(attr, &attr_param, error);
+ if (ret)
+ return ret;
+ /* parse requested actions */
+ ret = ci_flow_check_actions(actions, &ap_param, &parsed_actions, error);
if (ret)
return ret;
- return 0;
+ action = parsed_actions.actions[0];
+
+ return cons_parse_syn_filter(attr, pattern, action->conf, filter, error);
}
/**
--
2.47.3
^ permalink raw reply related
* [PATCH v7 05/27] net/ixgbe: use common checks in ethertype filter
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Vladimir Medvedkin
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Use the common attr and action parsing infrastructure in ethertype. This
allows us to remove some checks as they are no longer necessary (such as
whether DROP flag was set - if we do not accept DROP actions, we do not set
the DROP flag), as well as make some checks more stringent (such as
rejecting more than one action).
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 190 ++++++++++-----------------
1 file changed, 73 insertions(+), 117 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index 9038aae001..5816b35f55 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -46,6 +46,8 @@
#include "base/ixgbe_phy.h"
#include "rte_pmd_ixgbe.h"
+#include "../common/flow_check.h"
+
#define IXGBE_MIN_N_TUPLE_PRIO 1
#define IXGBE_MAX_N_TUPLE_PRIO 7
@@ -124,6 +126,41 @@ const struct rte_flow_action *next_no_void_action(
}
}
+/*
+ * All ixgbe engines mostly check the same stuff, so use a common check.
+ */
+static int
+ixgbe_flow_actions_check(const struct ci_flow_actions *actions,
+ const struct ci_flow_actions_check_param *param,
+ struct rte_flow_error *error)
+{
+ const struct rte_flow_action *action;
+ struct rte_eth_dev_data *dev_data = param->driver_ctx;
+ size_t idx;
+
+ for (idx = 0; idx < actions->count; idx++) {
+ action = actions->actions[idx];
+
+ switch (action->type) {
+ case RTE_FLOW_ACTION_TYPE_QUEUE:
+ {
+ const struct rte_flow_action_queue *queue = action->conf;
+ if (queue->index >= dev_data->nb_rx_queues) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION,
+ action,
+ "queue index out of range");
+ }
+ break;
+ }
+ default:
+ /* no specific validation */
+ break;
+ }
+ }
+ return 0;
+}
+
/**
* Please be aware there's an assumption for all the parsers.
* rte_flow_item is using big endian, rte_flow_attr and
@@ -725,38 +762,14 @@ ixgbe_parse_ntuple_filter(struct rte_eth_dev *dev,
* item->last should be NULL.
*/
static int
-cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
- const struct rte_flow_item *pattern,
- const struct rte_flow_action *actions,
- struct rte_eth_ethertype_filter *filter,
- struct rte_flow_error *error)
+cons_parse_ethertype_filter(const struct rte_flow_item *pattern,
+ const struct rte_flow_action *action,
+ struct rte_eth_ethertype_filter *filter,
+ struct rte_flow_error *error)
{
const struct rte_flow_item *item;
- const struct rte_flow_action *act;
const struct rte_flow_item_eth *eth_spec;
const struct rte_flow_item_eth *eth_mask;
- const struct rte_flow_action_queue *act_q;
-
- if (!pattern) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM_NUM,
- NULL, "NULL pattern.");
- return -rte_errno;
- }
-
- if (!actions) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION_NUM,
- NULL, "NULL action.");
- return -rte_errno;
- }
-
- if (!attr) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR,
- NULL, "NULL attribute.");
- return -rte_errno;
- }
item = next_no_void_pattern(pattern, NULL);
/* The first non-void item should be MAC. */
@@ -826,87 +839,30 @@ cons_parse_ethertype_filter(const struct rte_flow_attr *attr,
return -rte_errno;
}
- /* Parse action */
-
- act = next_no_void_action(actions, NULL);
- if (act->type != RTE_FLOW_ACTION_TYPE_QUEUE &&
- act->type != RTE_FLOW_ACTION_TYPE_DROP) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- if (act->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
- act_q = (const struct rte_flow_action_queue *)act->conf;
- filter->queue = act_q->index;
- } else {
- filter->flags |= RTE_ETHTYPE_FLAGS_DROP;
- }
-
- /* Check if the next non-void item is END */
- act = next_no_void_action(actions, act);
- if (act->type != RTE_FLOW_ACTION_TYPE_END) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act, "Not supported action.");
- return -rte_errno;
- }
-
- /* Parse attr */
- /* Must be input direction */
- if (!attr->ingress) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- attr, "Only support ingress.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->egress) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- attr, "Not support egress.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->transfer) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- attr, "No support for transfer.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->priority) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- attr, "Not support priority.");
- return -rte_errno;
- }
-
- /* Not supported */
- if (attr->group) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
- attr, "Not support group.");
- return -rte_errno;
- }
+ filter->queue = ((const struct rte_flow_action_queue *)action->conf)->index;
return 0;
}
static int
-ixgbe_parse_ethertype_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_eth_ethertype_filter *filter,
- struct rte_flow_error *error)
+ixgbe_parse_ethertype_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_eth_ethertype_filter *filter, struct rte_flow_error *error)
{
int ret;
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ struct ci_flow_actions parsed_actions;
+ struct ci_flow_actions_check_param ap_param = {
+ .allowed_types = (const enum rte_flow_action_type[]){
+ /* only queue is allowed here */
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_END
+ },
+ .max_actions = 1,
+ .driver_ctx = dev->data,
+ .check = ixgbe_flow_actions_check
+ };
+ const struct rte_flow_action *action;
if (hw->mac.type != ixgbe_mac_82599EB &&
hw->mac.type != ixgbe_mac_X540 &&
@@ -916,19 +872,27 @@ ixgbe_parse_ethertype_filter(struct rte_eth_dev *dev,
hw->mac.type != ixgbe_mac_E610)
return -ENOTSUP;
- ret = cons_parse_ethertype_filter(attr, pattern,
- actions, filter, error);
+ /* validate attributes */
+ ret = ci_flow_check_attr(attr, NULL, error);
+ if (ret)
+ return ret;
+ /* parse requested actions */
+ ret = ci_flow_check_actions(actions, &ap_param, &parsed_actions, error);
if (ret)
return ret;
- if (filter->queue >= dev->data->nb_rx_queues) {
- memset(filter, 0, sizeof(struct rte_eth_ethertype_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- NULL, "queue index much too big");
- return -rte_errno;
+ /* only one action is supported */
+ if (parsed_actions.count > 1) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ parsed_actions.actions[1],
+ "Only one action can be specified at a time");
}
+ action = parsed_actions.actions[0];
+
+ ret = cons_parse_ethertype_filter(pattern, action, filter, error);
+ if (ret)
+ return ret;
if (filter->ether_type == RTE_ETHER_TYPE_IPV4 ||
filter->ether_type == RTE_ETHER_TYPE_IPV6) {
@@ -947,14 +911,6 @@ ixgbe_parse_ethertype_filter(struct rte_eth_dev *dev,
return -rte_errno;
}
- if (filter->flags & RTE_ETHTYPE_FLAGS_DROP) {
- memset(filter, 0, sizeof(struct rte_eth_ethertype_filter));
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- NULL, "drop option is unsupported");
- return -rte_errno;
- }
-
return 0;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v7 04/27] net/intel/common: add common flow attr validation
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Bruce Richardson
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
There are a lot of commonalities between what kinds of flow attr each Intel
driver supports. Add a helper function that will validate attr based on
common requirements and (optional) parameter checks.
Things we check for:
- Rejecting NULL attr (obviously)
- Ingress/egress according to requested params
- Transfer, group, and priority are not allowed unless requested
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/common/flow_check.h | 70 +++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/net/intel/common/flow_check.h b/drivers/net/intel/common/flow_check.h
index 075c86e61e..b143adb8d3 100644
--- a/drivers/net/intel/common/flow_check.h
+++ b/drivers/net/intel/common/flow_check.h
@@ -57,6 +57,7 @@ ci_flow_action_type_in_list(const enum rte_flow_action_type type,
/* Forward declarations */
struct ci_flow_actions;
struct ci_flow_actions_check_param;
+struct ci_flow_attr_check_param;
static inline const char *
ci_flow_action_type_to_str(enum rte_flow_action_type type)
@@ -277,6 +278,75 @@ ci_flow_check_actions(const struct rte_flow_action *actions,
return 0;
}
+/**
+ * Parameter structure for attr check.
+ */
+struct ci_flow_attr_check_param {
+ bool allow_priority; /**< True if priority attribute is allowed. */
+ bool allow_transfer; /**< True if transfer attribute is allowed. */
+ bool allow_group; /**< True if group attribute is allowed. */
+ bool require_egress; /**< True if egress attribute is required. */
+};
+
+/**
+ * Validate rte_flow_attr structure against specified constraints.
+ *
+ * @param attr Pointer to rte_flow_attr structure to validate.
+ * @param attr_param Pointer to ci_flow_attr_check_param structure specifying constraints.
+ * @param error Pointer to rte_flow_error structure for error reporting.
+ *
+ * @return 0 on success, negative errno on failure.
+ */
+static inline int
+ci_flow_check_attr(const struct rte_flow_attr *attr,
+ const struct ci_flow_attr_check_param *attr_param,
+ struct rte_flow_error *error)
+{
+ if (attr == NULL) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR, attr,
+ "NULL attribute");
+ }
+
+ /* a rule can only be ingress or egress, never both or neither. */
+ if (attr_param != NULL && attr_param->require_egress) {
+ if (attr->egress != 1 || attr->ingress != 0) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, attr,
+ "Egress attribute is required");
+ }
+ } else {
+ if (attr->ingress != 1 || attr->egress != 0) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, attr,
+ "Ingress attribute is required");
+ }
+ }
+
+ /* May not be supported */
+ if (attr->transfer && (attr_param == NULL || !attr_param->allow_transfer)) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, attr,
+ "Transfer not supported");
+ }
+
+ /* May not be supported */
+ if (attr->group && (attr_param == NULL || !attr_param->allow_group)) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
+ "Group not supported");
+ }
+
+ /* May not be supported */
+ if (attr->priority && (attr_param == NULL || !attr_param->allow_priority)) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, attr,
+ "Priority not supported");
+ }
+
+ return 0;
+}
+
#ifdef __cplusplus
}
#endif
--
2.47.3
^ permalink raw reply related
* [PATCH v7 03/27] net/intel/common: add common flow action parsing
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev, Bruce Richardson
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Currently, each driver has their own code for action parsing, which results
in a lot of duplication and subtle mismatches in behavior between drivers.
Add common infrastructure, based on the following assumptions:
- All drivers support at most 32 actions at once, but usually far less
- Not every action is supported by all drivers
- We can check a few common things to filter out obviously wrong actions
- Driver performs semantic checks on all valid actions
So, the intention is to reject everything we can reasonably reject at the
outset without knowing anything about the drivers, parametrize what is
trivial to parametrize, and leave the rest for the driver to implement.
While we're at it, also add logging infrastructure for Intel common code,
using the new component name defines that are automatically passed to each
DPDK driver as it is being built.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/common/flow_check.h | 284 ++++++++++++++++++++++++++
drivers/net/intel/common/log.h | 40 ++++
2 files changed, 324 insertions(+)
create mode 100644 drivers/net/intel/common/flow_check.h
create mode 100644 drivers/net/intel/common/log.h
diff --git a/drivers/net/intel/common/flow_check.h b/drivers/net/intel/common/flow_check.h
new file mode 100644
index 0000000000..075c86e61e
--- /dev/null
+++ b/drivers/net/intel/common/flow_check.h
@@ -0,0 +1,284 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2025 Intel Corporation
+ */
+
+#ifndef _COMMON_INTEL_FLOW_CHECK_H_
+#define _COMMON_INTEL_FLOW_CHECK_H_
+
+#include <bus_pci_driver.h>
+#include <ethdev_driver.h>
+
+#include "log.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*
+ * Common attr and action validation code for Intel drivers.
+ */
+
+/**
+ * Maximum number of actions that can be stored in a parsed action list.
+ */
+#define CI_FLOW_PARSED_ACTIONS_MAX 32
+
+/* Actions that are reasonably expected to have a conf structure */
+static const enum rte_flow_action_type need_conf[] = {
+ RTE_FLOW_ACTION_TYPE_QUEUE,
+ RTE_FLOW_ACTION_TYPE_RSS,
+ RTE_FLOW_ACTION_TYPE_VF,
+ RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR,
+ RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT,
+ RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP,
+ RTE_FLOW_ACTION_TYPE_PROG,
+ RTE_FLOW_ACTION_TYPE_COUNT,
+ RTE_FLOW_ACTION_TYPE_MARK,
+ RTE_FLOW_ACTION_TYPE_SECURITY,
+ RTE_FLOW_ACTION_TYPE_END
+};
+
+/**
+ * Is action type in this list of action types?
+ */
+static inline bool
+ci_flow_action_type_in_list(const enum rte_flow_action_type type,
+ const enum rte_flow_action_type list[])
+{
+ size_t i = 0;
+ while (list[i] != RTE_FLOW_ACTION_TYPE_END) {
+ if (type == list[i])
+ return true;
+ i++;
+ }
+ return false;
+}
+
+/* Forward declarations */
+struct ci_flow_actions;
+struct ci_flow_actions_check_param;
+
+static inline const char *
+ci_flow_action_type_to_str(enum rte_flow_action_type type)
+{
+ const char *name = NULL;
+ int ret;
+
+ ret = rte_flow_conv(RTE_FLOW_CONV_OP_ACTION_NAME_PTR,
+ &name, sizeof(name), (const void *)(uintptr_t)type, NULL);
+ if (ret < 0 || name == NULL)
+ return "UNKNOWN";
+
+ return name;
+}
+
+/**
+ * Driver-specific action list validation callback.
+ *
+ * Performs driver-specific validation of action parameter list.
+ * Called after all actions have been parsed and added to the list,
+ * allowing validation based on the complete action set.
+ *
+ * @param actions
+ * The complete list of parsed actions (for context-dependent validation).
+ * @param driver_ctx
+ * Opaque driver context (e.g., adapter/queue configuration).
+ * @param error
+ * Pointer to rte_flow_error for reporting failures.
+ * @return
+ * 0 on success, negative errno on failure.
+ */
+typedef int (*ci_flow_actions_check_fn)(const struct ci_flow_actions *actions,
+ const struct ci_flow_actions_check_param *param,
+ struct rte_flow_error *error);
+
+/**
+ * List of actions that we know we've validated.
+ */
+struct ci_flow_actions {
+ /* Number of actions in the list. */
+ uint8_t count;
+ /* Parsed actions array. */
+ struct rte_flow_action const *actions[CI_FLOW_PARSED_ACTIONS_MAX];
+};
+
+/**
+ * Parameters for action list validation. Any element can be NULL/0 as checks are only performed
+ * against constraints specified.
+ */
+struct ci_flow_actions_check_param {
+ /**
+ * Driver-specific context pointer (e.g., adapter/queue configuration). Can be NULL.
+ */
+ void *driver_ctx;
+ /**
+ * Driver-specific action list validation callback. Can be NULL.
+ */
+ ci_flow_actions_check_fn check;
+ /**
+ * Allowed action types for this parse parameter. Must be terminated with
+ * RTE_FLOW_ACTION_TYPE_END. Can be NULL.
+ */
+ const enum rte_flow_action_type *allowed_types;
+ size_t max_actions; /**< Maximum number of actions allowed. */
+ bool rss_queues_contig; /**< If true, RSS queues must be contiguous. */
+};
+
+static inline int
+__flow_action_check_rss(const struct rte_flow_action_rss *rss,
+ const struct ci_flow_actions_check_param *param,
+ struct rte_flow_error *error)
+{
+ uint32_t qnum, q;
+
+ qnum = rss->queue_num;
+
+ /* either we have both queues and queue number, or we have neither */
+ if ((qnum == 0) != (rss->queue == NULL)) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss,
+ "If queue number is specified, queue array must also be specified");
+ }
+ /* check if queues are monotonic */
+ for (q = 1; q < qnum; q++) {
+ if (rss->queue[q] < rss->queue[q - 1]) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss,
+ "RSS queues must be in ascending order");
+ }
+ /* if user has requested contiguousness, check that as well */
+ if (param == NULL || !param->rss_queues_contig)
+ continue;
+ if (rss->queue[q] != rss->queue[0] + q) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss,
+ "RSS queues must be contiguous");
+ }
+ }
+
+ /* either we have both key and key length, or we have neither */
+ if ((rss->key_len == 0) != (rss->key == NULL)) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss,
+ "If RSS key is specified, key length must also be specified");
+ }
+ return 0;
+}
+
+static inline int
+__flow_action_check_generic(const struct rte_flow_action *action,
+ const struct ci_flow_actions_check_param *param,
+ struct rte_flow_error *error)
+{
+ /* is this action in our allowed list? */
+ if (param != NULL && param->allowed_types != NULL &&
+ !ci_flow_action_type_in_list(action->type, param->allowed_types)) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ action, "Unsupported action");
+ }
+ /* do we need to validate presence of conf? */
+ if (ci_flow_action_type_in_list(action->type, need_conf)) {
+ if (action->conf == NULL) {
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION_CONF, action,
+ "Action requires configuration");
+ }
+ }
+
+ /* type-specific validation */
+ switch (action->type) {
+ case RTE_FLOW_ACTION_TYPE_RSS:
+ {
+ const struct rte_flow_action_rss *rss =
+ (const struct rte_flow_action_rss *)action->conf;
+ int ret;
+
+ ret = __flow_action_check_rss(rss, param, error);
+ if (ret < 0)
+ return ret;
+ break;
+ }
+ default:
+ /* no specific validation */
+ break;
+ }
+
+ return 0;
+}
+
+/**
+ * Validate and parse a list of rte_flow_action into a parsed action list.
+ *
+ * @param actions pointer to array of rte_flow_action, terminated by RTE_FLOW_ACTION_TYPE_END
+ * @param param pointer to ci_flow_actions_check_param structure (can be NULL)
+ * @param parsed_actions pointer to ci_flow_actions structure to store parsed actions
+ * @param error pointer to rte_flow_error structure for error reporting
+ *
+ * @return 0 on success, negative errno on failure.
+ */
+static inline int
+ci_flow_check_actions(const struct rte_flow_action *actions,
+ const struct ci_flow_actions_check_param *param,
+ struct ci_flow_actions *parsed_actions,
+ struct rte_flow_error *error)
+{
+ size_t i = 0;
+ int ret;
+
+ if (actions == NULL) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "Missing actions");
+ }
+
+ /* reset the list */
+ *parsed_actions = (struct ci_flow_actions){0};
+
+ while (actions[i].type != RTE_FLOW_ACTION_TYPE_END) {
+ const struct rte_flow_action *action = &actions[i++];
+
+ /* skip VOID actions */
+ if (action->type == RTE_FLOW_ACTION_TYPE_VOID)
+ continue;
+
+ /* generic validation for actions - this will check against param as well */
+ ret = __flow_action_check_generic(action, param, error);
+ if (ret < 0)
+ return ret;
+
+ /* check against global maximum number of actions */
+ if (parsed_actions->count >= RTE_DIM(parsed_actions->actions)) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ action, "Too many actions");
+ }
+ /* user may have specified a maximum number of actions */
+ if (param != NULL && param->max_actions != 0 &&
+ parsed_actions->count >= param->max_actions) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ action, "Too many actions");
+ }
+ /* add action to the list */
+ CI_DRV_LOG(DEBUG, "Parsed action %u: type=%s", parsed_actions->count,
+ ci_flow_action_type_to_str(action->type));
+ parsed_actions->actions[parsed_actions->count++] = action;
+ }
+
+ /* if we didn't parse anything, valid action list is empty */
+ if (parsed_actions->count == 0) {
+ return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+ NULL, "No valid actions specified");
+ }
+
+ /* now, call into user validation if specified */
+ if (param != NULL && param->check != NULL) {
+ ret = param->check(parsed_actions, param, error);
+ if (ret < 0)
+ return ret;
+ }
+ return 0;
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _COMMON_INTEL_FLOW_CHECK_H_ */
diff --git a/drivers/net/intel/common/log.h b/drivers/net/intel/common/log.h
new file mode 100644
index 0000000000..d99e4d1a37
--- /dev/null
+++ b/drivers/net/intel/common/log.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Intel Corporation
+ */
+
+#ifndef _COMMON_INTEL_LOG_H_
+#define _COMMON_INTEL_LOG_H_
+
+#include <rte_log.h>
+
+/*
+ * Common logging for shared Intel driver code.
+ *
+ * This header must only be included from driver translation units, where the
+ * build system has already defined RTE_COMPONENT_NAME (e.g. "iavf"). It uses
+ * that token to reference the per-driver logtype variable that every Intel
+ * driver registers (e.g. iavf_logtype_driver), so no additional setup is
+ * needed beyond what each driver already provides.
+ *
+ * Usage: CI_DRV_LOG(DEBUG, "format %s", arg);
+ */
+
+#ifndef RTE_COMPONENT_NAME
+/* CI_DRV_LOG is a no-op when included outside a driver build context. */
+#define CI_DRV_LOG(level, fmt, ...) do { } while (0)
+#else /* RTE_COMPONENT_NAME */
+
+/* Resolves to the per-driver logtype variable, e.g. iavf_logtype_driver. */
+#define CI_DRV_LOGTYPE RTE_CONCAT(RTE_COMPONENT_NAME, _logtype_driver)
+
+/* Forward-declare the logtype so shared headers need not include driver headers. */
+extern int CI_DRV_LOGTYPE;
+
+#define CI_DRV_LOG(level, fmt, ...) \
+ rte_log(RTE_LOG_##level, CI_DRV_LOGTYPE, \
+ "PMD_INTEL_COMMON: %s(): " fmt "\n", \
+ __func__, ##__VA_ARGS__)
+
+#endif /* RTE_COMPONENT_NAME */
+
+#endif /* _COMMON_INTEL_LOG_H_ */
--
2.47.3
^ permalink raw reply related
* [PATCH v7 02/27] eal/common: add token concatenation macro
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
Add `RTE_CONCAT()` macro to enable token concatenation with proper macro
expansion.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
lib/eal/include/rte_common.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 71415346cf..2eb21d74d1 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -918,6 +918,10 @@ __extension__ typedef uint64_t RTE_MARKER64[0];
/** Take a macro value and get a string version of it */
#define RTE_STR(x) _RTE_STR(x)
+/** Concatenate two tokens after expanding macros in both. */
+#define _RTE_CONCAT2(a, b) a ## b
+#define RTE_CONCAT(a, b) _RTE_CONCAT2(a, b)
+
/**
* ISO C helpers to modify format strings using variadic macros.
* This is a replacement for the ", ## __VA_ARGS__" GNU extension.
--
2.47.3
^ permalink raw reply related
* [PATCH v7 01/27] build: add build defines for component name and class
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
From: Bruce Richardson <bruce.richardson@intel.com>
When building each component, pass in -D flags to define for it the
component class and name. This allows any files which may be used
multiple times to identify what component is including them.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/meson.build | 2 ++
lib/meson.build | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/meson.build b/drivers/meson.build
index e8873bf627..4d95604ecd 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -264,6 +264,8 @@ foreach subpath:subdirs
enabled_drivers += name
lib_name = '_'.join(['rte', class, name])
+ cflags += '-DRTE_COMPONENT_CLASS=pmd_' + class
+ cflags += '-DRTE_COMPONENT_NAME=' + name
cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
if annotate_locks and cc.get_id() == 'clang'
cflags += '-DRTE_ANNOTATE_LOCKS'
diff --git a/lib/meson.build b/lib/meson.build
index 8f5cfd28a5..af5c160cb8 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -237,6 +237,8 @@ foreach l:libraries
cflags += '-DRTE_USE_FUNCTION_VERSIONING'
endif
cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=lib.' + l
+ cflags += '-DRTE_COMPONENT_CLASS=lib'
+ cflags += '-DRTE_COMPONENT_NAME=' + name
if annotate_locks and cc.get_id() == 'clang'
cflags += '-DRTE_ANNOTATE_LOCKS'
cflags += '-Wthread-safety'
--
2.47.3
^ permalink raw reply related
* [PATCH v7 00/27] Add common flow attr/action parsing infrastructure to Intel PMD's
From: Anatoly Burakov @ 2026-06-04 16:19 UTC (permalink / raw)
To: dev
In-Reply-To: <cover.1770819433.git.anatoly.burakov@intel.com>
This patchset introduces common flow attr/action checking infrastructure to
some Intel PMD's (IXGBE, I40E, IAVF, and ICE). The aim is to reduce code
duplication, simplify implementation of new parsers/verification of existing
ones, and make action/attr handling more consistent across drivers.
v7:
- Fail on empty action list before calling into user callbacks
- Remove empty code
v6:
- Increase max number of actions to 32
- Add missing flow item types from list of types needing conf
- Improved egress/ingress handling
- ixgbe: allow mark and drop combination of actions
- ixgbe: fix set-then-reset when initializing rules for fdir engine
v5:
- Add missing patch for component/class name compile time constant
- Fixed missing RSS queue contiguousness checks
v4:
- First few commits were integrated as part of a different patchset
- Added more logging to common infrastructure
- RSS validation now allows discontiguous queue lists by default
- Fixed some checks being too stringent and failing common validation
v3:
- Rebase on latest next-net-intel
- Added 4 new commits that have to do with not using `rte_eth_dev` in rte_flow
- Converted the remaining commits to use adapter structures everywhere
- Minor fixes in how return values are handled
- Fixed incorrect check in i40e RSS validation
v2:
- Rebase on latest main
- Now depends on series 37585 [1]
[1] https://patches.dpdk.org/project/dpdk/list/?series=37585
Anatoly Burakov (21):
eal/common: add token concatenation macro
net/intel/common: add common flow action parsing
net/intel/common: add common flow attr validation
net/ixgbe: use common checks in ethertype filter
net/ixgbe: use common checks in syn filter
net/ixgbe: use common checks in L2 tunnel filter
net/ixgbe: use common checks in ntuple filter
net/ixgbe: use common checks in security filter
net/ixgbe: use common checks in FDIR filters
net/ixgbe: use common checks in RSS filter
net/i40e: use common flow attribute checks
net/i40e: refactor RSS flow parameter checks
net/i40e: use common action checks for ethertype
net/i40e: use common action checks for FDIR
net/i40e: use common action checks for tunnel
net/iavf: use common flow attribute checks
net/iavf: use common action checks for IPsec
net/iavf: use common action checks for hash
net/iavf: use common action checks for FDIR
net/iavf: use common action checks for fsub
net/iavf: use common action checks for flow query
Bruce Richardson (1):
build: add build defines for component name and class
Vladimir Medvedkin (5):
net/ice: use common flow attribute checks
net/ice: use common action checks for hash
net/ice: use common action checks for FDIR
net/ice: use common action checks for switch
net/ice: use common action checks for ACL
drivers/meson.build | 2 +
drivers/net/intel/common/flow_check.h | 354 ++++++
drivers/net/intel/common/log.h | 40 +
drivers/net/intel/i40e/i40e_ethdev.h | 1 -
drivers/net/intel/i40e/i40e_flow.c | 433 +++-----
drivers/net/intel/i40e/i40e_hash.c | 438 +++++---
drivers/net/intel/i40e/i40e_hash.h | 2 +-
drivers/net/intel/iavf/iavf_fdir.c | 368 +++----
drivers/net/intel/iavf/iavf_fsub.c | 262 ++---
drivers/net/intel/iavf/iavf_generic_flow.c | 107 +-
drivers/net/intel/iavf/iavf_generic_flow.h | 2 +-
drivers/net/intel/iavf/iavf_hash.c | 153 +--
drivers/net/intel/iavf/iavf_ipsec_crypto.c | 43 +-
drivers/net/intel/ice/ice_acl_filter.c | 146 ++-
drivers/net/intel/ice/ice_fdir_filter.c | 384 ++++---
drivers/net/intel/ice/ice_generic_flow.c | 59 +-
drivers/net/intel/ice/ice_generic_flow.h | 2 +-
drivers/net/intel/ice/ice_hash.c | 189 ++--
drivers/net/intel/ice/ice_switch_filter.c | 389 +++----
drivers/net/intel/ixgbe/ixgbe_flow.c | 1161 +++++++-------------
lib/eal/include/rte_common.h | 4 +
lib/meson.build | 2 +
22 files changed, 2269 insertions(+), 2272 deletions(-)
create mode 100644 drivers/net/intel/common/flow_check.h
create mode 100644 drivers/net/intel/common/log.h
--
2.47.3
^ permalink raw reply
* [PATCH] eal: check for invalid memory parameters
From: Stephen Hemminger @ 2026-06-04 16:12 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
The code to parse arguments like memory size, channels and rank
was using atoi() which has no check for garbage after the number.
Switch to using a helper that uses strtoull().
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/eal/common/eal_common_options.c | 44 +++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 1049838d73..49151c0a16 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -2062,6 +2062,29 @@ eal_parse_huge_worker_stack(const char *arg)
return 0;
}
+static int
+eal_parse_num(const char *str, unsigned int *val)
+{
+ char *endptr;
+ unsigned long long n;
+
+ while (isspace((unsigned char)*str))
+ str++;
+
+ if (*str == '-')
+ return -1;
+
+ errno = 0;
+ n = strtoull(str, &endptr, 10);
+
+ /* Error if string is empty or has trailing characters */
+ if (*str == '\0' || *endptr != '\0' || errno != 0 || n > UINT_MAX)
+ return -1;
+
+ *val = n;
+ return 0;
+}
+
/* Parse the arguments given in the command line of the application */
int
eal_parse_args(void)
@@ -2205,23 +2228,34 @@ eal_parse_args(void)
/* memory options */
if (args.memory_size != NULL) {
- int_cfg->memory = atoi(args.memory_size);
+ unsigned int mb;
+ if (eal_parse_num(args.memory_size, &mb) < 0) {
+ EAL_LOG(ERR, "invalid memory size parameter");
+ return -1;
+ }
+
+ int_cfg->memory = mb;
int_cfg->memory *= 1024ULL;
int_cfg->memory *= 1024ULL;
}
if (args.memory_channels != NULL) {
- int_cfg->force_nchannel = atoi(args.memory_channels);
- if (int_cfg->force_nchannel == 0) {
+ unsigned int n;
+ if (eal_parse_num(args.memory_channels, &n) < 0 ||
+ n == 0 || n > 32) {
EAL_LOG(ERR, "invalid memory channel parameter");
return -1;
}
+ int_cfg->force_nchannel = n;
}
if (args.memory_ranks != NULL) {
- int_cfg->force_nrank = atoi(args.memory_ranks);
- if (int_cfg->force_nrank == 0 || int_cfg->force_nrank > 16) {
+ unsigned int n;
+
+ if (eal_parse_num(args.memory_ranks, &n) < 0 ||
+ n == 0 || n > 16) {
EAL_LOG(ERR, "invalid memory rank parameter");
return -1;
}
+ int_cfg->force_nrank = n;
}
if (args.no_huge) {
int_cfg->no_hugetlbfs = 1;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v6 03/27] net/intel/common: add common flow action parsing
From: Burakov, Anatoly @ 2026-06-04 15:50 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev
In-Reply-To: <aiAmGWmhD2n5ZPAc@bricha3-mobl1.ger.corp.intel.com>
On 6/3/2026 3:03 PM, Bruce Richardson wrote:
> On Fri, May 29, 2026 at 04:36:05PM +0100, Anatoly Burakov wrote:
>> Currently, each driver has their own code for action parsing, which results
>> in a lot of duplication and subtle mismatches in behavior between drivers.
>>
>> Add common infrastructure, based on the following assumptions:
>>
>> - All drivers support at most 32 actions at once, but usually far less
>> - Not every action is supported by all drivers
>> - We can check a few common things to filter out obviously wrong actions
>> - Driver performs semantic checks on all valid actions
>>
>> So, the intention is to reject everything we can reasonably reject at the
>> outset without knowing anything about the drivers, parametrize what is
>> trivial to parametrize, and leave the rest for the driver to implement.
>>
>> While we're at it, also add logging infrastructure for Intel common code,
>> using the new component name defines that are automatically passed to each
>> DPDK driver as it is being built.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>> drivers/net/intel/common/flow_check.h | 279 ++++++++++++++++++++++++++
>> drivers/net/intel/common/log.h | 40 ++++
>> 2 files changed, 319 insertions(+)
>> create mode 100644 drivers/net/intel/common/flow_check.h
>> create mode 100644 drivers/net/intel/common/log.h
>>
>
> <snip>
>
>> +/**
>> + * Validate and parse a list of rte_flow_action into a parsed action list.
>> + *
>> + * @param actions pointer to array of rte_flow_action, terminated by RTE_FLOW_ACTION_TYPE_END
>> + * @param param pointer to ci_flow_actions_check_param structure (can be NULL)
>> + * @param parsed_actions pointer to ci_flow_actions structure to store parsed actions
>> + * @param error pointer to rte_flow_error structure for error reporting
>> + *
>> + * @return 0 on success, negative errno on failure.
>> + */
>> +static inline int
>> +ci_flow_check_actions(const struct rte_flow_action *actions,
>> + const struct ci_flow_actions_check_param *param,
>> + struct ci_flow_actions *parsed_actions,
>> + struct rte_flow_error *error)
>> +{
>> + size_t i = 0;
>> + int ret;
>> +
>> + if (actions == NULL) {
>> + return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
>> + NULL, "Missing actions");
>> + }
>> +
>> + /* reset the list */
>> + *parsed_actions = (struct ci_flow_actions){0};
>> +
>> + while (actions[i].type != RTE_FLOW_ACTION_TYPE_END) {
>> + const struct rte_flow_action *action = &actions[i++];
>> +
>> + /* skip VOID actions */
>> + if (action->type == RTE_FLOW_ACTION_TYPE_VOID)
>> + continue;
>> +
>> + /* generic validation for actions - this will check against param as well */
>> + ret = __flow_action_check_generic(action, param, error);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* check against global maximum number of actions */
>> + if (parsed_actions->count >= RTE_DIM(parsed_actions->actions)) {
>> + return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
>> + action, "Too many actions");
>> + }
>> + /* user may have specified a maximum number of actions */
>> + if (param != NULL && param->max_actions != 0 &&
>> + parsed_actions->count >= param->max_actions) {
>> + return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
>> + action, "Too many actions");
>> + }
>> + /* add action to the list */
>> + CI_DRV_LOG(DEBUG, "Parsed action %u: type=%s", parsed_actions->count,
>> + ci_flow_action_type_to_str(action->type));
>> + parsed_actions->actions[parsed_actions->count++] = action;
>> + }
>> +
>> + /* now, call into user validation if specified */
>> + if (param != NULL && param->check != NULL) {
>> + ret = param->check(parsed_actions, param, error);
>> + if (ret < 0)
>> + return ret;
>> + }
>
> Running an AI review on this code myself, it highlights the fact that we
> are missing a check for an empty parsed_actions array here, and not all
> check handlers verify the length as being >0 before dereferencing. For
> example, in patch 10, the callbacks don't explicitly check for empty lists
> I think it would be reasonable to have the return -EINVAL before this
> callback check, right?
Yep, that is correct. Empty list should fail *before* we go into use
callbacks. It is safe in the sense that references are never invalid,
and well-behaved callbacks should check count, but that only reasonably
applies to callbacks expecting multiple actions, but not just one.
>
>> + /* if we didn't parse anything, valid action list is empty */
>> + return parsed_actions->count == 0 ? -EINVAL : 0;
>> +}
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
> <snip>
--
Thanks,
Anatoly
^ permalink raw reply
* RE: [PATCH 2/5] ring: use GCC builtin as alternative to rte_atomic32
From: Konstantin Ananyev @ 2026-06-04 15:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev@dpdk.org, Wathsala Vithanage
In-Reply-To: <20260604082010.1fe8a27d@phoenix.local>
> On Thu, 4 Jun 2026 15:11:25 +0000
> Konstantin Ananyev <konstantin.ananyev@huawei.com> wrote:
>
> > > /**
> > > * @internal This is a helper function that moves the producer/consumer
> head
> > > * optimized for single threaded case
> > > @@ -82,7 +81,7 @@ __rte_ring_headtail_move_head_st(struct
> rte_ring_headtail
> > > *d,
> > > /* Single producer: only this thread writes d->head,
> > > * so a relaxed load is sufficient.
> > > */
> > > - *old_head = rte_atomic_load_explicit(&d->head,
> > > rte_memory_order_relaxed);
> > > + *old_head = rte_atomic_load_explicit(&d->head,
> > > rte_memory_order_acquire);
> >
> > Not sure, why it had changed to 'acquire' here?
> > Looks like just patch splitting mistake, no?
>
> I should have kept it as relaxed for the first load.
Yes, I believe so.
In fact, I reverted it back to 'relaxed' in final version (after applying all 5 patches)
and run both stress_ring_autotest and stress_soring_autotest on ARM box in our lab.
All passed.
Konstantin
^ permalink raw reply
* RE: [PATCH 5/5] ring: use C11 for single thread move head
From: Konstantin Ananyev @ 2026-06-04 15:41 UTC (permalink / raw)
To: Stephen Hemminger, dev@dpdk.org; +Cc: Wathsala Vithanage
In-Reply-To: <20260602171552.686349-6-stephen@networkplumber.org>
> The function to move head for single threaded case can always
> use the C11 code, there is no performance difference from GCC
> intrinsics.
>
> This reduces the exception code to just one function.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/ring/rte_ring_c11_pvt.h | 62 ------------------------------
> lib/ring/rte_ring_elem_pvt.h | 74 +++++++++++++++++++++++++++++++++---
> lib/ring/rte_ring_gcc_pvt.h | 59 ----------------------------
> 3 files changed, 68 insertions(+), 127 deletions(-)
>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> --
> 2.53.0
^ permalink raw reply
* RE: [PATCH 4/5] ring: drop unused arg to update_tail
From: Konstantin Ananyev @ 2026-06-04 15:40 UTC (permalink / raw)
To: Stephen Hemminger, dev@dpdk.org; +Cc: Wathsala Vithanage
In-Reply-To: <20260602171552.686349-5-stephen@networkplumber.org>
> The internal functions to update tail of ring no longer use
> the enqueue flag argument.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/ring/rte_ring_elem_pvt.h | 8 +++-----
> lib/ring/rte_ring_hts_elem_pvt.h | 8 +++-----
> lib/ring/soring.c | 10 +++++-----
> 3 files changed, 11 insertions(+), 15 deletions(-)
>
> --
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> 2.53.0
^ permalink raw reply
* RE: [PATCH 3/5] ring: use C11 for update_tail
From: Konstantin Ananyev @ 2026-06-04 15:39 UTC (permalink / raw)
To: Stephen Hemminger, dev@dpdk.org; +Cc: Wathsala Vithanage
In-Reply-To: <20260602171552.686349-4-stephen@networkplumber.org>
> The GCC builtin atomic special case is not needed for updating tail.
> The performance is the same with C11 memory model.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/ring/rte_ring_c11_pvt.h | 24 ------------------------
> lib/ring/rte_ring_elem_pvt.h | 22 ++++++++++++++++++++++
> lib/ring/rte_ring_gcc_pvt.h | 25 -------------------------
> 3 files changed, 22 insertions(+), 49 deletions(-)
>
> --
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> 2.53.0
^ permalink raw reply
* Re: [PATCH 2/5] ring: use GCC builtin as alternative to rte_atomic32
From: Stephen Hemminger @ 2026-06-04 15:20 UTC (permalink / raw)
To: Konstantin Ananyev; +Cc: dev@dpdk.org, Wathsala Vithanage
In-Reply-To: <d65ce1df239445e0bb795c852e075b1a@huawei.com>
On Thu, 4 Jun 2026 15:11:25 +0000
Konstantin Ananyev <konstantin.ananyev@huawei.com> wrote:
> > /**
> > * @internal This is a helper function that moves the producer/consumer head
> > * optimized for single threaded case
> > @@ -82,7 +81,7 @@ __rte_ring_headtail_move_head_st(struct rte_ring_headtail
> > *d,
> > /* Single producer: only this thread writes d->head,
> > * so a relaxed load is sufficient.
> > */
> > - *old_head = rte_atomic_load_explicit(&d->head,
> > rte_memory_order_relaxed);
> > + *old_head = rte_atomic_load_explicit(&d->head,
> > rte_memory_order_acquire);
>
> Not sure, why it had changed to 'acquire' here?
> Looks like just patch splitting mistake, no?
I should have kept it as relaxed for the first load.
^ permalink raw reply
* RE: [PATCH 2/5] ring: use GCC builtin as alternative to rte_atomic32
From: Konstantin Ananyev @ 2026-06-04 15:11 UTC (permalink / raw)
To: Stephen Hemminger, dev@dpdk.org; +Cc: Wathsala Vithanage
In-Reply-To: <20260602171552.686349-3-stephen@networkplumber.org>
> This patch replaces use of the deprecated rte_atomic32 code with
> GCC builtin atomic operations.
>
> Although it would be preferable to use C11 version on all architectures,
> there is a performance loss if we do it that way:
>
> Measured on i9-13900H, two physical cores MP/MC bulk n=128, 10 runs:
> with C11 builtin: 5.86 cycles/elem
> with __sync builtin: 5.36 cycles/elem (-9.4%)
>
> The C11 __atomic_compare_exchange_n builtin writes the actual value back
> to its expected pointer on failure. On x86 this forces GCC
> to emit extra instructions on the critical path between the CAS
> and the success-test.
>
> __sync_bool_compare_and_swap returns a plain bool with no pointer
> writeback, allowing GCC to emit tighter code.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/ring/meson.build | 2 +-
> lib/ring/rte_ring_c11_pvt.h | 3 +-
> lib/ring/rte_ring_elem_pvt.h | 2 +-
> ..._ring_generic_pvt.h => rte_ring_gcc_pvt.h} | 37 +++++++++++--------
> 4 files changed, 24 insertions(+), 20 deletions(-)
> rename lib/ring/{rte_ring_generic_pvt.h => rte_ring_gcc_pvt.h} (87%)
>
> diff --git a/lib/ring/meson.build b/lib/ring/meson.build
> index 21f2c12989..2ba160b178 100644
> --- a/lib/ring/meson.build
> +++ b/lib/ring/meson.build
> @@ -9,7 +9,7 @@ indirect_headers += files (
> 'rte_ring_elem.h',
> 'rte_ring_elem_pvt.h',
> 'rte_ring_c11_pvt.h',
> - 'rte_ring_generic_pvt.h',
> + 'rte_ring_gcc_pvt.h',
> 'rte_ring_hts.h',
> 'rte_ring_hts_elem_pvt.h',
> 'rte_ring_peek.h',
> diff --git a/lib/ring/rte_ring_c11_pvt.h b/lib/ring/rte_ring_c11_pvt.h
> index 5afc14dec9..8358b0f21f 100644
> --- a/lib/ring/rte_ring_c11_pvt.h
> +++ b/lib/ring/rte_ring_c11_pvt.h
> @@ -43,7 +43,6 @@ __rte_ring_update_tail(struct rte_ring_headtail *ht,
> uint32_t old_val,
> */
> rte_atomic_store_explicit(&ht->tail, new_val,
> rte_memory_order_release);
> }
> -
> /**
> * @internal This is a helper function that moves the producer/consumer head
> * optimized for single threaded case
> @@ -82,7 +81,7 @@ __rte_ring_headtail_move_head_st(struct rte_ring_headtail
> *d,
> /* Single producer: only this thread writes d->head,
> * so a relaxed load is sufficient.
> */
> - *old_head = rte_atomic_load_explicit(&d->head,
> rte_memory_order_relaxed);
> + *old_head = rte_atomic_load_explicit(&d->head,
> rte_memory_order_acquire);
Not sure, why it had changed to 'acquire' here?
Looks like just patch splitting mistake, no?
>
> /* Acquire pairs with the consumer's release-store of tail in
> __rte_ring_update_tail,
> * ensuring the consumer's ring-element reads are complete before
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox