[PWCI] "[ovs-dev] netdev-dpdk: Remove workarounds for fixed checksum bugs."

[ovsrobot]

NOTE: This is an auto submission for "[ovs-dev] netdev-dpdk: Remove workarounds for fixed checksum bugs.".

See "http://patchwork.ozlabs.org/project/openvswitch/list/?series=473093" for details.

Summary by Sourcery

Remove obsolete DPDK checksum workarounds and improve native tunnel parsing by handling the IP 'Don't Fragment' bit and preserving existing GRE flags.

New Features:

• Extract and record the IP 'Don't Fragment' (DF) flag in native tunnel metadata

Enhancements:

• Remove disabling of L4 checksum offloads for net/tap and net/txgbe drivers in netdev-dpdk
• Preserve existing tunnel flags when marking GRE checksum instead of overwriting them

Summary by CodeRabbit

• Bug Fixes

  • Correctly preserve GRE tunnel flags and propagate the IPv4 DF flag in tunnel metadata, aligning flow-dump outputs with actual packet flags.

• Refactor

  • Removed legacy DPDK TX offload overrides so driver-reported capabilities are used as-is.

• Tests

  • Updated expected flow-dump outputs to reflect +df-csum+key flags, adjusted GRE in_port representation, and minor ordering tweaks across tunnel-related scenarios.

• Chores

  • Updated maintainers list (moved one maintainer to Emeritus).

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR removes obsolete DPDK driver checksum-workarounds in the datapath while improving native tunnel parsing by adding DF-bit support and fixing GRE checksum flag handling, and updates related metadata and test expectations.

Entity relationship diagram for updated tunnel metadata flags

erDiagram
    DP_PACKET ||--o| FLOW_TNL : extracts
    FLOW_TNL {
        int ip_tos
        int ip_ttl
        int flags
    }
    DP_PACKET {
    }

    FLOW_TNL ||--|{ FLAGS : has
    FLAGS {
        FLOW_TNL_F_DONT_FRAGMENT
        FLOW_TNL_F_CSUM
    }

Loading

Class diagram for updated netdev_dpdk and tunnel metadata handling

classDiagram
    class netdev_dpdk {
        hw_ol_features
        +dpdk_eth_dev_init()
    }
    class dp_packet {
    }
    class flow_tnl {
        ip_tos
        ip_ttl
        flags
    }
    netdev_dpdk -- dp_packet : uses
    dp_packet -- flow_tnl : extracts metadata

    class ip_extract_tnl_md {
        +ip_extract_tnl_md(packet, tnl, ...)
    }
    dp_packet -- ip_extract_tnl_md : used in
    ip_extract_tnl_md -- flow_tnl : sets flags

    class parse_gre_header {
        +parse_gre_header(packet, tnl, ...)
    }
    dp_packet -- parse_gre_header : used in
    parse_gre_header -- flow_tnl : sets flags

    flow_tnl : +FLOW_TNL_F_DONT_FRAGMENT
    flow_tnl : +FLOW_TNL_F_CSUM

Loading

File-Level Changes

Change	Details	Files
Remove outdated DPDK driver-specific checksum offload workarounds	Dropped net_tap L4 checksum disable block Dropped net_txgbe outer UDP checksum disable block	lib/netdev-dpdk.c
Enhance native tunnel parsing in ip_extract_tnl_md	Set FLOW_TNL_F_DONT_FRAGMENT when IP DF bit is present Changed GRE checksum flag handling to OR existing flags instead of overwrite	lib/netdev-native-tnl.c
Update project metadata and test baselines	Updated MAINTAINERS.rst entries Regenerated nsh.at, packet-type-aware.at, tunnel-push-pop.at to match new behavior	MAINTAINERS.rst tests/nsh.at tests/packet-type-aware.at tests/tunnel-push-pop.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.

[coderabbitai]

Walkthrough

The change updates maintainers metadata, removes DPDK driver-specific TX offload overrides, adjusts tunnel metadata parsing to set DF and preserve flags when GRE CSUM is present, and updates test expectations to reflect DF flag visibility (and some in_port renderings).

Changes

Cohort / File(s)	Summary of Changes
Maintainers list update MAINTAINERS.rst	Moved Russell Bryant from “OVS Maintainers” to “OVS Emeritus Maintainers”.
DPDK TX offload overrides removal lib/netdev-dpdk.c	Deleted net_tap and net_txgbe workaround blocks that forcibly cleared certain TX offload bits; TX offloads now rely solely on driver-reported capabilities.
Tunnel metadata flags handling lib/netdev-native-tnl.c	IPv4 parsing: set DONT_FRAGMENT in tunnel metadata when IP DF bit is set. GRE parsing: use bitwise OR to add FLOW_TNL_F_CSUM, preserving existing flags (e.g., KEY).
Test expectation updates (DF flag, render order/ports) tests/nsh.at, tests/packet-type-aware.at, tests/tunnel-push-pop.at	Updated expected flow-dump text: flags switch from -df to +df (often +df-csum+key). Several blocks show in_port(gre_sys) and minor field reordering. No test logic changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant NIC as NIC
  participant PMD as DPDK PMD
  participant OVS as OVS netdev
  participant TNL as Tunnel Parser

  NIC->>PMD: Advertise TX offload capabilities
  PMD->>OVS: rte_eth_dev_info (capabilities)
  OVS->>OVS: Initialize TX offloads = PMD capabilities
  Note over OVS: No driver-specific overrides applied

Loading

sequenceDiagram
  autonumber
  participant RX as Rx Path
  participant IP as IPv4 Parser
  participant GRE as GRE Parser
  participant MD as Tunnel Metadata

  RX->>IP: Parse IPv4 header
  IP->>MD: Set header fields
  IP-->>MD: If IP.DF set → MD.flags |= DF
  IP->>GRE: If proto=GRE, parse GRE hdr
  GRE->>MD: If GRE_CSUM → MD.flags |= CSUM (preserve existing flags)
  GRE->>MD: If KEY present → MD.flags |= KEY
  Note over MD: Flags accumulate via OR (DF/CSUM/KEY)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks (1 passed, 2 warnings)

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The current title only references the DPDK checksum workaround removal, but the pull request also moves a maintainer entry, updates native tunnel code for DF flag handling, and revises multiple tests; it does not comprehensively summarize the scope of changes in a concise way.	Revise the title to clearly and concisely capture the primary changes across the PR, for example “netdev: remove DPDK checksum workarounds, add DF flag in tunnel metadata, and update related tests” or split unrelated maintainer updates into a separate commit so the title can focus on functional changes.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

Poem

I twitch my nose at flags anew,
A DF bit now shining through.
GRE and friends all play along,
We OR the tunes into one song.
Offloads heed the driver’s say—
Hippity-hop, we merge today! 🐇✨

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

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

---

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]

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

