|SUCCESS| [v1,1/1] net/iavf: remove RAW flow item support from fsub
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Apr 30 19:00:04 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163472
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-30
I have reviewed the patch against the DPDK coding standards and guidelines. Below is my review:
```
commit 34827309e49eaba498cfe41500ee8de1fd3a2bb5
Author: Anatoly Burakov <anatoly.burakov at intel.com>
net/iavf: remove RAW flow item support from fsub
Currently, no kernel driver implements support for RAW items, so having
this support in our parser is extra work for no value. Remove the support
for RAW flow items until such time as there is kernel driver support.
Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
---
drivers/net/intel/iavf/iavf_fsub.c | 75 ------------------------------
1 file changed, 75 deletions(-)
diff --git a/drivers/net/intel/iavf/iavf_fsub.c b/drivers/net/intel/iavf/iavf_fsub.c
index bfb34695de..a00d02ded5 100644
--- a/drivers/net/intel/iavf/iavf_fsub.c
+++ b/drivers/net/intel/iavf/iavf_fsub.c
@@ -57,7 +57,6 @@ static struct iavf_flow_parser iavf_fsub_parser;
static struct
iavf_pattern_match_item iavf_fsub_pattern_list[] = {
- {iavf_pattern_raw, IAVF_INSET_NONE, IAVF_INSET_NONE},
{iavf_pattern_ethertype, IAVF_SW_INSET_ETHER, IAVF_INSET_NONE},
[ ... ]
@@ -154,7 +153,6 @@ iavf_fsub_parse_pattern(const struct rte_flow_item pattern[],
{
struct virtchnl_proto_hdrs *hdrs = &filter->sub_fltr.proto_hdrs;
enum rte_flow_item_type item_type;
- const struct rte_flow_item_raw *raw_spec, *raw_mask;
const struct rte_flow_item_eth *eth_spec, *eth_mask;
const struct rte_flow_item_ipv4 *ipv4_spec, *ipv4_mask;
const struct rte_flow_item_ipv6 *ipv6_spec, *ipv6_mask;
@@ -166,7 +164,6 @@ iavf_fsub_parse_pattern(const struct rte_flow_item pattern[],
uint64_t outer_input_set = IAVF_INSET_NONE;
uint64_t *input = NULL;
uint16_t input_set_byte = 0;
- uint8_t item_num = 0;
uint32_t layer = 0;
for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
@@ -178,79 +175,7 @@ iavf_fsub_parse_pattern(const struct rte_flow_item pattern[],
}
item_type = item->type;
- item_num++;
-
switch (item_type) {
- case RTE_FLOW_ITEM_TYPE_RAW: {
- raw_spec = item->spec;
- raw_mask = item->mask;
-
- if (!raw_spec || !raw_mask) {
- PMD_DRV_LOG(ERR, "NULL RAW spec/mask");
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- item, "NULL RAW spec/mask");
- return -rte_errno;
- }
-
- if (item_num != 1)
- return -rte_errno;
-
- if (raw_spec->length != raw_mask->length)
- return -rte_errno;
Does this code leak the rte_flow_error state? When returning -rte_errno in
these error paths, should rte_flow_error_set() be called first to set the
error context?
-
- uint16_t pkt_len = 0;
- uint16_t tmp_val = 0;
- uint8_t tmp = 0;
- int i, j;
Variable declarations after the case label do not follow coding style. Per
the coding standards, variables should be declared at the start of the
enclosing block, not mid-function after a case label within a braced
section. Was this code style non-compliant before removal?
-
- pkt_len = raw_spec->length;
-
- for (i = 0, j = 0; i < pkt_len; i += 2, j++) {
- tmp = raw_spec->pattern[i];
- if (tmp >= 'a' && tmp <= 'f')
- tmp_val = tmp - 'a' + 10;
- if (tmp >= 'A' && tmp <= 'F')
- tmp_val = tmp - 'A' + 10;
- if (tmp >= '0' && tmp <= '9')
- tmp_val = tmp - '0';
-
- tmp_val *= 16;
- tmp = raw_spec->pattern[i + 1];
- if (tmp >= 'a' && tmp <= 'f')
- tmp_val += (tmp - 'a' + 10);
- if (tmp >= 'A' && tmp <= 'F')
- tmp_val += (tmp - 'A' + 10);
- if (tmp >= '0' && tmp <= '9')
- tmp_val += (tmp - '0');
-
- hdrs->raw.spec[j] = tmp_val;
-
- tmp = raw_mask->pattern[i];
- if (tmp >= 'a' && tmp <= 'f')
- tmp_val = tmp - 'a' + 10;
- if (tmp >= 'A' && tmp <= 'F')
- tmp_val = tmp - 'A' + 10;
- if (tmp >= '0' && tmp <= '9')
- tmp_val = tmp - '0';
-
- tmp_val *= 16;
- tmp = raw_mask->pattern[i + 1];
- if (tmp >= 'a' && tmp <= 'f')
- tmp_val += (tmp - 'a' + 10);
- if (tmp >= 'A' && tmp <= 'F')
- tmp_val += (tmp - 'A' + 10);
- if (tmp >= '0' && tmp <= '9')
- tmp_val += (tmp - '0');
-
- hdrs->raw.mask[j] = tmp_val;
- }
-
- hdrs->raw.pkt_len = pkt_len / 2;
- hdrs->tunnel_level = 0;
- hdrs->count = 0;
- return 0;
- }
case RTE_FLOW_ITEM_TYPE_ETH:
eth_spec = item->spec;
eth_mask = item->mask;
Are there any other references to iavf_pattern_raw in the codebase that
should also be removed? Does the pattern definition still exist elsewhere
now that its use is removed from the match list?
```
More information about the test-report
mailing list