-
Notifications
You must be signed in to change notification settings - Fork 4
[PWCI] "[v3] examples/flow_filtering: add more snippets" #518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Starting with NVIDIA ConnectX-9, the future devices will support only hardware steering (HWS) flow engine. The software steering options (legacy Verbs and Direct Verbs) have lower performances, and won't be available for new devices. Both flow APIs (sync and async template) will still be supported with the hardware steering flow engine. Fixes: 1b55eeb ("common/mlx5: add ConnectX-9 SuperNIC") Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
The tool ripgrep allows to find files not ending with a line break: rg -Ul '[^\n]\z' The files with a trailing blank lines are shown with this command: rg -Ul '\n\n\z' Files are fixed to end with a single line break. Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Added the following snippets: - Steering by Integrity Check Flags - Match packet type - ECN in IP Header Modification - Random Match - VXLAN-GPE Header Fields Matching - VXLAN-GBP Header Fields Matching - NVGRE Matching - NAT64 Action Usage Signed-off-by: Shani Peretz <shperetz@nvidia.com> ----------------- v3: fix makefile v2: fix compilation errors Signed-off-by: 0-day Robot <robot@bytheb.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WalkthroughRelease finalization updating version to 25.11.0, removing trailing whitespace/blank lines across drivers and test data files, removing two public API function declarations, updating documentation for mlx5 DV flow steering, adding eight new flow filtering example snippets with supporting build configurations, and removing a build dependency from hinic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 19
🧹 Nitpick comments (5)
examples/flow_filtering/snippets/snippet_match_integrity_flags.h (1)
23-26: Minor documentation clarification needed.The documentation states "Level > 2 - inner" which would mean level 3 and above is inner. Typically in DPDK flow API, level 1 is outer and level 2 is inner. Please verify if "Level > 2" or "Level >= 2" is the correct threshold for inner matching.
examples/flow_filtering/snippets/snippet_NAT64.c (2)
38-43: Inconsistent indentation.The function body is missing indentation, which is inconsistent with the rest of the file.
void snippet_match_nat64_create_patterns(struct rte_flow_item *pattern) { -pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH; -pattern[1].type = RTE_FLOW_ITEM_TYPE_END; + pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH; + pattern[1].type = RTE_FLOW_ITEM_TYPE_END; }
45-59: Inconsistent indentation in function body.The local variable declarations and initializations lack proper indentation.
static struct rte_flow_pattern_template * snippet_match_nat64_create_pattern_template(uint16_t port_id, struct rte_flow_error *error) { -const struct rte_flow_pattern_template_attr pt_attr = { .egress = 1 }; -const struct rte_flow_item pattern[2] = { - [0] = { - .type = RTE_FLOW_ITEM_TYPE_ETH, - }, - [1] = { - .type = RTE_FLOW_ITEM_TYPE_END, - }, -}; + const struct rte_flow_pattern_template_attr pt_attr = { .egress = 1 }; + const struct rte_flow_item pattern[2] = { + [0] = { + .type = RTE_FLOW_ITEM_TYPE_ETH, + }, + [1] = { + .type = RTE_FLOW_ITEM_TYPE_END, + }, + }; -return rte_flow_pattern_template_create(port_id, &pt_attr, pattern, error); + return rte_flow_pattern_template_create(port_id, &pt_attr, pattern, error); }examples/flow_filtering/snippets/snippet_match_packet_type.h (1)
5-6: Missing#include <rte_flow.h>for type declarations.This header uses
uint16_t,struct rte_flow_action,struct rte_flow_item,struct rte_flow_template_table, andstruct rte_flow_errorin function declarations without including the necessary header. The sibling headersnippet_match_vxlan_gpe.hincludes<rte_flow.h>for consistency.#ifndef SNIPPET_MATCH_PACKET_TYPE_H #define SNIPPET_MATCH_PACKET_TYPE_H + +#include <rte_flow.h>examples/flow_filtering/snippets/snippet_match_vxlan_gpe.h (1)
31-32: Nit: extra blank line before#endif.There's an extra blank line that's inconsistent with
snippet_match_packet_type.h. Consider removing for consistency.#define snippet_skeleton_flow_create_table snippet_match_vxlan_gpe_create_table - #endif /* SNIPPET_MATCH_VXLAN_GPE_H */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
doc/guides/contributing/img/abi_stability_policy.svgis excluded by!**/*.svgdoc/guides/contributing/img/what_is_an_abi.svgis excluded by!**/*.svgdoc/guides/howto/img/flow_bifurcation_overview.svgis excluded by!**/*.svgdoc/guides/howto/img/virtio_user_for_container_networking.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-0.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-1.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-2.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-3.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-4.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-5.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/lcore_var_mem_layout.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/pdcp_functional_overview.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/predictable_snat_1.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/predictable_snat_2.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/rss_queue_assign.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/static_array_mem_layout.svgis excluded by!**/*.svgdoc/guides/sample_app_ug/img/overlay_networking.svgis excluded by!**/*.svg
📒 Files selected for processing (67)
VERSION(1 hunks)app/test-bbdev/test_vectors/bbdev_null.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_high_snr.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_low_snr.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e34560_sbd_posllr.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c2_k3136_r0_e4920_sbd_negllr_crc24b.data(1 hunks)app/test-bbdev/test_vectors/turbo_enc_c1_k6144_r0_e18448_crc24a.data(1 hunks)app/test-bbdev/test_vectors/turbo_enc_c3_k4800_r2_e14412_crc24b.data(1 hunks)doc/guides/nics/cxgbe.rst(0 hunks)doc/guides/nics/ipn3ke.rst(0 hunks)doc/guides/nics/mlx5.rst(2 hunks)drivers/event/dlb2/dlb2_iface.c(0 hunks)drivers/net/bnxt/tf_core/tf_em_common.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_act.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_class.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_defs.h(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_tbl.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor2_class.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor_class.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_wh_plus_class.c(0 hunks)drivers/net/gve/base/gve_adminq.c(0 hunks)drivers/net/gve/gve_rss.c(0 hunks)drivers/net/gve/gve_rss.h(0 hunks)drivers/net/hinic/base/hinic_pmd_mbox.c(0 hunks)drivers/net/hinic/base/hinic_pmd_niccfg.c(0 hunks)drivers/net/hinic/base/meson.build(0 hunks)drivers/net/intel/e1000/base/e1000_82575.c(0 hunks)drivers/net/intel/e1000/base/e1000_mbx.c(0 hunks)drivers/net/intel/e1000/base/e1000_vf.c(0 hunks)drivers/net/intel/i40e/base/i40e_adminq.c(0 hunks)drivers/net/intel/ice/base/ice_flex_pipe.c(0 hunks)drivers/net/intel/ice/base/ice_hw_autogen.h(0 hunks)drivers/net/intel/ixgbe/base/ixgbe_dcb_82599.c(0 hunks)drivers/net/intel/ixgbe/base/ixgbe_x540.h(0 hunks)drivers/net/mlx5/hws/mlx5dr_buddy.c(0 hunks)drivers/net/nfp/nfp_ipsec.c(0 hunks)drivers/net/nfp/nfpcore/nfp_elf.c(0 hunks)drivers/net/ngbe/base/ngbe_dummy.h(0 hunks)drivers/net/ngbe/base/ngbe_phy.c(0 hunks)drivers/net/octeontx/base/octeontx_bgx.h(0 hunks)drivers/net/txgbe/base/txgbe_dcb.c(0 hunks)drivers/net/txgbe/base/txgbe_dcb_hw.c(0 hunks)drivers/net/txgbe/base/txgbe_dummy.h(0 hunks)drivers/net/txgbe/base/txgbe_eeprom.c(0 hunks)drivers/net/virtio/virtio_cvq.c(0 hunks)drivers/raw/cnxk_rvu_lf/cnxk_rvu_lf_selftest.c(0 hunks)drivers/vdpa/mlx5/mlx5_vdpa_event.c(0 hunks)dts/configurations/tests_config.example.yaml(1 hunks)examples/flow_filtering/Makefile(1 hunks)examples/flow_filtering/meson.build(1 hunks)examples/flow_filtering/snippets/snippet_NAT64.c(1 hunks)examples/flow_filtering/snippets/snippet_NAT64.h(1 hunks)examples/flow_filtering/snippets/snippet_match_integrity_flags.c(1 hunks)examples/flow_filtering/snippets/snippet_match_integrity_flags.h(1 hunks)examples/flow_filtering/snippets/snippet_match_nvgre.c(1 hunks)examples/flow_filtering/snippets/snippet_match_nvgre.h(1 hunks)examples/flow_filtering/snippets/snippet_match_packet_type.c(1 hunks)examples/flow_filtering/snippets/snippet_match_packet_type.h(1 hunks)examples/flow_filtering/snippets/snippet_match_vxlan_gbp.c(1 hunks)examples/flow_filtering/snippets/snippet_match_vxlan_gbp.h(1 hunks)examples/flow_filtering/snippets/snippet_match_vxlan_gpe.c(1 hunks)examples/flow_filtering/snippets/snippet_match_vxlan_gpe.h(1 hunks)examples/flow_filtering/snippets/snippet_modify_ecn.c(1 hunks)examples/flow_filtering/snippets/snippet_modify_ecn.h(1 hunks)examples/flow_filtering/snippets/snippet_random_match.c(1 hunks)examples/flow_filtering/snippets/snippet_random_match.h(1 hunks)license/Linux-syscall-note(0 hunks)
💤 Files with no reviewable changes (39)
- drivers/net/ngbe/base/ngbe_dummy.h
- drivers/net/txgbe/base/txgbe_eeprom.c
- drivers/net/intel/e1000/base/e1000_82575.c
- drivers/net/bnxt/tf_core/tf_em_common.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor_class.c
- drivers/vdpa/mlx5/mlx5_vdpa_event.c
- doc/guides/nics/cxgbe.rst
- drivers/net/octeontx/base/octeontx_bgx.h
- drivers/net/intel/ixgbe/base/ixgbe_dcb_82599.c
- drivers/net/mlx5/hws/mlx5dr_buddy.c
- drivers/net/gve/gve_rss.h
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_tbl.c
- drivers/net/txgbe/base/txgbe_dcb.c
- doc/guides/nics/ipn3ke.rst
- drivers/net/hinic/base/meson.build
- drivers/net/virtio/virtio_cvq.c
- drivers/net/gve/gve_rss.c
- drivers/net/intel/e1000/base/e1000_mbx.c
- drivers/event/dlb2/dlb2_iface.c
- drivers/net/hinic/base/hinic_pmd_mbox.c
- drivers/net/nfp/nfpcore/nfp_elf.c
- drivers/net/txgbe/base/txgbe_dummy.h
- drivers/net/gve/base/gve_adminq.c
- drivers/net/txgbe/base/txgbe_dcb_hw.c
- drivers/net/intel/ice/base/ice_hw_autogen.h
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_act.c
- drivers/net/intel/i40e/base/i40e_adminq.c
- license/Linux-syscall-note
- drivers/net/nfp/nfp_ipsec.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_defs.h
- drivers/net/intel/ixgbe/base/ixgbe_x540.h
- drivers/net/intel/e1000/base/e1000_vf.c
- drivers/net/intel/ice/base/ice_flex_pipe.c
- drivers/raw/cnxk_rvu_lf/cnxk_rvu_lf_selftest.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_wh_plus_class.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor2_class.c
- drivers/net/ngbe/base/ngbe_phy.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_class.c
- drivers/net/hinic/base/hinic_pmd_niccfg.c
🧰 Additional context used
🧬 Code graph analysis (9)
examples/flow_filtering/snippets/snippet_random_match.c (2)
examples/flow_filtering/common.h (1)
init_default_snippet(14-19)lib/ethdev/rte_flow.c (3)
rte_flow_pattern_template_create(1770-1824)rte_flow_actions_template_create(1858-1922)rte_flow_template_table_create(1956-2027)
examples/flow_filtering/snippets/snippet_match_integrity_flags.h (1)
examples/flow_filtering/snippets/snippet_match_integrity_flags.c (4)
snippet_init_integrity_flags(11-15)snippet_match_integrity_flags_create_actions(17-30)snippet_match_integrity_flags_create_patterns(32-93)snippet_match_integrity_flags_create_table(95-101)
examples/flow_filtering/snippets/snippet_NAT64.h (1)
examples/flow_filtering/snippets/snippet_NAT64.c (4)
snippet_init_nat64(12-17)snippet_match_nat64_create_actions(19-36)snippet_match_nat64_create_patterns(38-43)snippet_match_nat64_create_table(101-130)
examples/flow_filtering/snippets/snippet_NAT64.c (2)
examples/flow_filtering/jump_flow.c (1)
create_jump_flow(10-39)lib/ethdev/rte_flow.c (3)
rte_flow_pattern_template_create(1770-1824)rte_flow_actions_template_create(1858-1922)rte_flow_template_table_create(1956-2027)
examples/flow_filtering/snippets/snippet_random_match.h (1)
examples/flow_filtering/snippets/snippet_random_match.c (1)
snippet_init_random_match(12-16)
examples/flow_filtering/snippets/snippet_match_packet_type.c (1)
lib/ethdev/rte_flow.c (3)
rte_flow_pattern_template_create(1770-1824)rte_flow_actions_template_create(1858-1922)rte_flow_template_table_create(1956-2027)
examples/flow_filtering/snippets/snippet_match_vxlan_gpe.c (2)
examples/flow_filtering/jump_flow.c (1)
create_jump_flow(10-39)lib/ethdev/rte_flow.c (3)
rte_flow_pattern_template_create(1770-1824)rte_flow_actions_template_create(1858-1922)rte_flow_template_table_create(1956-2027)
examples/flow_filtering/snippets/snippet_match_vxlan_gpe.h (1)
examples/flow_filtering/snippets/snippet_match_vxlan_gpe.c (4)
snippet_init_vxlan_gpe(12-17)snippet_match_vxlan_gpe_create_actions(19-33)snippet_match_vxlan_gpe_create_patterns(35-50)snippet_match_vxlan_gpe_create_table(120-149)
examples/flow_filtering/snippets/snippet_match_nvgre.c (2)
examples/flow_filtering/jump_flow.c (1)
create_jump_flow(10-39)lib/ethdev/rte_flow.c (3)
rte_flow_pattern_template_create(1770-1824)rte_flow_actions_template_create(1858-1922)rte_flow_template_table_create(1956-2027)
🔇 Additional comments (24)
app/test-bbdev/test_vectors/turbo_enc_c3_k4800_r2_e14412_crc24b.data (1)
152-153: Formatting cleanup approved.The trailing newline adjustment to the
expected_statusfield is a non-functional formatting change that aligns with the broader test data cleanup in this PR. No issues detected.doc/guides/nics/mlx5.rst (2)
691-702: Documentation updates clarify device support and requirements for DV flow steering. ✓The changes to the
dv_flow_enparameter documentation improve clarity by explicitly stating device compatibility constraints (devices older than ConnectX-9 for Values 0 and 1) and the rdma-core minimum requirement ("24 or later" for Value 1 DV flow steering). The updates are well-integrated with the Hardware Steering section.
1337-1345: Hardware Steering section update reinforces ConnectX-9 as the support boundary. ✓The clarification that hardware steering (HWS) is the only mode supporting the flow template async API and the only mode supported on ConnectX-9 and later creates a coherent picture with the dv_flow_en parameter documentation. Device progression is now explicit: ConnectX-4 through ConnectX-8 support multiple steering modes, while ConnectX-9 requires HWS.
app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e34560_sbd_posllr.data (1)
1222-1222: LGTM! Formatting cleanup with no functional impact.This is a trailing newline adjustment at the end of the test vector data file. The change is purely cosmetic and does not affect the test data values or expected behavior.
app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_high_snr.data (1)
640-640: LGTM - Trailing newline addition.Adding a trailing newline to end the file is POSIX-compliant and aligns with the formatting cleanup across other test vector files in this PR.
app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_low_snr.data (1)
640-640: LGTM - Trailing newline addition.Consistent formatting fix matching other test vector files in this PR.
dts/configurations/tests_config.example.yaml (1)
17-17: LGTM - No functional change.The commented configuration option content is unchanged; this is a formatting-only adjustment.
VERSION (1)
1-1: LGTM - Release version finalization.Version updated from
25.11.0-rc4to25.11.0, removing the release candidate suffix for the final release.app/test-bbdev/test_vectors/bbdev_null.data (1)
5-5: LGTM - Trailing newline fix.Adds proper trailing newline to the file; no change to the
op_typevalue.app/test-bbdev/test_vectors/turbo_enc_c1_k6144_r0_e18448_crc24a.data (1)
159-159: LGTM - Trailing newline addition.Consistent formatting fix for the
expected_statusvalue, matching other test vector files in this PR.app/test-bbdev/test_vectors/turbo_dec_c2_k3136_r0_e4920_sbd_negllr_crc24b.data (1)
676-677: LGTM - trailing newline adjustment.Formatting-only change ensuring the file ends with a proper newline.
examples/flow_filtering/snippets/snippet_random_match.c (1)
120-156: Table creation logic looks correct.The error handling for template creation is properly implemented with null checks and appropriate error messages before returning NULL.
examples/flow_filtering/snippets/snippet_match_nvgre.h (1)
1-40: LGTM - well-structured NVGRE snippet header.The header correctly declares the public API, provides appropriate documentation describing the NVGRE matching fields, and defines consistent macro aliases for the skeleton integration.
examples/flow_filtering/snippets/snippet_match_integrity_flags.h (1)
28-45: LGTM - integrity flags header structure is correct.Function declarations and macro aliases are properly defined and consistent with the implementation.
examples/flow_filtering/snippets/snippet_match_integrity_flags.c (1)
95-101: Stub implementation returns NULL.The
snippet_match_integrity_flags_create_tablefunction returns NULL without creating a template table. Other snippets (e.g.,snippet_match_nvgre_create_table,snippet_match_nat64_create_table) implement full table creation logic.Is this intentional for this snippet, or should a proper implementation be added?
examples/flow_filtering/snippets/snippet_NAT64.h (1)
1-45: LGTM - well-documented NAT64 snippet header.The header provides clear documentation on NAT64 action usage, automatic field conversion, and shared rule recommendations. Function declarations and macro aliases are correctly defined.
examples/flow_filtering/meson.build (1)
18-29: LGTM!The new snippet source files are correctly added to the Meson build configuration. The additions integrate properly with the existing build structure.
examples/flow_filtering/Makefile (1)
6-6: LGTM!Adding
jump_flow.cto the build sources is correct, as the new snippets (NAT64, VXLAN-GPE) depend oncreate_jump_flow.examples/flow_filtering/snippets/snippet_modify_ecn.c (1)
55-60: Stub table creation function returns NULL unconditionally.Unlike other snippets (e.g.,
snippet_match_packet_type.c,snippet_match_nvgre.c) that implement full template table creation, this function is a stub. If this is intentional for example purposes, consider adding a comment explaining why. Otherwise, implement proper table creation for consistency.examples/flow_filtering/snippets/snippet_match_packet_type.c (1)
99-129: LGTM!The
snippet_match_packet_type_create_tablefunction has proper error handling for template creation failures, correctly returning NULL on error. The template table creation logic follows the expected pattern.examples/flow_filtering/snippets/snippet_match_vxlan_gbp.h (1)
1-33: LGTM!The header correctly declares the public API and provides macro aliases for skeleton integration.
MAX_PATTERN_NUM(5) andMAX_ACTION_NUM(2) match the implementation's array usage insnippet_match_vxlan_gbp.c.examples/flow_filtering/snippets/snippet_modify_ecn.h (1)
1-32: LGTM!The header correctly declares the public API with appropriate
MAX_PATTERN_NUM(3) andMAX_ACTION_NUM(3) matching the implementation's usage. The macro aliases follow the consistent pattern used by other snippet headers.examples/flow_filtering/snippets/snippet_match_packet_type.h (1)
8-43: Well-structured header with comprehensive documentation.The detailed documentation of supported packet types and the consistent macro aliasing pattern for skeleton flow API compatibility looks good.
examples/flow_filtering/snippets/snippet_match_vxlan_gpe.h (1)
1-32: LGTM - well-structured header with proper includes.The header follows the established pattern with proper include, header guards, and macro aliases for skeleton flow API compatibility. The MAX_PATTERN_NUM of 5 correctly matches the implementation which uses pattern indices 0-4 (ETH, IPV4, UDP, VXLAN_GPE, END).
| struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue)); | ||
| if (queue == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for queue\n"); | ||
|
|
||
| queue->index = 1; | ||
| action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE; | ||
| action[0].conf = queue; | ||
| action[1].type = RTE_FLOW_ACTION_TYPE_END; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Null pointer dereference if memory allocation fails.
Same issue as in other snippets: if calloc returns NULL, line 26 dereferences queue->index causing a crash.
Apply this diff:
struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue));
- if (queue == NULL)
+ if (queue == NULL) {
fprintf(stderr, "Failed to allocate memory for queue\n");
+ return;
+ }
queue->index = 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue)); | |
| if (queue == NULL) | |
| fprintf(stderr, "Failed to allocate memory for queue\n"); | |
| queue->index = 1; | |
| action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE; | |
| action[0].conf = queue; | |
| action[1].type = RTE_FLOW_ACTION_TYPE_END; | |
| } | |
| struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue)); | |
| if (queue == NULL) { | |
| fprintf(stderr, "Failed to allocate memory for queue\n"); | |
| return; | |
| } | |
| queue->index = 1; | |
| action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE; | |
| action[0].conf = queue; | |
| action[1].type = RTE_FLOW_ACTION_TYPE_END; | |
| } |
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_match_integrity_flags.c around lines
22 to 30, the code dereferences queue after calloc without handling NULL; update
the function to check if calloc returned NULL and on failure print the error and
return an appropriate error code (or goto cleanup) instead of proceeding, only
assign queue->index and set action[0].conf when queue is non-NULL, and ensure
any allocated resources are freed on the error path to avoid leaks.
| integrity_spec = calloc(1, sizeof(struct rte_flow_item_integrity)); | ||
| if (integrity_spec == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for integrity_spec\n"); | ||
|
|
||
| integrity_spec->level = 0; | ||
| integrity_spec->l3_ok = 1; | ||
| integrity_spec->ipv4_csum_ok = 1; | ||
|
|
||
| integrity_mask = calloc(1, sizeof(struct rte_flow_item_integrity)); | ||
| if (integrity_mask == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for integrity_mask\n"); | ||
|
|
||
| integrity_mask->level = 0; | ||
| integrity_mask->l3_ok = 1; | ||
| integrity_mask->l4_ok = 0; | ||
| integrity_mask->ipv4_csum_ok = 1; | ||
|
|
||
| ip_spec = calloc(1, sizeof(struct rte_flow_item_ipv4)); | ||
| if (ip_spec == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for ip_spec\n"); | ||
|
|
||
| ip_mask = calloc(1, sizeof(struct rte_flow_item_ipv4)); | ||
| if (ip_mask == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for ip_mask\n"); | ||
|
|
||
| ip_spec->hdr.dst_addr = htonl(((192<<24) + (168<<16) + (1<<8) + 1)); | ||
| ip_mask->hdr.dst_addr = 0xffffffff; | ||
| ip_spec->hdr.src_addr = htonl(((0<<24) + (0<<16) + (0<<8) + 0)); | ||
| ip_mask->hdr.src_addr = 0x0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple potential null pointer dereferences after failed allocations.
The code continues executing after allocation failures at lines 41-42, 48-50, 57-59, and 61-63. If any allocation fails, subsequent dereferences (lines 44-46, 52-55, 65-68) will crash.
Consider returning early on any allocation failure:
integrity_spec = calloc(1, sizeof(struct rte_flow_item_integrity));
- if (integrity_spec == NULL)
+ if (integrity_spec == NULL) {
fprintf(stderr, "Failed to allocate memory for integrity_spec\n");
+ return;
+ }Apply similar patterns for integrity_mask, ip_spec, and ip_mask allocations.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_match_integrity_flags.c around lines
40 to 68, several calloc() calls (for integrity_spec, integrity_mask, ip_spec,
ip_mask) are followed only by fprintf on failure but the function continues and
dereferences those pointers causing potential crashes; update each allocation
check to handle failure immediately—log the error, free any previously allocated
structures as needed, and return an error (or goto cleanup) instead of
continuing so no NULL pointer is dereferenced. Ensure you apply the same pattern
for integrity_spec, integrity_mask, ip_spec and ip_mask and use a single cleanup
path if multiple allocations may need freeing.
| struct rte_flow_error error; | ||
| create_jump_flow(port_id, 1, &error); | ||
|
|
||
| struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue)); | ||
| if (queue == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for queue\n"); | ||
| queue->index = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignored error from create_jump_flow and null pointer dereference.
Two issues:
- The return value and error from
create_jump_floware ignored - callers won't know if the jump flow setup failed. - If
callocfails at line 25, the NULL pointer is dereferenced at line 28.
struct rte_flow_error error;
- create_jump_flow(port_id, 1, &error);
+ if (create_jump_flow(port_id, 1, &error) == NULL) {
+ fprintf(stderr, "Failed to create jump flow: %s\n", error.message);
+ return;
+ }
struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue));
- if (queue == NULL)
+ if (queue == NULL) {
fprintf(stderr, "Failed to allocate memory for queue\n");
- queue->index = 1;
+ return;
+ }
+ queue->index = 1;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_match_nvgre.c around lines 22 to 28,
the call to create_jump_flow ignores its return value and rte_flow_error result,
and the calloc for queue is not checked before use; update the code to capture
and check create_jump_flow's return value and/or the rte_flow_error struct and
handle failure (log error and abort or return an error code) instead of
proceeding, and after calloc ensure queue != NULL before dereferencing (on NULL
log error, free/cleanup as needed and exit/return), avoiding any use of
queue->index when allocation failed.
| struct rte_flow_item_nvgre *nvgre = calloc(1, sizeof(struct rte_flow_item_nvgre)); | ||
| if (nvgre == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for nvgre\n"); | ||
|
|
||
| struct rte_flow_item_udp *udp = calloc(1, sizeof(struct rte_flow_item_udp)); | ||
| if (udp == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for udp\n"); | ||
|
|
||
| struct rte_flow_item_nvgre *nvgre_mask = calloc(1, sizeof(struct rte_flow_item_nvgre)); | ||
| if (nvgre_mask == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for nvgre_mask\n"); | ||
|
|
||
| struct rte_flow_item_udp *udp_mask = calloc(1, sizeof(struct rte_flow_item_udp)); | ||
| if (udp_mask == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for udp_mask\n"); | ||
|
|
||
| /* build rule to match specific NVGRE: | ||
| * tni = 0x12346, flow_id = 0x78, inner_udp_src = 0x1234 | ||
| */ | ||
| nvgre->tni[0] = 0x12; | ||
| nvgre->tni[1] = 0x34; | ||
| nvgre->tni[2] = 0x56; | ||
| nvgre->flow_id = 0x78; | ||
| udp->hdr.src_port = RTE_BE16(0x1234); | ||
|
|
||
| /* define nvgre and udp match mask for all bits */ | ||
| nvgre_mask->tni[0] = 0xff; | ||
| nvgre_mask->tni[1] = 0xff; | ||
| nvgre_mask->tni[2] = 0xff; | ||
| nvgre_mask->flow_id = 0xff; | ||
| udp_mask->hdr.src_port = 0xffff; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Null pointer dereferences after allocation failures.
All four allocations (nvgre, udp, nvgre_mask, udp_mask) print errors on failure but continue execution, leading to NULL pointer dereferences when accessing their fields at lines 57-68.
Add early returns after each allocation failure, freeing any previously allocated memory:
struct rte_flow_item_nvgre *nvgre = calloc(1, sizeof(struct rte_flow_item_nvgre));
- if (nvgre == NULL)
+ if (nvgre == NULL) {
fprintf(stderr, "Failed to allocate memory for nvgre\n");
+ return;
+ }
struct rte_flow_item_udp *udp = calloc(1, sizeof(struct rte_flow_item_udp));
- if (udp == NULL)
+ if (udp == NULL) {
fprintf(stderr, "Failed to allocate memory for udp\n");
+ free(nvgre);
+ return;
+ }Apply similar pattern for nvgre_mask and udp_mask.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_match_nvgre.c around lines 38 to 68,
the code prints allocation failure messages for nvgre, udp, nvgre_mask and
udp_mask but continues and dereferences NULL; after each calloc failure add an
immediate error return or jump to a cleanup path that frees any previously
allocated structures (e.g., if nvgre allocation fails return/error immediately;
if udp fails free nvgre then return; if nvgre_mask fails free nvgre and udp then
return; if udp_mask fails free nvgre, udp, nvgre_mask then return), ensuring no
NULL pointer is dereferenced and all allocated memory is released on error.
| pt = snippet_match_nvgre_create_pattern_template(port_id, error); | ||
| if (pt == NULL) { | ||
| printf("Failed to create pattern template: %s (%s)\n", | ||
| error->message, rte_strerror(rte_errno)); | ||
| return NULL; | ||
| } | ||
|
|
||
| at = snippet_match_nvgre_create_actions_template(port_id, error); | ||
| if (at == NULL) { | ||
| printf("Failed to create actions template: %s (%s)\n", | ||
| error->message, rte_strerror(rte_errno)); | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource leak: pattern template not destroyed on actions template failure.
If snippet_match_nvgre_create_actions_template fails at line 171, the pattern template pt created at line 164 is leaked. Consider destroying pt before returning NULL.
at = snippet_match_nvgre_create_actions_template(port_id, error);
if (at == NULL) {
printf("Failed to create actions template: %s (%s)\n",
error->message, rte_strerror(rte_errno));
+ rte_flow_pattern_template_destroy(port_id, pt, error);
return NULL;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pt = snippet_match_nvgre_create_pattern_template(port_id, error); | |
| if (pt == NULL) { | |
| printf("Failed to create pattern template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| return NULL; | |
| } | |
| at = snippet_match_nvgre_create_actions_template(port_id, error); | |
| if (at == NULL) { | |
| printf("Failed to create actions template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| return NULL; | |
| } | |
| pt = snippet_match_nvgre_create_pattern_template(port_id, error); | |
| if (pt == NULL) { | |
| printf("Failed to create pattern template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| return NULL; | |
| } | |
| at = snippet_match_nvgre_create_actions_template(port_id, error); | |
| if (at == NULL) { | |
| printf("Failed to create actions template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| rte_flow_pattern_template_destroy(port_id, pt, error); | |
| return NULL; | |
| } |
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_match_nvgre.c around lines 164-176,
the pattern template (pt) created at line 164 is leaked if
snippet_match_nvgre_create_actions_template fails at line 171; before returning
NULL on actions template failure, call the appropriate destroy/free function for
pt (e.g., snippet_match_nvgre_destroy_pattern_template or the matching free
function used elsewhere) to release resources, then return NULL.
| void | ||
| snippet_match_nat64_create_actions(uint16_t port_id, struct rte_flow_action *action) | ||
| { | ||
| struct rte_flow_error error; | ||
| create_jump_flow(port_id, 1, &error); | ||
|
|
||
| struct rte_flow_action_nat64 *nat64_v = calloc(1, sizeof(struct rte_flow_action_nat64)); | ||
| struct rte_flow_action_jump *jump_v = calloc(1, sizeof(struct rte_flow_action_jump)); | ||
|
|
||
| nat64_v->type = RTE_FLOW_NAT64_4TO6; | ||
| jump_v->group = 2; | ||
|
|
||
| action[0].type = RTE_FLOW_ACTION_TYPE_NAT64; | ||
| action[0].conf = nat64_v; | ||
| action[1].type = RTE_FLOW_ACTION_TYPE_JUMP; | ||
| action[1].conf = jump_v; | ||
| action[2].type = RTE_FLOW_ACTION_TYPE_END; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NULL pointer dereference after failed allocation.
The function allocates memory using calloc but doesn't check for allocation failure before dereferencing the pointers. If calloc returns NULL, accessing nat64_v->type or jump_v->group will cause a crash.
Apply this diff to add proper error handling:
void
snippet_match_nat64_create_actions(uint16_t port_id, struct rte_flow_action *action)
{
struct rte_flow_error error;
create_jump_flow(port_id, 1, &error);
struct rte_flow_action_nat64 *nat64_v = calloc(1, sizeof(struct rte_flow_action_nat64));
+ if (nat64_v == NULL) {
+ fprintf(stderr, "Failed to allocate memory for nat64_v\n");
+ return;
+ }
struct rte_flow_action_jump *jump_v = calloc(1, sizeof(struct rte_flow_action_jump));
+ if (jump_v == NULL) {
+ fprintf(stderr, "Failed to allocate memory for jump_v\n");
+ free(nat64_v);
+ return;
+ }
nat64_v->type = RTE_FLOW_NAT64_4TO6;
jump_v->group = 2;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void | |
| snippet_match_nat64_create_actions(uint16_t port_id, struct rte_flow_action *action) | |
| { | |
| struct rte_flow_error error; | |
| create_jump_flow(port_id, 1, &error); | |
| struct rte_flow_action_nat64 *nat64_v = calloc(1, sizeof(struct rte_flow_action_nat64)); | |
| struct rte_flow_action_jump *jump_v = calloc(1, sizeof(struct rte_flow_action_jump)); | |
| nat64_v->type = RTE_FLOW_NAT64_4TO6; | |
| jump_v->group = 2; | |
| action[0].type = RTE_FLOW_ACTION_TYPE_NAT64; | |
| action[0].conf = nat64_v; | |
| action[1].type = RTE_FLOW_ACTION_TYPE_JUMP; | |
| action[1].conf = jump_v; | |
| action[2].type = RTE_FLOW_ACTION_TYPE_END; | |
| } | |
| void | |
| snippet_match_nat64_create_actions(uint16_t port_id, struct rte_flow_action *action) | |
| { | |
| struct rte_flow_error error; | |
| create_jump_flow(port_id, 1, &error); | |
| struct rte_flow_action_nat64 *nat64_v = calloc(1, sizeof(struct rte_flow_action_nat64)); | |
| if (nat64_v == NULL) { | |
| fprintf(stderr, "Failed to allocate memory for nat64_v\n"); | |
| return; | |
| } | |
| struct rte_flow_action_jump *jump_v = calloc(1, sizeof(struct rte_flow_action_jump)); | |
| if (jump_v == NULL) { | |
| fprintf(stderr, "Failed to allocate memory for jump_v\n"); | |
| free(nat64_v); | |
| return; | |
| } | |
| nat64_v->type = RTE_FLOW_NAT64_4TO6; | |
| jump_v->group = 2; | |
| action[0].type = RTE_FLOW_ACTION_TYPE_NAT64; | |
| action[0].conf = nat64_v; | |
| action[1].type = RTE_FLOW_ACTION_TYPE_JUMP; | |
| action[1].conf = jump_v; | |
| action[2].type = RTE_FLOW_ACTION_TYPE_END; | |
| } |
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_NAT64.c around lines 19-36, the code
dereferences nat64_v and jump_v without checking calloc return values; add
checks immediately after each calloc and if either is NULL free the other
allocated pointer (if non-NULL), set action[0].type = RTE_FLOW_ACTION_TYPE_END
(or otherwise terminate the actions array), and return from the function to
avoid further dereference; optionally call rte_errno/rte_flow_error_set or log
an error before returning to aid debugging.
| pt = snippet_match_nat64_create_pattern_template(port_id, error); | ||
| if (pt == NULL) { | ||
| printf("Failed to create pattern template: %s (%s)\n", | ||
| error->message, rte_strerror(rte_errno)); | ||
| return NULL; | ||
| } | ||
|
|
||
| at = snippet_match_nat64_create_actions_template(port_id, error); | ||
| if (at == NULL) { | ||
| printf("Failed to create actions template: %s (%s)\n", | ||
| error->message, rte_strerror(rte_errno)); | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource leak when actions template creation fails.
If snippet_match_nat64_create_actions_template fails, the previously created pattern template (pt) is not destroyed, causing a resource leak.
pt = snippet_match_nat64_create_pattern_template(port_id, error);
if (pt == NULL) {
printf("Failed to create pattern template: %s (%s)\n",
error->message, rte_strerror(rte_errno));
return NULL;
}
at = snippet_match_nat64_create_actions_template(port_id, error);
if (at == NULL) {
printf("Failed to create actions template: %s (%s)\n",
error->message, rte_strerror(rte_errno));
+ rte_flow_pattern_template_destroy(port_id, pt, NULL);
return NULL;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pt = snippet_match_nat64_create_pattern_template(port_id, error); | |
| if (pt == NULL) { | |
| printf("Failed to create pattern template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| return NULL; | |
| } | |
| at = snippet_match_nat64_create_actions_template(port_id, error); | |
| if (at == NULL) { | |
| printf("Failed to create actions template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| return NULL; | |
| } | |
| pt = snippet_match_nat64_create_pattern_template(port_id, error); | |
| if (pt == NULL) { | |
| printf("Failed to create pattern template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| return NULL; | |
| } | |
| at = snippet_match_nat64_create_actions_template(port_id, error); | |
| if (at == NULL) { | |
| printf("Failed to create actions template: %s (%s)\n", | |
| error->message, rte_strerror(rte_errno)); | |
| rte_flow_pattern_template_destroy(port_id, pt, NULL); | |
| return NULL; | |
| } |
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_NAT64.c around lines 115 to 127,
when snippet_match_nat64_create_actions_template fails the previously created
pattern template (pt) is not destroyed causing a resource leak; fix by
destroying/freeing pt before returning (e.g., call the matching destroy/free
function for the pattern template —
snippet_match_nat64_destroy_pattern_template(pt) or the correct cleanup function
used in this codebase), then proceed to log the error and return NULL so no
resources are leaked.
| struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue)); | ||
| if (queue == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for queue\n"); | ||
| queue->index = UINT16_MAX; /* The selected target queue.*/ | ||
| action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE; | ||
| action[0].conf = queue; | ||
| action[1].type = RTE_FLOW_ACTION_TYPE_END; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Null pointer dereference if memory allocation fails.
If calloc fails, the code prints an error but continues to dereference queue->index on line 25, causing a null pointer dereference and crash.
Apply this diff to handle the allocation failure properly:
struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue));
- if (queue == NULL)
+ if (queue == NULL) {
fprintf(stderr, "Failed to allocate memory for queue\n");
+ return;
+ }
queue->index = UINT16_MAX; /* The selected target queue.*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue)); | |
| if (queue == NULL) | |
| fprintf(stderr, "Failed to allocate memory for queue\n"); | |
| queue->index = UINT16_MAX; /* The selected target queue.*/ | |
| action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE; | |
| action[0].conf = queue; | |
| action[1].type = RTE_FLOW_ACTION_TYPE_END; | |
| } | |
| struct rte_flow_action_queue *queue = calloc(1, sizeof(struct rte_flow_action_queue)); | |
| if (queue == NULL) { | |
| fprintf(stderr, "Failed to allocate memory for queue\n"); | |
| return; | |
| } | |
| queue->index = UINT16_MAX; /* The selected target queue.*/ | |
| action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE; | |
| action[0].conf = queue; | |
| action[1].type = RTE_FLOW_ACTION_TYPE_END; | |
| } |
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_random_match.c around lines 22 to
29, the code prints an error when calloc returns NULL but then dereferences
queue->index causing a null pointer dereference; add a NULL-check immediately
after calloc and handle the failure by not dereferencing queue (e.g., print the
error and return an error code or jump to the function cleanup/exit path),
ensuring action[0].conf is only set when queue is valid and freeing or cleaning
up any previously allocated resources as appropriate.
| struct rte_flow_item_random *random_item; | ||
| random_item = calloc(1, sizeof(struct rte_flow_item_random)); | ||
| if (random_item == NULL) | ||
| fprintf(stderr, "Failed to allocate memory for port_representor_spec\n"); | ||
|
|
||
| random_item->value = 0; | ||
|
|
||
| /* Set the patterns. */ | ||
| pattern[0].type = RTE_FLOW_ITEM_TYPE_RANDOM; | ||
| pattern[0].spec = random_item; | ||
| pattern[1].type = RTE_FLOW_ITEM_TYPE_ETH; | ||
| pattern[2].type = RTE_FLOW_ITEM_TYPE_END; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Null pointer dereference and incorrect error message.
Two issues:
- Line 37: Error message says "port_representor_spec" but should reference "random_item" (copy-paste error).
- Lines 36-39: If
callocfails, code continues to dereferencerandom_item->valuecausing a crash.
Apply this diff:
struct rte_flow_item_random *random_item;
random_item = calloc(1, sizeof(struct rte_flow_item_random));
- if (random_item == NULL)
- fprintf(stderr, "Failed to allocate memory for port_representor_spec\n");
+ if (random_item == NULL) {
+ fprintf(stderr, "Failed to allocate memory for random_item\n");
+ return;
+ }
random_item->value = 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct rte_flow_item_random *random_item; | |
| random_item = calloc(1, sizeof(struct rte_flow_item_random)); | |
| if (random_item == NULL) | |
| fprintf(stderr, "Failed to allocate memory for port_representor_spec\n"); | |
| random_item->value = 0; | |
| /* Set the patterns. */ | |
| pattern[0].type = RTE_FLOW_ITEM_TYPE_RANDOM; | |
| pattern[0].spec = random_item; | |
| pattern[1].type = RTE_FLOW_ITEM_TYPE_ETH; | |
| pattern[2].type = RTE_FLOW_ITEM_TYPE_END; | |
| } | |
| struct rte_flow_item_random *random_item; | |
| random_item = calloc(1, sizeof(struct rte_flow_item_random)); | |
| if (random_item == NULL) { | |
| fprintf(stderr, "Failed to allocate memory for random_item\n"); | |
| return; | |
| } | |
| random_item->value = 0; | |
| /* Set the patterns. */ | |
| pattern[0].type = RTE_FLOW_ITEM_TYPE_RANDOM; | |
| pattern[0].spec = random_item; | |
| pattern[1].type = RTE_FLOW_ITEM_TYPE_ETH; | |
| pattern[2].type = RTE_FLOW_ITEM_TYPE_END; | |
| } |
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_random_match.c around lines 34 to
46, the allocation error message is a copy-paste referencing
"port_representor_spec" and the code dereferences random_item despite calloc
possibly returning NULL; change the fprintf message to reference "random_item"
and add a NULL check after calloc that handles the failure (e.g., log the error
and return an error code or goto cleanup) so random_item->value is not accessed
when allocation fails, and ensure pattern[0].spec is only assigned when
allocation succeeded.
| static void | ||
| snippet_init_random_match(void); | ||
| #define snippet_init snippet_init_random_match | ||
|
|
||
| static void | ||
| snippet_match_random_create_actions(uint16_t port_id, struct rte_flow_action *action); | ||
| #define snippet_skeleton_flow_create_actions snippet_match_random_create_actions | ||
|
|
||
| static void | ||
| snippet_match_random_create_patterns(struct rte_flow_item *pattern); | ||
| #define snippet_skeleton_flow_create_patterns snippet_match_random_create_patterns | ||
|
|
||
| static struct rte_flow_template_table * | ||
| snippet_match_random_create_table(uint16_t port_id, struct rte_flow_error *error); | ||
| #define snippet_skeleton_flow_create_table snippet_match_random_create_table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name mismatch between header declarations and implementation.
The function names declared here don't match the implementations in snippet_random_match.c:
| Header Declaration | Implementation |
|---|---|
snippet_match_random_create_actions |
snippet_match_random_value_create_actions |
snippet_match_random_create_patterns |
snippet_match_random_value_create_patterns |
snippet_match_random_create_table |
snippet_match_random_value_create_table |
This will cause compilation/linker failures when the skeleton macros try to invoke these functions.
Apply this diff to fix the function names:
static void
-snippet_match_random_create_actions(uint16_t port_id, struct rte_flow_action *action);
-#define snippet_skeleton_flow_create_actions snippet_match_random_create_actions
+snippet_match_random_value_create_actions(uint16_t port_id, struct rte_flow_action *action);
+#define snippet_skeleton_flow_create_actions snippet_match_random_value_create_actions
static void
-snippet_match_random_create_patterns(struct rte_flow_item *pattern);
-#define snippet_skeleton_flow_create_patterns snippet_match_random_create_patterns
+snippet_match_random_value_create_patterns(struct rte_flow_item *pattern);
+#define snippet_skeleton_flow_create_patterns snippet_match_random_value_create_patterns
static struct rte_flow_template_table *
-snippet_match_random_create_table(uint16_t port_id, struct rte_flow_error *error);
-#define snippet_skeleton_flow_create_table snippet_match_random_create_table
+snippet_match_random_value_create_table(uint16_t port_id, struct rte_flow_error *error);
+#define snippet_skeleton_flow_create_table snippet_match_random_value_create_tableAlso note: this header uses static function declarations while other snippet headers (e.g., snippet_match_nvgre.h, snippet_NAT64.h) use non-static declarations. Consider aligning with the pattern used in other snippets for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static void | |
| snippet_init_random_match(void); | |
| #define snippet_init snippet_init_random_match | |
| static void | |
| snippet_match_random_create_actions(uint16_t port_id, struct rte_flow_action *action); | |
| #define snippet_skeleton_flow_create_actions snippet_match_random_create_actions | |
| static void | |
| snippet_match_random_create_patterns(struct rte_flow_item *pattern); | |
| #define snippet_skeleton_flow_create_patterns snippet_match_random_create_patterns | |
| static struct rte_flow_template_table * | |
| snippet_match_random_create_table(uint16_t port_id, struct rte_flow_error *error); | |
| #define snippet_skeleton_flow_create_table snippet_match_random_create_table | |
| static void | |
| snippet_init_random_match(void); | |
| #define snippet_init snippet_init_random_match | |
| static void | |
| snippet_match_random_value_create_actions(uint16_t port_id, struct rte_flow_action *action); | |
| #define snippet_skeleton_flow_create_actions snippet_match_random_value_create_actions | |
| static void | |
| snippet_match_random_value_create_patterns(struct rte_flow_item *pattern); | |
| #define snippet_skeleton_flow_create_patterns snippet_match_random_value_create_patterns | |
| static struct rte_flow_template_table * | |
| snippet_match_random_value_create_table(uint16_t port_id, struct rte_flow_error *error); | |
| #define snippet_skeleton_flow_create_table snippet_match_random_value_create_table |
🤖 Prompt for AI Agents
In examples/flow_filtering/snippets/snippet_random_match.h around lines 18 to
32, the declared function names (snippet_match_random_create_actions,
snippet_match_random_create_patterns, snippet_match_random_create_table) do not
match the implementations in snippet_random_match.c (which are named
snippet_match_random_value_create_actions,
snippet_match_random_value_create_patterns,
snippet_match_random_value_create_table); rename the declarations to the
implementation names so the skeleton macros resolve correctly; additionally,
remove the static keyword from these declarations to match the non-static
pattern used by other snippet headers (e.g., snippet_match_nvgre.h) for
consistent linkage.
NOTE: This is an auto submission for "[v3] examples/flow_filtering: add more snippets".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36829" for details.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.