[PWCI] "dpdk: Update dpdk-lcore-mask handling."

[ovsrobot]

Auto-submission for "http://patchwork.ozlabs.org/project/openvswitch/list/?series=472733"

Summary by Sourcery

Introduce a core-mask conversion utility and integrate it into DPDK startup options, replacing the old dpdk-lcore-mask handling and improving parameter conversion and validation.

Enhancements:

• Add cmask_to_lcore_list helper to generate sorted DPDK lcore list strings from hexadecimal core masks
• Refactor construct_dpdk_options to support generic param_conversion callbacks and apply it to dpdk-lcore-mask
• Add warning and skip logic for invalid dpdk-lcore-mask values

Documentation:

• Update vswitchd/vswitch.xml documentation to explain dpdk-lcore-mask thread assignment and default behavior

[coderabbitai]

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch series_472733

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sourcery-ai]

Reviewer's Guide

This PR enhances dpdk-lcore-mask support by introducing a conversion helper that translates a hex core mask into a sorted DPDK lcore list, refactors the options mapping to use a generic conversion callback for parameters, and updates documentation and tests to reflect the new behavior.

Class diagram for updated DPDK options mapping and core mask conversion

classDiagram
    class dpdk_options_map {
        +const char *ovs_configuration
        +const char *dpdk_option
        +char *(*param_conversion)(const char *)
    }
    class cmask_to_lcore_list {
        +char *cmask_to_lcore_list(const char *cmask)
    }
    dpdk_options_map --> cmask_to_lcore_list : uses param_conversion
    class construct_dpdk_options {
        +void construct_dpdk_options(const struct smap *, struct svec *)
    }
    construct_dpdk_options --> dpdk_options_map : iterates opts[]
    construct_dpdk_options --> cmask_to_lcore_list : calls for conversion
    class compare_core_ids {
        +int compare_core_ids(const void *, const void *)
    }
    cmask_to_lcore_list --> compare_core_ids : uses for sorting
    class ovs_numa_dump_cores_with_cmask {
        +struct ovs_numa_dump *ovs_numa_dump_cores_with_cmask(const char *)
    }
    cmask_to_lcore_list --> ovs_numa_dump_cores_with_cmask : gets core dump
    class ovs_numa_dump_count {
        +unsigned int ovs_numa_dump_count(struct ovs_numa_dump *)
    }
    cmask_to_lcore_list --> ovs_numa_dump_count : gets core count
    class ovs_numa_dump_destroy {
        +void ovs_numa_dump_destroy(struct ovs_numa_dump *)
    }
    cmask_to_lcore_list --> ovs_numa_dump_destroy : cleans up
    class svec {
        +void svec_add(struct svec *, const char *)
    }
    construct_dpdk_options --> svec : adds options

Loading

Flow diagram for hex core mask to DPDK lcore list conversion

flowchart TD
    A["Input: Hexadecimal core mask"] --> B["ovs_numa_dump_cores_with_cmask(cmask)"]
    B --> C["ovs_numa_dump_count(lcore_dump)"]
    C --> D["Extract core IDs from dump"]
    D --> E["Sort core IDs (compare_core_ids)"]
    E --> F["Limit to RTE_MAX_LCORE if needed"]
    F --> G["Format as DPDK lcore list string"]
    G --> H["Output: lcore list string"]

Loading

File-Level Changes

Change	Details	Files
Introduce core‐mask to lcore‐list conversion	Add compare_core_ids for sorting core IDs Implement cmask_to_lcore_list using ovs_numa_dump and qsort Build comma-separated lcores string and handle RTE_MAX_LCORE limit	lib/dpdk.c
Refactor DPDK options mapping to support parameter conversion	Remove default_enabled/default_value fields Add param_conversion callback to options map Invoke conversion, warn on invalid value, add/ free converted string	lib/dpdk.c
Update dpdk-lcore-mask documentation	Clarify assignment of main and worker threads Describe auto-determination and threading behavior Adjust text on default behavior and threading model	vswitchd/vswitch.xml
Adjust test suite for updated lcore-mask handling	Update or add tests to verify cmask_to_lcore_list output Refactor macros and system tests to exercise new parameter conversion	tests/pmd.at tests/system-dpdk-macros.at tests/system-dpdk.at tests/testsuite.at

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `lib/dpdk.c:90` </location>
<code_context>
+    lcore_dump = ovs_numa_dump_cores_with_cmask(cmask);
+
+    num_cores = ovs_numa_dump_count(lcore_dump);
+    core_ids = xmalloc(num_cores * sizeof *core_ids);
+
+    FOR_EACH_CORE_ON_DUMP(core, lcore_dump) {
</code_context>

<issue_to_address>
Potential allocation of zero bytes if num_cores is zero.

Check if num_cores is zero before allocating memory, and return NULL early to prevent unsafe zero-byte allocation.
</issue_to_address>

### Comment 2
<location> `lib/dpdk.c:113` </location>
<code_context>
+        free(lcores);
+        lcores = new_lcores;
+    }
+    if (lcores) {
+        lcores[strlen(lcores) - 1] = '\0';
+    }
+
</code_context>

<issue_to_address>
Manual removal of trailing comma may be error-prone.

Setting the last character to '\0' without verifying the string length can cause undefined behavior if lcores is empty. Please add a length check before modifying the string.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Potential allocation of zero bytes if num_cores is zero.

Check if num_cores is zero before allocating memory, and return NULL early to prevent unsafe zero-byte allocation.

[sourcery-ai]

issue (bug_risk): Manual removal of trailing comma may be error-prone.

Setting the last character to '\0' without verifying the string length can cause undefined behavior if lcores is empty. Please add a length check before modifying the string.