---

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.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

MAINTAINERS.rst (1)

94-95: Move to Emeritus looks right; minor nit on ordering.

Russell Bryant’s entry under Emeritus LGTM. Please ensure the list remains alphabetically sorted by name if that’s the convention in this file.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a2169a and 38df31e.

📒 Files selected for processing (6)

• MAINTAINERS.rst (1 hunks)
• lib/netdev-dpdk.c (0 hunks)
• lib/netdev-native-tnl.c (2 hunks)
• tests/nsh.at (2 hunks)
• tests/packet-type-aware.at (8 hunks)
• tests/tunnel-push-pop.at (1 hunks)

💤 Files with no reviewable changes (1)

• lib/netdev-dpdk.c

🧰 Additional context used

🧬 Code graph analysis (1)

lib/netdev-native-tnl.c (1)

include/sparse/netinet/in.h (1)

• htons (158-161)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: freebsd_build COMPILER:clang freebsd_instance:family/freebsd-13-5-snap
• GitHub Check: freebsd_build COMPILER:gcc freebsd_instance:family/freebsd-14-3-snap
• GitHub Check: build oss-fuzz fuzzers

🔇 Additional comments (12)

tests/tunnel-push-pop.at (1)

688-689: Updated tunnel flags expectation (+df-csum+key) matches DF propagation change.

This aligns with setting FLOW_TNL_F_DONT_FRAGMENT from the outer IPv4 DF bit. No further changes needed.

lib/netdev-native-tnl.c (2)

155-158: Correct: propagate IPv4 DF into tunnel metadata.

Using ‘ip->ip_frag_off & htons(IP_DF)’ and OR-ing FLOW_TNL_F_DONT_FRAGMENT is correct and symmetric with header build logic.

---

474-474: Fix GRE flag clobbering by OR-ing CSUM.

Changing to ‘tnl->flags |= FLOW_TNL_F_CSUM’ preserves previously set flags (e.g., KEY). Good catch.

tests/nsh.at (2)

722-724: NSH over VXLAN: +df-csum+key expectation is consistent.

Matches DF flag extraction and checksum preservation. Looks good.

---

776-779: Follow-on DF flag updates LGTM.

The flipped DF bit in subsequent recirc entries is consistent with the parser change.

tests/packet-type-aware.at (7)

327-328: GRE path: switch to flags(+df-csum) and in_port(gre_sys) is correct.

Reflects DF propagation and sys GRE port naming in dumps.

---

345-346: DF flag expectation update LGTM.

Keeps consistency across N1→N2 case.

---

363-364: DF flag expectation update LGTM.

Matches reverse-path GRE handling.

---

381-383: Chained GRE hops: +df-csum updates are correct.

Both intermediate and final hop expectations align with new metadata flags.

---

400-402: L3 GRE decap path: DF flag shown for tunnel flows is correct.

Expectation matches IPv4 DF extraction before push/recirc.

---

419-420: DF flag expectation update LGTM.

Consistent with earlier sections.

---

1018-1020: MPLS-over-GRE recirc: DF flag presence is correct.

Dumps showing flags(+df-csum) before and after recirc are as expected.