[PWCI] "Architectural refactoring of hardware offload infrastructure"

[ovsrobot]

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

Summary by CodeRabbit

• New Features
  • Introduced pluggable hardware offload framework with TC and DPDK rte_flow providers.
  • Added dpif/offload/show (text and JSON) to inspect offload state, providers, ports, and stats.
  • New global and per-port configuration: other_config:hw-offload-priority.
  • Enhanced dpctl offload stats output with labeled, multi-section formatting.
• Documentation
  • Added guide for running userspace datapath with DPDK and rte_flow offload, including prerequisites and test invocation.
• Tests
  • Added system RTE offloads testsuite and supporting macros; expanded offload visibility and traffic tests.

[sourcery-ai]

Sorry @ovsrobot, your pull request is larger than the review limit of 150000 diff characters

[coderabbitai]

Walkthrough

Adds a new DPIF offload framework with provider plugins (rte_flow, tc, dummy), integrates it across dpif, netdev, and ofproto, replaces legacy netdev-offload provider API, updates build/test infrastructure, introduces new public headers/APIs, adjusts flow dump/operate paths, and adds documentation and tests for rte_flow offloads.

Changes

Cohort / File(s)	Summary
Documentation Documentation/topics/testing.rst	Adds instructions for userspace datapath testing with DPDK rte_flow offload, env var for VF PCI list, and make target to run rte_flow tests.
Build system & sources lib/automake.mk	Adds dpif-offload core and providers (dummy, tc, rte_flow), tc/DPDK headers; removes legacy netdev-offload-provider header from exports.
DPIF offload public API (new) lib/dpif-offload.h, lib/dpif-offload.c, lib/dpif-offload-provider.h	Introduces provider model, per-dpif offload management, port manager, flow dump/operate hooks, stats, meters, marks, and iteration helpers.
DPIF core integration lib/dpif.c, lib/dpif-provider.h, lib/dpif.h, lib/dpif-netlink.c	Wires offload attach/detach, operate, flow/port dump, meters; updates dpif_class (remove synced_dp_layers/offload_stats_get; add get_features); renames flow dump type field to offloaded_flows; adds dpif_get_features.
Userspace datapath integration lib/dpif-netdev.c, lib/dpif-netdev-avx512.c, lib/dpif-netdev-private-flow.h	Replaces internal offload threads with provider calls; adds per-dp mark-to-flow map; adds datapath full_name; updates flow add/del/stats paths; switches gating to dpif_offload_is_offload_enabled.
DPDK and TC providers (implementations) lib/dpif-offload-rte_flow.c, lib/dpif-offload-rte_flow-private.h, lib/netdev-offload-dpdk.c, lib/netdev-offload-dpdk.h	Adds rte_flow provider with offload threads, port management, flow ops, stats, and helpers; updates DPDK netdev offload API to offload-context based signatures and per-thread counters.
TC provider (implementations) lib/dpif-offload-tc.c, lib/netdev-offload-tc.c, lib/netdev-offload-tc.h	Adds tc provider; refactors tc netdev API to offload-centric entry points (flows, dumps, meters) and offload-aware port resolution.
Dummy provider and helpers lib/dpif-offload-dummy.c, lib/dummy.h, lib/netdev-dummy.c	Adds dummy offload provider; exposes dummy class checks and simulation hook; removes legacy dummy netdev offload provider and state.
DPDK integration tweaks lib/dpdk.c, lib/netdev-dpdk.c, lib/netdev-dpdk.h	Drops legacy provider registration; gates features on dpif_offload_is_offload_enabled; updates flow API support signature (adds check_only).
Kernel datapath adjustments lib/dpif-netlink.c	Simplifies flow dump to kernel-only path; removes netdev dump mixing; adds get_features; drops offload_stats_get and synced_dp_layers.
CLI tooling lib/dpctl.c	Updates offload stats to array-of-custom-stats with labels; renames dump types member to offloaded_flows.
vswitchd integration & schema vswitchd/bridge.c, vswitchd/vswitch.xml	Pushes global offload config via dpif_offload_set_global_cfg; adds hw-offload-priority keys to Open_vSwitch and Port other_config.
Legacy netdev-offload removal lib/netdev-offload.c, lib/netdev-offload.h, lib/netdev-offload-provider.h	Removes legacy provider registry and flow API surface; trims header to hardware info; deletes netdev-offload-provider API.
Core netdev API tweaks lib/netdev-provider.h, lib/netdev.c, lib/netdev.h, include/openvswitch/netdev.h	Replaces netdev.flow_api with dpif_offload pointer; adds netdev_get_block_id; enhances custom stats with optional label; frees label in cleanup.
JSON utility include/openvswitch/json.h, lib/json.c	Adds json_object_is_empty() public API.
Linux netdev lib/netdev-linux.c	Switches policing offload gating to dpif_offload_is_offload_enabled.
Open vSwitch ofproto ofproto/ofproto-dpif.c, ofproto/ofproto-dpif-upcall.c	Adds dpif/offload show command (text/JSON); reworks counters, rebalance, and netdev lookups to use new offload APIs.
Tests: new rte_flow suite and updates tests/automake.mk, tests/.gitignore, tests/system-rte-offloads-*.at, tests/system-dpdk.at, tests/system-offloads-traffic.at, tests/dpif-netdev.at, tests/ofproto-macros.at	Adds rte_flow tests/macros/suite and make targets; updates tests to new offload logs/paths; adds offload visibility checks; macro signature adjustments.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Admin
  participant vswitchd
  participant DPIF as dpif (core)
  participant Offload as dpif-offload (mgr)
  participant Provider as provider(s): rte_flow/tc/dummy

  Admin->>vswitchd: Configure Open_vSwitch.other_config (hw-offload, priority)
  vswitchd->>Offload: dpif_offload_set_global_cfg(cfg)
  Note over Offload: Parse global hw-offload and provider priority

  vswitchd->>DPIF: open datapath
  DPIF->>Offload: attach_providers(dpif)
  Offload->>Provider: open(dpif) in priority order
  Provider-->>Offload: ok / EOPNOTSUPP
  Offload-->>DPIF: providers attached
  DPIF->>Offload: port_add(dpif, netdev, port_no) [on each port]
  Offload->>Provider: port_add(...)

Loading

sequenceDiagram
  autonumber
  participant PMD as dpif-netdev (pmd)
  participant OffMgr as dpif-offload (mgr)
  participant Prov as provider (e.g., rte_flow)
  participant ND as netdev
  participant CB as completion callback

  PMD->>OffMgr: datapath_flow_put(dpif_name, match, actions, ufid, in_port, ...)
  OffMgr->>Prov: netdev_flow_put(offload, ND, ..., ufid, flow_mark, orig_in_port)
  alt success
    Prov-->>OffMgr: stats, flow_mark, 0
    OffMgr->>CB: callback(stats, flow_mark, 0)
  else error
    Prov-->>OffMgr: -, INVALID_FLOW_MARK, err
    OffMgr->>CB: callback(NULL, INVALID_FLOW_MARK, err)
  end

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~180 minutes

Poem

A rabbit taps the kernel’s door,
“Bring me marks and flows galore!”
TC, rte_flow hop in line,
DPIF guides the grand design.
Ports all mapped, the stats now sing—
Offload’s spring has found its spring.
c(’.’)ɔ ᚛ swift bits on the wing ᚜

✨ 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_472717

---

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

[coderabbitai]

Actionable comments posted: 32

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)

lib/netdev-dummy.c (2)

1847-1853: netdev_dummy_receive: possible NULL deref on netdev_close().

When netdev_from_name() fails, you still call netdev_close(netdev).

-    netdev = netdev_from_name(argv[k++]);
-    if (!netdev || !is_dummy_netdev_class(netdev->netdev_class)) {
+    netdev = netdev_from_name(argv[k++]);
+    if (!netdev || !is_dummy_netdev_class(netdev->netdev_class)) {
         unixctl_command_reply_error(conn, "no such dummy netdev");
         goto exit_netdev;
     }
...
-exit_netdev:
-    netdev_close(netdev);
+exit_netdev:
+    if (netdev) {
+        netdev_close(netdev);
+    }

Also applies to: 1906-1907

---

1997-2011: netdev_dummy_conn_state: leak when device exists but isn’t dummy.

If netdev is non-dummy, you don’t close it.

-        if (netdev && is_dummy_netdev_class(netdev->netdev_class)) {
+        if (netdev && is_dummy_netdev_class(netdev->netdev_class)) {
             ...
             netdev_close(netdev);
-        }
+        } else if (netdev) {
+            netdev_close(netdev);
+        }

lib/dpif-netdev.c (1)

7723-7737: Potential NPE: last_rxq can be NULL in non-Rx entry paths

dp_netdev_hw_flow() dereferences pmd->ctx.last_rxq without a NULL check. While normal Rx sets it, other call sites or future refactors could hit this path with last_rxq unset.

Patch defensively:

 #ifdef ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */
-    struct dp_netdev_rxq *rxq = pmd->ctx.last_rxq;
-    bool miss_api_supported;
-    atomic_read_relaxed(&rxq->port->netdev->hw_info.miss_api_supported,
-                        &miss_api_supported);
-    if (miss_api_supported) {
-        int err = dpif_offload_netdev_hw_miss_packet_recover(rxq->port->netdev,
-                                                             packet);
+    struct dp_netdev_rxq *rxq = pmd->ctx.last_rxq;
+    if (rxq) {
+        bool miss_api_supported;
+        atomic_read_relaxed(&rxq->port->netdev->hw_info.miss_api_supported,
+                            &miss_api_supported);
+        if (miss_api_supported) {
+            int err = dpif_offload_netdev_hw_miss_packet_recover(
+                rxq->port->netdev, packet);
         if (err && err != EOPNOTSUPP) {
             COVERAGE_INC(datapath_drop_hw_miss_recover);
             return -1;
         }
+        }
     }
 #endif

lib/netdev-offload-tc.c (2)

100-106: Fix function signature mismatch and add missing parameters.

The function declaration shows a struct dpif * parameter, but there's no corresponding dpif field in the tc_offload_info structure that would be passed to recursive calls. This creates an inconsistency in the recursive function invocations.

Add the missing dpif field to struct tc_offload_info:

struct tc_offload_info {
+   struct dpif *dpif;
    // ... other fields
};

Or if tc_offload_info already contains a reference to dpif through another field, ensure it's properly accessed and passed in recursive calls.

---

969-979: Potential issue with uninitialized outport when ifindex is 0.

When action->out.ifindex_out is 0, the code attempts to use an uninitialized outport variable (it's initialized to ODPP_NONE but the logic doesn't handle the zero case properly before the lookup).

The logic should be restructured to handle the zero ifindex case more explicitly:

case TC_ACT_OUTPUT: {
    odp_port_t outport = ODPP_NONE;

    if (action->out.ifindex_out) {
        outport = netdev_tc_get_port_id_by_ifindex(
            netdev, action->out.ifindex_out);
        if (outport == ODPP_NONE) {
            return -ENOENT;
        }
+   } else {
+       /* Handle the case where ifindex_out is 0 */
+       return -EINVAL;
    }
    nl_msg_put_u32(buf, OVS_ACTION_ATTR_OUTPUT, odp_to_u32(outport));
}

lib/netdev-offload-dpdk.c (3)

74-78: Make per-thread counters safe: use atomics and remember the array size.

The counters are updated concurrently and from threads different than the creator; plain uint64_t is racy and there’s no stored bound for tid checks.

Proposed struct changes:

 struct netdev_offload_dpdk_data {
   struct cmap ufid_to_rte_flow;
-  uint64_t *rte_flow_counters;
+  atomic_uint64_t *rte_flow_counters;
+  unsigned int n_counters;
   struct ovs_mutex map_lock;
 };

---

933-939: Fix per-thread flow counter updates: bounds + atomic add.

• Potential OOB write if tid >= offload_thread_count.
• Non-atomic ++ under concurrency can race.

-        unsigned int tid = rte_flow_offload_thread_id();
+        unsigned int tid = rte_flow_offload_thread_id();
         data = (struct netdev_offload_dpdk_data *)
             ovsrcu_get(void *, &netdev->hw_info.offload_data);
-        data->rte_flow_counters[tid]++;
+        if (OVS_UNLIKELY(!data || tid >= data->n_counters)) {
+            VLOG_WARN_RL(&rl, "%s: invalid offload tid %u (n=%u), using 0",
+                         netdev_get_name(netdev),
+                         tid, data ? data->n_counters : 0);
+            tid = 0;
+        }
+        atomic_fetch_add_explicit(&data->rte_flow_counters[tid], 1,
+                                  memory_order_relaxed);

---

2389-2395: Decrement the creator’s bucket, not the current thread; make it atomic.

Using the current tid skews per-thread counts and can go negative; also needs bounds checks.

-        unsigned int tid = rte_flow_offload_thread_id();
+        unsigned int tid = rte_flow_data->creation_tid;
         data = (struct netdev_offload_dpdk_data *)
             ovsrcu_get(void *, &physdev->hw_info.offload_data);
-        data->rte_flow_counters[tid]--;
+        if (OVS_UNLIKELY(!data || tid >= data->n_counters)) {
+            VLOG_WARN_RL(&rl, "%s: invalid creation tid %u (n=%u) on destroy",
+                         netdev_get_name(physdev),
+                         tid, data ? data->n_counters : 0);
+            tid = 0;
+        }
+        atomic_fetch_sub_explicit(&data->rte_flow_counters[tid], 1,
+                                  memory_order_relaxed);

♻️ Duplicate comments (2)

tests/dpif-netdev.at (2)

479-483: Same readiness wait tweak applies here.

---

556-560: Same readiness wait tweak applies here.

🧹 Nitpick comments (74)

include/openvswitch/netdev.h (1)

104-107: Clarify ownership/NULL semantics for netdev_custom_stats.label.
Please document in the struct comment that:

• label may be NULL,
• when non-NULL it must be heap-allocated (xstrdup) and will be freed by netdev_free_custom_stats_counters().
  This prevents provider leaks and confusion for consumers.

lib/netdev-dpdk.h (1)

35-36: Document or rename check_only parameter in netdev_dpdk_flow_api_supported to clarify side-effect semantics
Add a brief doc comment in lib/netdev-dpdk.h above the signature, for example:

/* Returns true if the DPDK flow API is supported.
 * @param netdev         The network device.
 * @param check_only     true to query support without side effects;
 *                       false to allow clearing steer flags and reconfigure.
 */
bool netdev_dpdk_flow_api_supported(struct netdev *, bool check_only);

Alternatively, rename check_only to allow_side_effects for clearer intent.

include/openvswitch/json.h (1)

125-127: Const-correctness for json_object_is_empty().
The function does not mutate; take a const pointer.

Apply:

-struct json *json_object_create(void);
-bool json_object_is_empty(struct json *);
+struct json *json_object_create(void);
+bool json_object_is_empty(const struct json *);

lib/json.c (1)

400-405: Match const-correctness with header; keep NULL-safe behavior.
Adopt const input to align with declaration and avoid accidental mutation.

Apply:

-bool
-json_object_is_empty(struct json *json)
+bool
+json_object_is_empty(const struct json *json)
 {
     return json && json->type == JSON_OBJECT
            && shash_is_empty(json->object);
 }

Documentation/topics/testing.rst (1)

366-384: Clarify VF list format and prerequisites.

• Spell out the expected value format as “space-separated PFPCI,VF-Index pairs” and that exactly six entries are required.
• Note whether VFs can span multiple PFs or must share a PF (the example uses a single PF).
• Mention root privileges/hugepages need (link to earlier DPDK prereqs) and supported NIC families if relevant.

Proposed wording tweak:

-These six VFs need to be passed as a list of PF PCI addresses with their
-corresponding VF indexes in the OVS_RTE_FLOW_VF_PCI_ADDRS variable.
+Provide six VFs as space-separated PFPCI,VF-Index pairs in OVS_RTE_FLOW_VF_PCI_ADDRS
+(e.g., 0000:17:00.0,0). Use exactly six pairs. VFs may [all share one PF | span PFs] (clarify here).

lib/dummy.h (1)

46-49: LGTM: exported helpers for dummy offload simulation.
Consider a short comment above each prototype explaining when to call them (tests only).

lib/netdev.c (1)

2427-2435: Block ID helper: define/document sentinel for “unknown”.
Returning 0 for “not available” can be ambiguous. Define a constant (e.g., NETDEV_BLOCK_ID_INVALID 0) in a public header and document it, or return UINT32_MAX.

If you prefer a constant:

+/* include/openvswitch/netdev-provider.h (or similar) */
+#define NETDEV_BLOCK_ID_INVALID ((uint32_t)0)

Then reference it here and in providers.

tests/system-dpdk.at (3)

874-882: Avoid racy reads before provider/port become visible.

Right after adding br0/p1, add a short wait for dpif/offload to report the provider to reduce flakes on slower CI.

 AT_CHECK([ovs-vsctl add-port br0 p1 \
   -- set Interface p1 type=dpdk options:dpdk-devargs=net_null0,no-rx=1],
   [], [stdout], [stderr])

+OVS_WAIT_UNTIL([ovs-appctl dpif/offload/show | grep -q "netdev@ovs-netdev"])

---

883-890: Don’t hard-code port_no (2); normalize it to avoid brittle tests.

The assigned port_no can vary; compare after normalizing it.

-AT_CHECK([ovs-appctl dpif/offload/show], [0], [dnl
+AT_CHECK([ovs-appctl dpif/offload/show | sed -E 's/(port_no: )[[0-9]]+/\1<N>/' ], [0], [dnl
 Globally enabled: false
 Datapaths:
 netdev@ovs-netdev:
   rte_flow
-  - p1: port_no: 2
+  - p1: port_no: <N>
 ])

---

891-902: Make JSON assertion resilient to dynamic port number.

Normalize the numeric field so the check doesn't depend on the exact value.

-AT_CHECK([ovs-appctl --format json --pretty dpif/offload/show], [0], [dnl
+AT_CHECK([ovs-appctl --format json --pretty dpif/offload/show \
+  | sed -E 's/"port_no": [[0-9]]+/"port_no": <N>/' ], [0], [dnl
 {
   "enabled": false,
   "netdev@ovs-netdev": {
     "priority": [[
       "rte_flow"]],
     "providers": {
       "rte_flow": {
         "ports": {
           "p1": {
-            "port_no": 2}}}}}}
+            "port_no": <N>}}}}}}
 ])

lib/dpif.h (2)

444-444: Make dpif_get_features take a const dpif pointer.

The accessor does not modify dpif; const improves API clarity and usage.

-uint32_t dpif_get_features(struct dpif *dpif);
+uint32_t dpif_get_features(const struct dpif *dpif);

Support change in lib/dpif.c (for reference):

-uint32_t
-dpif_get_features(struct dpif *dpif)
+uint32_t
+dpif_get_features(const struct dpif *dpif)
 {
     return dpif->dpif_class->get_features
                ? dpif->dpif_class->get_features(dpif)
                : 0;
 }

---

524-527: Preserve source compatibility for the field rename.

Renaming netdev_flows->offloaded_flows can break out-of-tree users including tests or tooling. Provide a temporary alias to ease transition.

 struct dpif_flow_dump_types {
     bool ovs_flows;
     bool offloaded_flows;
 };
+
+/* Back-compat alias for older code. */
+#define netdev_flows offloaded_flows

Please verify all in-tree references were updated.

tests/ofproto-macros.at (1)

130-131: Broaden ‘used:’ normalization to also match integer values.

Current pattern requires a decimal point; some outputs may print integers.

-sed -E 's/used:[[0-9]]+\.[[0-9]]*/used:0.0/'
+sed -E 's/used:[[0-9]]+(\.[[0-9]]*)?/used:0.0/'

tests/system-rte-offloads-macros.at (7)

21-26: Precheck: accept “at least 6” VFs and ensure helper is callable.

Requiring exactly 6 is brittle. Also, calling the helper is fine, but keep quoting consistent.

Apply:

-  AT_SKIP_IF(
-    [test "$(printf '%s' "$OVS_RTE_FLOW_VF_PCI_ADDRS" | wc -w)" -ne 6])
+  AT_SKIP_IF(
+    [test "$(printf '%s' "$OVS_RTE_FLOW_VF_PCI_ADDRS" | wc -w)" -lt 6])
 
-  AT_SKIP_IF([! ovs_rte_flow_verify_vf_cfg "$OVS_RTE_FLOW_VF_PCI_ADDRS"])
+  AT_SKIP_IF([! ovs_rte_flow_verify_vf_cfg "$OVS_RTE_FLOW_VF_PCI_ADDRS"])

---

91-96: ADD_VF: quoting and iface-id consistency.

Quote variable expansions; keep iface-id stable but avoid accidental word-splitting.

-      AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \
-                set interface ovs-$1 external-ids:iface-id="$1" -- \
-                set interface ovs-$1 type=dpdk -- \
-                set interface ovs-$1 \
-                  options:dpdk-devargs=$PF_PCI,representor=vf$VF_IDX])
+      AT_CHECK([ovs-vsctl add-port "$3" "ovs-$1" -- \
+                set interface "ovs-$1" external-ids:iface-id="$1" -- \
+                set interface "ovs-$1" type=dpdk -- \
+                set interface "ovs-$1" \
+                  options:dpdk-devargs="$PF_PCI,representor=vf$VF_IDX"])

---

96-107: ADD_VF: minor quoting and default MAC generation.

Quote args to avoid globbing.

-      NS_CHECK_EXEC([$2], [ip addr add $4 dev $1 $7])
+      NS_CHECK_EXEC([$2], [ip addr add "$4" dev "$1" $7])
-      NS_CHECK_EXEC([$2], [ip link set dev $1 up])
+      NS_CHECK_EXEC([$2], [ip link set dev "$1" up])
-      if test -n "$5"; then
-        NS_CHECK_EXEC([$2], [ip link set dev $1 address $5])
+      if test -n "$5"; then
+        NS_CHECK_EXEC([$2], [ip link set dev "$1" address "$5"])
       else
-        NS_CHECK_EXEC([$2],
-                      [ip link set dev $1 address 02:00:00:00:EC:0$PORT_NO])
+        NS_CHECK_EXEC([$2],
+                      [ip link set dev "$1" address "02:00:00:00:EC:0$PORT_NO"])
       fi
-      if test -n "$6"; then
-        NS_CHECK_EXEC([$2], [ip route add default via $6])
+      if test -n "$6"; then
+        NS_CHECK_EXEC([$2], [ip route add default via "$6"])
       fi
-      on_exit "ip netns exec $2 ip link set $1 netns 1 \
-               && ip link set $1 name $REP"
+      on_exit "ip netns exec \"$2\" ip link set \"$1\" netns 1 \
+               && ip link set \"$1\" name \"$REP\""

---

117-121: Flow-dump normalizer: make sed regex case-insensitive.

DP dump may emit uppercase hex; use -i to normalize both.

-  grep 'eth_type(0x0800)' \
-    | sed -e 's/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/recirc_id(0),//' \
+  grep -i 'eth_type(0x0800)' \
+    | sed -e 's/[eE][tT][hH](src=[[a-fA-F0-9:]]*,dst=[[a-fA-F0-9:]]*)/eth(macs)/;s/recirc_id(0),//' \

---

128-131: Nit: typos in comments.

“give” → “given”.

-# Check if the give PF PCI address and the VF number are valid.
+# Check if the given PF PCI address and the VF number are valid.

---

145-151: Nit: function name typo in comment.

‘ovr_rte…’ → ‘ovs_rte…’.

-# ovr_rte_get_representor_netdev(<PF_PCI>, <VF_INDEX>)
+# ovs_rte_get_representor_netdev(<PF_PCI>, <VF_INDEX>)

---

177-193: Helper: log index and representor safely.

Tiny nit: quote vars to avoid word-splitting; harmless but cleaner.

-        echo "ovs-p$i: PF PCI $PCI_ID with VF $VF_IDX has representor $REP"
-        i=$((i + 1))
+        echo "ovs-p$i: PF PCI $PCI_ID with VF $VF_IDX has representor $REP"
+        i=$((i + 1))

(No functional change; just a heads-up—current output is fine.)

lib/netdev-dummy.c (1)

98-104: Dead code: struct offloaded_flow is now unused.

Remove to avoid confusion and future drift.

-struct offloaded_flow {
-    struct hmap_node node;
-    ovs_u128 ufid;
-    struct match match;
-    uint32_t mark;
-};

vswitchd/bridge.c (1)

3400-3403: Confirm init order: dpif_offload_set_global_cfg before dpdk_init may be fragile.

dpif_offload_set_global_cfg() triggers dp_offload_initialize() on first call. If any offload provider (e.g., rte_flow) expects DPDK EAL to be initialized, calling this before dpdk_init() could cause partial/failed provider setup. Consider moving the cfg push after dpdk_init()/userspace_tso_init(), or confirm providers are EAL-independent at init time.

Proposed reorder:

-    if (cfg && ovsdb_idl_get_seqno(idl) != idl_seqno) {
-        dpif_offload_set_global_cfg(cfg);
-    }
-
     if (cfg) {
         dpdk_init(&cfg->other_config);
         userspace_tso_init(&cfg->other_config);
     }
+
+    if (cfg && ovsdb_idl_get_seqno(idl) != idl_seqno) {
+        dpif_offload_set_global_cfg(cfg);
+    }

If rte_flow (or other providers) do not require EAL at provider registration time, leaving the current order is fine—please confirm with a quick runtime smoke test (hw-offload=true, rte providers present) that providers are detected as expected.

tests/dpif-netdev.at (3)

45-47: LGTM; consider minor cleanup.

Pattern and normalization look fine. Optionally collapse sort | uniq to sort -u for brevity and speed.

-    grep 'dpif_offload_dummy.*flow del' | sed 's/.*|DBG|//' \
-      | sed -E 's/mark: [0-9]+/mark: 1/' | sort | uniq
+    grep 'dpif_offload_dummy.*flow del' | sed 's/.*|DBG|//' \
+      | sed -E 's/mark: [0-9]+/mark: 1/' | sort -u

---

49-52: LGTM; same optional sort tweak as above.

-    grep 'dpif_offload_dummy.*: packet:.*with mark' | sed 's/.*|DBG|//' \
-      | sed -E 's/mark: [0-9]+/mark: 1/' | sort | uniq
+    grep 'dpif_offload_dummy.*: packet:.*with mark' | sed 's/.*|DBG|//' \
+      | sed -E 's/mark: [0-9]+/mark: 1/' | sort -u

---

417-421: Stabilize readiness wait for hw-offload enablement.

Good to wait on enablement; to reduce flakes, make grep quiet and tolerant to tiny message wording changes (e.g., punctuation/case).

-   OVS_WAIT_UNTIL([grep "hw-offload API Enabled" ovs-vswitchd.log])
+   OVS_WAIT_UNTIL([grep -qi "hw-.*offload.*API.*Enabled" ovs-vswitchd.log])

tests/system-offloads-traffic.at (2)

1175-1186: Avoid gawk-specific ‘asort’ to keep tests portable.

asort() is a gawk extension and may not exist in mawk/nawk. Prefer POSIX sh + coreutils to sort only the “- brX …” lines.

-sort_dpif_offload_show () {
-    awk '
-        /^  -/ { dashlines[++n] = $0; next }
-        { print }
-        END {
-            asort(dashlines)
-            for (i=1; i<=n; i++) print dashlines[i]
-        }
-    ' | sed -E 's/ifindex: [0-9]+/ifindex: 0/g'
-}
+sort_dpif_offload_show () {
+    # Print non-dash lines as-is; collect dash lines and append them sorted.
+    tmp="$(mktemp)"; trap 'rm -f "$tmp"' RETURN
+    awk '/^  -/{print > "'"$tmp"'"; next} {print}'
+    sort "$tmp" | sed -E 's/ifindex: [0-9]+/ifindex: 0/g'
+}

---

1188-1194: Gate on offload readiness to prevent race.

Right after daemon start, provider listing may be incomplete. Add a wait similar to other tests.

-AT_CHECK([ovs-appctl dpif/offload/show | sort_dpif_offload_show], [0], [dnl
+OVS_WAIT_UNTIL([grep -qi "hw-.*offload.*API.*Enabled" ovs-vswitchd.log])
+AT_CHECK([ovs-appctl dpif/offload/show | sort_dpif_offload_show], [0], [dnl

lib/netdev-dpdk.c (1)

6523-6555: Signature change verified
All declarations and call sites have been updated to the new two-parameter signature.

Optional refactor

• Make this function side-effect free by moving the rx-steering fallback into configuration paths.
• If retaining the WARN here, switch to rate-limited logging (e.g., VLOG_WARN_RL) to avoid noisy output when called frequently.

vswitchd/vswitch.xml (2)

257-274: Fix typos and clarify provider-list semantics (global hw-offload-priority).

• Typo: “The The argument...” → “The argument...”.
• Wording: “implementations are missing” → “providers are missing”.
• Recommend documenting case-sensitivity, whitespace handling, and behavior for unknown names for consistency with the per-port key.

Apply this diff within the block:

-          to try when multiple exist for a given datapath implementation.  The
-          The argument provided should be a list of comma-separated hardware
-          offload provider names.
+          to try when multiple exist for a given datapath implementation.
+          The argument should be a comma-separated list of hardware
+          offload provider names.

-          If implementations are missing from the list, they are added at the
-          end in undefined order.
+          If providers present on the system are missing from the list,
+          they are appended at the end in an undefined order.

+        <p>
+          Provider names are case-sensitive. Unknown provider names are
+          ignored. Leading/trailing whitespace around names is ignored.
+        </p>

---

2593-2616: Tighten wording and mirror global semantics (per-port hw-offload-priority).

• Keep semantics identical to the global key where possible (case-sensitivity, whitespace, unknown names).
• Minor wording polish.

Apply this diff within the block:

-          providers to try on this port.  The argument should be a
-          comma-separated list of hardware offload provider names, or the
-          word <code>none</code>.
+          providers to try on this port. The argument should be a
+          comma-separated list of provider names, or the word
+          <code>none</code>.

+        <p>
+          Provider names are case-sensitive. Unknown provider names are
+          ignored. Leading/trailing whitespace around names is ignored.
+        </p>

-          The default value is <code>none</code>, i.e., not set, which uses the
-          global
+          The default is to inherit the global setting from
           <ref table="Open_vSwitch" column="other_config" key="hw-offload-priority"/>
-          configuration. Changing this value after offload enablement requires
+          when this key is not set. Changing this value after offload enablement requires
           restarting the daemon.

lib/dpif-offload-rte_flow-private.h (3)

23-31: Scope note: keep these APIs private.

Consider adding a brief comment that this header is not installed and is only for internal provider use (build-system already enforces this, but the note helps avoid accidental exposure).

---

17-18: Nit: align comment style with tree.

OVS headers typically place the brief file description above the include guard. Optional clean-up.

---

27-31: Signature consistency for callback.

Style nit: name the callback parameters for readability in tooling.

-    bool (*cb)(struct netdev *, odp_port_t, void *), void *aux);
+    bool (*cb)(struct netdev *netdev, odp_port_t port_no, void *aux), void *aux);

lib/dpif-offload-dummy.c (5)

69-74: Use a better UFID hash to avoid clustering.

Hashing only ufid->u32[0] increases collisions. Use OVS hash_bytes() over the whole ufid.

Apply:

+#include "hash.h"
 static uint32_t
 dpif_offload_dummy_flow_hash(const ovs_u128 *ufid)
 {
-    return ufid->u32[0];
+    return hash_bytes(ufid, sizeof *ufid, 0);
 }

---

467-469: Operation string inconsistent with actual action.

You compute a local 'modify' flag but log using put->modify, which may not reflect the performed action. Use the local flag for logging.

Apply:

-    dpif_offload_dummy_log_operation(put->modify ? "modify" : "add", error,
+    dpif_offload_dummy_log_operation(modify ? "modify" : "add", error,
                                      put->ufid);

---

374-380: Improve log phrasing.

“succeed to %s” is awkward. Use “Successfully %s” / “Failed to %s”.

Apply:

-    VLOG_DBG("%s to %s netdev flow "UUID_FMT,
-             error == 0 ? "succeed" : "failed", op,
+    VLOG_DBG("%s %s netdev flow "UUID_FMT,
+             error == 0 ? "Successfully" : "Failed to", op,
              UUID_ARGS((struct uuid *) ufid));

---

595-601: Avoid mutating caller-provided flow during debug logging.

This mutates ‘flow’ (if provided) just for formatting. Copy locally to prevent side effects.

Apply:

-                /* 'flow' does not contain proper port number here.
-                 * Let's just clear it as it's wildcarded anyway. */
-                flow->in_port.ofp_port = 0;
-                flow_format(&ds, flow, NULL);
+                /* 'flow' may not have a proper port. Use a local copy for logging. */
+                struct flow flow_copy = *flow;
+                flow_copy.in_port.ofp_port = 0;
+                flow_format(&ds, &flow_copy, NULL);

---

367-371: Minor: simplify boolean return.

Directly return the predicate.

Apply:

-    return is_dummy_netdev_class(netdev->netdev_class) ? true : false;
+    return is_dummy_netdev_class(netdev->netdev_class);

ofproto/ofproto-dpif.c (3)

6706-6719: Text renderer: stable provider ordering (optional).

For reproducible output, consider sorting providers by name before printing.

---

6720-6753: JSON shape is sensible; minor suggestion: deterministic provider order.

Consider sorting when building “priority” to keep outputs stable across runs.

---

6756-6788: New unixctl command LGTM; add minimal docs (optional).

Command is well-integrated. Consider adding a short man fragment for dpif/offload/show to mirror other dpif/* commands.

lib/netdev-offload-dpdk.h (1)

35-38: Clarify recover() return semantics in the declaration comment.

netdev_offload_dpdk_hw_miss_packet_recover() in the C file returns 0, or EOPNOTSUPP when not applicable. Document that here to avoid misuse by callers expecting negative errno.

lib/dpif-offload-tc.c (3)

198-200: Tweak debug message grammar.

Minor readability nit: drop the trailing space and make the sentence complete.

Apply:

-    VLOG_DBG("Datapath %s recirculation id sharing ",
-             offload_tc->recirc_id_shared ? "supports" : "does not support");
+    VLOG_DBG("Datapath %s recirculation ID sharing.",
+             offload_tc->recirc_id_shared ? "supports" : "does not support");

---

349-358: flow_flush: OK, but consider partial-failure reporting.

You return the first error only. If helpful for troubleshooting, aggregate errors or log per-port failures at DBG level.

---

612-645: Output-port existence check only covers OVS_ACTION_ATTR_OUTPUT.

If other actions emit packets (e.g., USERSPACE, RECIRC to different in_port, or tunnel actions), consider whether they also need validation here to avoid partial HW programming inconsistencies.

Would you like a follow-up patch to extend validation to other action types used by tc offload?

lib/dpif-netlink.c (3)

37-37: Include order inconsistency

The new header dpif-offload.h is included between dpif-netlink-rtnl.h and dpif-provider.h, disrupting the alphabetical ordering of includes within this group.

 #include "bitmap.h"
 #include "dpif-netlink-rtnl.h"
-#include "dpif-offload.h"
 #include "dpif-provider.h"
+#include "dpif-offload.h"
 #include "fat-rwlock.h"

---

124-126: New function declaration without implementation context

The function dpif_netlink_get_features is declared here but its implementation at lines 882-889 is trivial (just returns dpif->user_features). Consider if this getter warrants a separate function or could be inlined.

---

882-889: Consider inlining this trivial getter

This function simply returns a member variable. Unless there's a specific need for ABI stability or future extensibility, consider making this an inline function in the header or a macro.

lib/netdev-offload-tc.h (1)

38-39: Consider more descriptive comment formatting

The comment about tc_modify_flow_deleted could be clearer. Consider rewording for better readability.

-    bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success
-                                  * to delete the original flow. */
+    bool tc_modify_flow_deleted; /* Indicates whether the original flow was
+                                  * successfully deleted during tc flow
+                                  * modification. */

lib/dpif-offload-rte_flow.c (2)

204-206: Missing null check for megaflow lookup

The warning message is logged when a megaflow is not found, but the function continues to return INVALID_FLOW_MARK. While this is safe, consider if this warrants a more severe log level or additional error handling.

---

583-585: FIXME comment needs attention

The comment indicates that time_now might be delayed by 64ms due to thread scheduling. This could affect latency measurements and timeouts.

This timing issue could significantly impact latency statistics. Would you like me to help implement a more accurate timing mechanism using time_usec() directly?

lib/netdev-offload-tc.c (3)

831-842: Add null pointer checks for defensive programming.

The function dereferences in_netdev->dpif_offload without verifying that in_netdev is not NULL. While the caller might guarantee this, adding a defensive check would prevent potential crashes.

static odp_port_t
netdev_tc_get_port_id_by_ifindex(const struct netdev *in_netdev, int ifindex)
{
+   if (!in_netdev) {
+       return ODPP_NONE;
+   }
+
    const struct dpif_offload *offload = ovsrcu_get(
        const struct dpif_offload *, &in_netdev->dpif_offload);

---

2319-2324: Fix inconsistent function naming pattern.

The function is named netdev_offload_tc_flow_put but should include the dpif parameter consistently with the new offload architecture pattern seen elsewhere in the file.

Consider renaming to match the pattern used in other functions:

int
-netdev_offload_tc_flow_put(struct dpif *dpif, struct netdev *netdev,
+dpif_offload_tc_flow_put(struct dpif *dpif, struct netdev *netdev,

---

3370-3376: Document the FIXME for future resolution.

The FIXME comment indicates a known issue where meters configured before offload is enabled won't be offloaded. This should be tracked properly.

Would you like me to create a GitHub issue to track this bug where meter_id is not offloaded if it was configured before offload was enabled?

lib/dpif-offload.h (1)

50-51: Document the special meaning of INVALID_FLOW_MARK.

The macro INVALID_FLOW_MARK is defined as 0, but there's no documentation explaining why 0 is considered invalid or what valid flow marks should be.

+/* Flow mark value 0 is reserved to indicate an invalid or unset flow mark.
+ * Valid flow marks must be non-zero. */
 #define INVALID_FLOW_MARK 0

lib/dpif.c (1)

365-366: Check return value of dpif_offload_attach_providers.

The function dpif_offload_attach_providers returns an integer error code, but it's being ignored. While this might be intentional to allow dpif to work without offload, it would be better to at least log warnings on failure.

-        dpif_offload_attach_providers(dpif);
+        int offload_err = dpif_offload_attach_providers(dpif);
+        if (offload_err) {
+            VLOG_WARN("Failed to attach offload providers to %s: %s",
+                      dpif_name(dpif), ovs_strerror(offload_err));
+        }

lib/netdev-offload-dpdk.c (2)

2592-2609: Flush respects creation_tid; clarify and consider full flush mode.

Only destroying flows whose creation_tid matches the current tid is intentional, but surprising. Add a comment and optionally a flag to force full flush (all tids) when needed (e.g., device removal).

Would you like a follow-up patch to add a “flush_all_threads” parameter and wire it from the caller paths?

---

865-873: Use %zu for size_t in logs.

raw_encap->size is size_t; %ld is incorrect on ILP32 and UB-prone.

-            ds_put_format(s_extra, "Raw-encap size=%ld set raw_encap 0 raw "
+            ds_put_format(s_extra, "Raw-encap size=%zu set raw_encap 0 raw "
                           "pattern is ", raw_encap->size);

lib/dpif-offload.c (7)

241-260: Provider open loop: handle partial failures deterministically.

Open errors are logged but we still push any successes into a temp list; good. Consider sorting provider_list deterministically (e.g., by class->type) before applying the priority list to avoid order variance across hash iteration.

---

263-290: Priority parsing: validate tokens once and de-dup.

If the priority list contains duplicates or unknown providers, you may attach the same provider twice or log multiple warns elsewhere. Normalize tokens with a set before the inner loop.

I can provide a small helper to parse and validate the comma list into a shash for O(1) lookups.

---

374-389: Set-config fanout is best-effort; note error handling policy.

Currently, provider set_config errors are ignored. Either document this “best-effort” policy or aggregate/log first error, similar to flow_dump_destroy.

---

688-770: Global cfg: heap-alloc priority string leaks the original static pointer.

Not harmful but inconsistent. Free/reuse previous heap value if it was already heap-allocated.

-    dpif_offload_provider_priority_list = xstrdup(priority);
+    free(dpif_offload_provider_priority_list);
+    dpif_offload_provider_priority_list = xstrdup(priority);

---

1287-1363: Operate path: only HW-only providers are called. Confirm intended.

This skips mixed providers. If intentional, add a comment; otherwise consider allowing providers that can do both.

---

1434-1446: Datapath flow put: clarify INVALID_FLOW_MARK contract.

The function sets INVALID_FLOW_MARK when no provider is found. Document that callers must ignore flow_mark in that case to avoid stale usage.

---

1552-1563: Port manager: missing ovsrcu cleanup on mgr destruction.

Maps are destroyed, but if any ports remain due to logic bugs, freeing will be deferred; add a VLOG_ERR or assert message hint to aid debugging (you assert counts, good). Consider documenting ownership of dpif_offload_port_mgr_port memory (caller frees after remove()).

lib/dpif-offload-provider.h (6)

54-61: Mark dpif_offload as opaque in the header and minimize exposed fields.

The comment says “should be treated as opaque,” yet exposes fields. Consider moving internals to the C file and keeping only what providers must see.

---

100-121: Clarify init/open/close threading expectations.

Callers use RCU and mutexes; add a note that providers must be thread-safe for open/close with respect to dpif lifecycle and should avoid blocking long.

---

196-233: Flow dump contract: mandate dpif_offload_flow_dump_init/thread_init.

You mention it in comments; consider making helpers mandatory by providing default inline wrappers providers must call, or validate in the core with OVS_ASSERTs.

---

246-274: Meter APIs: specify units and behavior on partial HW support.

Document units (kbps/pps vs bytes/packets) and expected errno for unsupported ops to keep providers consistent.

---

341-371: Port manager API: lifetime/ownership notes.

Add explicit ownership rules:

• add(): mgr takes a netdev ref.
• remove(): returns port for caller to free or reuse; whether mgr drops netdev ref depends on keep_netdev_ref.

---

372-390: Public helpers: consider const where applicable.

dpif_offload_set_config could take const dpif*? (mutating providers yes, but dpif pointer itself is not mutated). Minor, but improves API clarity.

lib/dpif-netdev-avx512.c (1)

114-114: Prefer bool for offload gating.

dpif_offload_is_offload_enabled() returns bool; store it as bool for clarity and to avoid implicit int/bool mixing.

-    const uint32_t hwol_enabled = dpif_offload_is_offload_enabled();
+    const bool hwol_enabled = dpif_offload_is_offload_enabled();

lib/netdev.h (1)

220-220: Public API addition LGTM; consider documenting semantics.

netdev_get_block_id(struct netdev *) is fine and implemented defensively (returns 0 when unsupported). Add a brief comment here about when a non-zero block_id is expected (e.g., Linux LAG primary) to guide providers/users.

lib/netdev-linux.c (1)

3021-3028: Gate matchall policer on provider capability, not just global “offload enabled”.

Switching from per-netdev flow API gating to global dpif_offload_is_offload_enabled() may trigger tc matchall policer programming even when the active offload provider isn’t tc. This can be surprising for configurations using a different provider (e.g., rte_flow) alongside system ports.

Recommendation:

• Also check the active offload provider/capability before using the tc path, or keep a per-netdev capability check. If a provider/caps query API exists, use it; otherwise, please confirm this behavior is intended across providers.

Would you like me to propose a small helper (e.g., dpif_offload_supports_matchall_policer()) and wire it here?

📜 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 4f16390.

📒 Files selected for processing (50)

• Documentation/topics/testing.rst (1 hunks)
• include/openvswitch/json.h (1 hunks)
• include/openvswitch/netdev.h (1 hunks)
• lib/automake.mk (3 hunks)
• lib/dpctl.c (3 hunks)
• lib/dpdk.c (0 hunks)
• lib/dpif-netdev-avx512.c (2 hunks)
• lib/dpif-netdev-private-flow.h (0 hunks)
• lib/dpif-netdev.c (24 hunks)
• lib/dpif-netlink.c (10 hunks)
• lib/dpif-offload-dummy.c (1 hunks)
• lib/dpif-offload-provider.h (1 hunks)
• lib/dpif-offload-rte_flow-private.h (1 hunks)
• lib/dpif-offload-rte_flow.c (1 hunks)
• lib/dpif-offload-tc.c (1 hunks)
• lib/dpif-offload.c (1 hunks)
• lib/dpif-offload.h (1 hunks)
• lib/dpif-provider.h (5 hunks)
• lib/dpif.c (20 hunks)
• lib/dpif.h (2 hunks)
• lib/dummy.h (2 hunks)
• lib/json.c (1 hunks)
• lib/netdev-dpdk.c (5 hunks)
• lib/netdev-dpdk.h (1 hunks)
• lib/netdev-dummy.c (7 hunks)
• lib/netdev-linux.c (2 hunks)
• lib/netdev-offload-dpdk.c (35 hunks)
• lib/netdev-offload-dpdk.h (1 hunks)
• lib/netdev-offload-provider.h (0 hunks)
• lib/netdev-offload-tc.c (33 hunks)
• lib/netdev-offload-tc.h (1 hunks)
• lib/netdev-offload.c (1 hunks)
• lib/netdev-offload.h (0 hunks)
• lib/netdev-provider.h (1 hunks)
• lib/netdev.c (1 hunks)
• lib/netdev.h (1 hunks)
• ofproto/ofproto-dpif-upcall.c (9 hunks)
• ofproto/ofproto-dpif.c (4 hunks)
• tests/.gitignore (1 hunks)
• tests/automake.mk (7 hunks)
• tests/dpif-netdev.at (4 hunks)
• tests/ofproto-dpif.at (2 hunks)
• tests/ofproto-macros.at (3 hunks)
• tests/system-dpdk.at (1 hunks)
• tests/system-offloads-traffic.at (1 hunks)
• tests/system-rte-offloads-macros.at (1 hunks)
• tests/system-rte-offloads-testsuite.at (1 hunks)
• tests/system-rte-offloads.at (1 hunks)
• vswitchd/bridge.c (2 hunks)
• vswitchd/vswitch.xml (2 hunks)

🔥 Files not summarized due to errors (1)

• tests/ofproto-dpif.at: Error: Server error: no LLM provider could handle the message

💤 Files with no reviewable changes (4)

• lib/dpdk.c
• lib/dpif-netdev-private-flow.h
• lib/netdev-offload-provider.h
• lib/netdev-offload.h

👮 Files not reviewed due to content moderation or server errors (1)

• tests/ofproto-dpif.at

🧰 Additional context used

🧬 Code graph analysis (29)

lib/dummy.h (2)

lib/netdev-dummy.c (1)

• is_dummy_netdev_class (203-207)

lib/dpif-offload-dummy.c (1)

• dpif_offload_dummy_netdev_simulate_offload (561-615)

lib/dpif-netdev-avx512.c (1)

lib/dpif-offload.c (1)

• dpif_offload_is_offload_enabled (463-470)

lib/netdev.h (1)

lib/netdev.c (1)

• netdev_get_block_id (2427-2435)

lib/dpif.h (1)

lib/dpif.c (1)

• dpif_get_features (555-561)

lib/dpif-offload-rte_flow-private.h (1)

lib/dpif-offload-rte_flow.c (3)

• rte_flow_offload_thread_id (119-139)
• dpif_offload_rte_get_netdev (770-783)
• dpif_offload_rte_traverse_ports (656-667)

lib/dpif-offload-dummy.c (6)

lib/util.h (1)

• ovs_u128_equals (536-540)

lib/dpif-offload.c (11)

• dpif_offload_set_netdev_offload (481-486)
• dpif_offload_port_mgr_add (1667-1703)
• dpif_offload_is_offload_enabled (463-470)
• dpif_offload_port_mgr_remove (1638-1665)
• dpif_offload_port_mgr_find_by_odp_port (1621-1636)
• dpif_offload_port_mgr_init (1551-1563)
• dpif_offload_port_mgr_traverse_ports (1705-1718)
• dpif_offload_name (404-408)
• dpif_offload_port_mgr_find_by_netdev (1601-1619)
• dpif_offload_allocate_flow_mark (1369-1388)
• dpif_offload_class_type (410-414)

lib/ovs-thread.c (2)

• ovs_mutex_init (219-223)
• ovsthread_once_done (515-525)

lib/odp-util.c (1)

• odp_format_ufid (4706-4710)

lib/flow.c (2)

• flow_extract (668-680)
• flow_equal_except (2254-2269)

lib/dp-packet.h (1)

• dp_packet_set_flow_mark (1040-1045)

lib/netdev-dpdk.h (1)

lib/netdev-dpdk.c (1)

• netdev_dpdk_flow_api_supported (6522-6555)

lib/json.c (1)

lib/shash.c (1)

• shash_is_empty (95-99)

lib/netdev-offload-tc.h (2)

lib/netdev-offload-tc.c (13)

• netdev_offload_tc_init (3178-3254)
• netdev_offload_tc_flow_flush (566-586)
• netdev_offload_tc_flow_dump_create (588-619)
• netdev_offload_tc_flow_dump_destroy (620-628)
• netdev_offload_tc_flow_dump_next (1367-1417)
• netdev_offload_tc_flow_put (2319-2739)
• netdev_offload_tc_flow_del (2798-2811)
• netdev_offload_tc_flow_get (2741-2796)
• dpif_offload_tc_meter_init (3163-3176)
• dpif_offload_tc_meter_set (3360-3421)
• dpif_offload_tc_meter_get (3423-3449)
• dpif_offload_tc_meter_del (3451-3480)
• dpif_offload_tc_flow_get_n_offloaded (2813-2824)

lib/dpif-offload-tc.c (1)

• dpif_offload_tc_get_port_id_by_ifindex (830-843)

include/openvswitch/json.h (1)

lib/json.c (1)

• json_object_is_empty (400-405)

lib/netdev-dpdk.c (1)

lib/dpif-offload.c (1)

• dpif_offload_is_offload_enabled (463-470)

lib/dpif-offload-rte_flow.c (11)

lib/dpif-offload-provider.h (1)

• dpif_offload_assert_class (391-396)

lib/util.c (6)

• get_subprogram_name (636-641)
• xzalloc (165-170)
• xnanosleep (2421-2427)
• xcalloc (158-163)
• xmalloc (172-177)
• xstrdup (203-207)

lib/cmap.c (2)

• cmap_insert (854-870)
• cmap_init (242-246)

lib/util.h (2)

• ovs_u128_equals (536-540)
• nullable_memcpy (203-209)

lib/mpsc-queue.c (3)

• mpsc_queue_insert (243-251)
• mpsc_queue_tail (207-227)
• mpsc_queue_init (118-126)

lib/ovs-atomic.h (4)

• atomic_count_inc64 (487-495)
• atomic_count_inc (451-459)
• atomic_count_dec64 (497-505)
• atomic_count_init (445-449)

lib/netdev-offload-dpdk.c (11)

• netdev_offload_dpdk_flow_del (2487-2503)
• netdev_offload_dpdk_flow_put (2434-2485)
• netdev_offload_dpdk_flow_flush (2627-2638)
• netdev_offload_dpdk_init (2505-2523)
• OVS_UNUSED (1372-1394)
• OVS_UNUSED (2171-2213)
• netdev_offload_dpdk_uninit (2525-2531)
• netdev_offload_dpdk_flow_get_n_offloaded (2793-2812)
• netdev_offload_dpdk_flow_get_n_offloaded_by_thread (2814-2827)
• netdev_offload_dpdk_hw_miss_packet_recover (2703-2791)
• netdev_offload_dpdk_flow_get (2533-2589)

lib/dpif-offload.h (1)

• dpif_offload_datapath_flow_op_continue (204-212)

lib/dpif-offload.c (8)

• dpif_offload_allocate_flow_mark (1369-1388)
• dpif_offload_free_flow_mark (1390-1398)
• dpif_offload_is_offload_enabled (463-470)
• dpif_offload_set_netdev_offload (481-486)
• dpif_offload_port_mgr_add (1667-1703)
• dpif_offload_port_mgr_find_by_odp_port (1621-1636)
• dpif_offload_port_mgr_remove (1638-1665)
• dpif_offload_port_mgr_traverse_ports (1705-1718)

lib/ovs-thread.c (5)

• ovs_barrier_block (364-391)
• ovs_thread_create (455-501)
• ovs_barrier_init (333-345)
• ovs_barrier_destroy (348-356)
• ovsthread_once_done (515-525)

lib/mov-avg.h (6)

• mov_avg_cma_update (49-59)
• mov_avg_cma_init (43-47)
• mov_avg_ema_init (132-137)
• mov_avg_cma (61-65)
• mov_avg_ema (159-163)
• mov_avg_ema_std_dev (165-169)

lib/netdev-linux.c (1)

lib/dpif-offload.c (1)

• dpif_offload_is_offload_enabled (463-470)

lib/netdev-dummy.c (2)

lib/netdev.c (1)

• netdev_get_class (2105-2109)

lib/dpif-offload-dummy.c (1)

• dpif_offload_dummy_netdev_simulate_offload (561-615)

lib/dpif-offload.h (2)

lib/dpif-offload.c (35)

• dpif_offload_set_global_cfg (688-770)
• dpif_offload_is_offload_enabled (463-470)
• dpif_offload_is_offload_rebalance_policy_enabled (472-479)
• dpif_offload_allocate_flow_mark (1369-1388)
• dpif_offload_free_flow_mark (1390-1398)
• dpif_offload_init (393-402)
• dpif_offload_attach_providers (313-330)
• dpif_offload_detach_providers (355-373)
• dpif_offload_name (404-408)
• dpif_offload_class_type (410-414)
• dpif_offload_get_debug (416-425)
• dpif_offload_flow_flush (772-792)
• dpif_offload_dump_start (622-628)
• dpif_offload_dump_next (630-680)
• dpif_offload_dump_done (682-686)
• dpif_offload_flow_get_n_offloaded (825-843)
• dpif_offload_flow_get_n_offloaded_by_impl (845-865)
• dpif_offload_meter_set (867-892)
• dpif_offload_meter_get (894-916)
• dpif_offload_meter_del (918-940)
• dpif_offload_get_netdev_by_port_id (1246-1258)
• dpif_offload_offload_get_netdev_by_port_id (1214-1223)
• dpif_offload_port_offloaded_by (602-620)
• dpif_offload_netdevs_out_of_resources (1260-1281)
• dpif_offload_get_impl_type (794-798)
• dpif_offload_get_impl_type_by_class (800-823)
• dpif_offload_port_dump_start (1154-1166)
• dpif_offload_port_dump_next (1168-1202)
• dpif_offload_port_dump_done (1204-1212)
• dpif_offload_stats_get (427-461)
• dpif_offload_netdev_same_offload (1401-1411)
• dpif_offload_netdev_hw_miss_packet_recover (1516-1548)
• dpif_offload_datapath_flow_put (1413-1446)
• dpif_offload_datapath_flow_del (1448-1481)
• dpif_offload_datapath_flow_stats (1483-1514)

lib/dpif.c (1)

• dpif_name (478-482)

vswitchd/bridge.c (2)

lib/ovsdb-idl.c (1)

• ovsdb_idl_get_seqno (558-562)

lib/dpif-offload.c (1)

• dpif_offload_set_global_cfg (688-770)

ofproto/ofproto-dpif.c (6)

lib/dynamic-string.c (3)

• ds_put_format (137-145)
• ds_cstr (341-349)
• ds_destroy (368-372)

lib/dpif.c (1)

• dpif_name (478-482)

lib/dpif-offload.c (3)

• dpif_offload_class_type (410-414)
• dpif_offload_get_debug (416-425)
• dpif_offload_is_offload_enabled (463-470)

lib/json.c (6)

• json_object_create (391-398)
• json_object_put (423-427)
• json_array_create_empty (225-234)
• json_array_add (236-278)
• json_string_create (187-202)
• json_boolean_create (172-176)

lib/unixctl.c (4)

• unixctl_command_get_output_format (221-225)
• unixctl_command_reply_json (291-296)
• unixctl_command_reply (265-282)
• unixctl_command_register (198-219)

lib/shash.c (2)

• shash_sort (291-313)
• shash_count (101-106)

lib/dpctl.c (3)

lib/dpif-offload.c (1)

• dpif_offload_stats_get (427-461)

utilities/ovs-dpctl.c (1)

• dpctl_print (50-55)

lib/netdev.c (1)

• netdev_free_custom_stats_counters (2413-2425)

lib/dpif-offload-provider.h (1)

lib/dpif-offload.c (24)

• dpif_offload_port_mgr_init (1551-1563)
• dpif_offload_port_mgr_add (1667-1703)
• dpif_offload_port_mgr_remove (1638-1665)
• dpif_offload_port_mgr_uninit (1565-1579)
• dpif_offload_port_mgr_port_count (1720-1724)
• dpif_offload_port_mgr_find_by_ifindex (1581-1599)
• dpif_offload_port_mgr_find_by_netdev (1601-1619)
• dpif_offload_port_mgr_find_by_odp_port (1621-1636)
• dpif_offload_port_mgr_traverse_ports (1705-1718)
• dpif_offload_port_mgr_port_dump_start (1733-1758)
• dpif_offload_port_mgr_port_dump_next (1760-1783)
• dpif_offload_port_mgr_port_dump_done (1785-1794)
• dp_offload_initialize (138-174)
• dpif_offload_set_config (375-390)
• dpif_offload_port_add (511-563)
• dpif_offload_port_del (565-583)
• dpif_offload_port_set_config (585-600)
• dpif_offload_set_netdev_offload (481-486)
• dpif_offload_flow_dump_create (948-982)
• dpif_offload_flow_dump_destroy (992-1014)
• dpif_offload_flow_dump_next (1047-1073)
• dpif_offload_flow_dump_thread_create (1078-1100)
• dpif_offload_flow_dump_thread_destroy (1105-1117)
• dpif_offload_operate (1286-1363)

ofproto/ofproto-dpif-upcall.c (2)

lib/dpif-offload.c (6)

• dpif_offload_flow_get_n_offloaded_by_impl (845-865)
• dpif_offload_netdevs_out_of_resources (1260-1281)
• dpif_offload_is_offload_rebalance_policy_enabled (472-479)
• dpif_offload_get_netdev_by_port_id (1246-1258)
• dpif_offload_get_impl_type_by_class (800-823)
• dpif_offload_flow_get_n_offloaded (825-843)

lib/netlink.c (1)

• nl_attr_get_odp_port (777-781)

lib/netdev-offload-dpdk.h (1)

lib/netdev-offload-dpdk.c (9)

• netdev_offload_dpdk_init (2505-2523)
• netdev_offload_dpdk_uninit (2525-2531)
• netdev_offload_dpdk_flow_flush (2627-2638)
• netdev_offload_dpdk_flow_get_n_offloaded (2793-2812)
• netdev_offload_dpdk_flow_get_n_offloaded_by_thread (2814-2827)
• netdev_offload_dpdk_hw_miss_packet_recover (2703-2791)
• netdev_offload_dpdk_flow_put (2434-2485)
• netdev_offload_dpdk_flow_del (2487-2503)
• netdev_offload_dpdk_flow_get (2533-2589)

lib/dpif-provider.h (2)

lib/dpif-offload.c (2)

• dpif_offload_flow_dump_create (948-982)
• dpif_offload_flow_dump_thread_create (1078-1100)

lib/ovs-thread.c (1)

• ovs_mutex_init (219-223)

lib/dpif-netdev.c (7)

lib/util.c (1)

• xasprintf (367-378)

lib/cmap.c (3)

• cmap_init (242-246)
• cmap_destroy (252-261)
• cmap_insert (854-870)

lib/ovs-thread.c (1)

• ovs_mutex_init (219-223)

lib/hash.h (1)

• hash_int (407-410)

lib/cmap.h (1)

• cmap_remove (291-295)

lib/dpif-offload.c (6)

• dpif_offload_free_flow_mark (1390-1398)
• dpif_offload_is_offload_enabled (463-470)
• dpif_offload_datapath_flow_del (1448-1481)
• dpif_offload_datapath_flow_stats (1483-1514)
• dpif_offload_datapath_flow_put (1413-1446)
• dpif_offload_netdev_hw_miss_packet_recover (1516-1548)

lib/dpif-provider.h (1)

• dpif_flow_dump_init (80-93)

lib/dpif-netlink.c (4)

lib/dpif-provider.h (1)

• dpif_flow_dump_init (80-93)

lib/ofpbuf.c (1)

• ofpbuf_new (165-171)

lib/netlink-socket.c (2)

• nl_dump_start (1167-1182)
• nl_dump_done (1305-1338)

include/openvswitch/ofpbuf.h (1)

• ofpbuf_delete (171-177)

lib/dpif.c (3)

lib/dpif-offload.c (16)

• dp_offload_initialize (138-174)
• dpif_offload_attach_providers (313-330)
• dpif_offload_detach_providers (355-373)
• dpif_offload_port_add (511-563)
• dpif_offload_port_del (565-583)
• dpif_offload_port_set_config (585-600)
• dpif_offload_flow_flush (772-792)
• dpif_offload_flow_dump_destroy (992-1014)
• dpif_offload_flow_dump_thread_destroy (1105-1117)
• dpif_offload_flow_dump_next (1047-1073)
• dpif_offload_is_offload_enabled (463-470)
• dpif_offload_operate (1286-1363)
• dpif_offload_set_config (375-390)
• dpif_offload_meter_set (867-892)
• dpif_offload_meter_get (894-916)
• dpif_offload_meter_del (918-940)

lib/util.c (1)

• xmemdup (186-192)

lib/dpif-netdev.c (1)

• dpif_is_netdev (608-612)

lib/netdev-offload-tc.c (3)

lib/dpif-offload.c (4)

• dpif_offload_class_type (410-414)
• dpif_offload_get_netdev_by_port_id (1246-1258)
• dpif_offload_netdev_same_offload (1401-1411)
• dpif_offload_is_offload_enabled (463-470)

lib/dpif-offload-tc.c (1)

• dpif_offload_tc_get_port_id_by_ifindex (830-843)

include/openvswitch/hmap.h (1)

• hmap_count (261-265)

lib/netdev-offload-dpdk.c (4)

lib/dpif-offload-rte_flow.c (3)

• rte_flow_offload_thread_id (119-139)
• dpif_offload_rte_get_netdev (770-783)
• dpif_offload_rte_traverse_ports (656-667)

lib/dpif-offload.c (1)

• dpif_offload_netdev_same_offload (1401-1411)

lib/netdev-vport.c (1)

• netdev_vport_is_vport_class (79-83)

lib/netdev-dpdk.c (1)

• netdev_dpdk_flow_api_supported (6522-6555)

lib/dpif-offload.c (10)

lib/shash.c (6)

• shash_find (234-238)
• shash_add (128-132)
• shash_sort (291-313)
• shash_count (101-106)
• shash_find_data (248-253)
• shash_find_and_delete (255-266)

lib/ovs-thread.c (1)

• ovsthread_once_done (515-525)

include/openvswitch/list.h (7)

• ovs_list_push_back (202-206)
• ovs_list_init (147-151)
• ovs_list_remove (249-255)
• ovs_list_size (305-315)
• ovs_list_back (293-301)
• ovs_list_is_empty (318-322)
• ovs_list_front (281-289)

lib/ovs-atomic.h (3)

• ovs_refcount_ref (539-547)
• ovs_refcount_init (529-533)
• ovs_refcount_unref_relaxed (659-668)

lib/dpif.c (6)

• dpif_normalize_type (510-514)
• dpif_type (493-497)
• dpif_name (478-482)
• log_flow_put_message (1829-1856)
• log_flow_del_message (1858-1870)
• log_flow_get_message (1926-1939)

lib/netdev.c (4)

• netdev_get_name (1086-1090)
• netdev_close (602-610)
• netdev_ref (477-490)
• netdev_get_ifindex (1171-1179)

lib/smap.c (4)

• smap_get (197-201)
• smap_get_bool (223-232)
• smap_clear (185-193)
• smap_add (51-57)

lib/ovs-thread.h (1)

• ovsthread_id_self (465-475)

lib/id-fpool.c (3)

• id_fpool_create (139-163)
• id_fpool_new_id (185-243)
• id_fpool_free_id (245-278)

lib/cmap.c (4)

• cmap_init (242-246)
• cmap_count (264-268)
• cmap_destroy (252-261)
• cmap_insert (854-870)

lib/dpif-offload-tc.c (11)

lib/dpif-offload-provider.h (3)

• dpif_offload_assert_class (391-396)
• dpif_offload_flow_dump_init (68-74)
• dpif_offload_flow_dump_thread_init (80-86)

lib/netdev-offload-tc.c (13)

• netdev_offload_tc_init (3178-3254)
• dpif_offload_tc_meter_init (3163-3176)
• netdev_offload_tc_flow_flush (566-586)
• netdev_offload_tc_flow_dump_create (588-619)
• netdev_offload_tc_flow_dump_next (1367-1417)
• netdev_offload_tc_flow_dump_destroy (620-628)
• netdev_offload_tc_flow_put (2319-2739)
• netdev_offload_tc_flow_del (2798-2811)
• netdev_offload_tc_flow_get (2741-2796)
• dpif_offload_tc_flow_get_n_offloaded (2813-2824)
• dpif_offload_tc_meter_set (3360-3421)
• dpif_offload_tc_meter_get (3423-3449)
• dpif_offload_tc_meter_del (3451-3480)

lib/netdev.c (3)

• netdev_get_name (1086-1090)
• netdev_close (602-610)
• netdev_get_dpif_type (2121-2125)

lib/dpif-offload.c (12)

• dpif_offload_port_mgr_add (1667-1703)
• dpif_offload_is_offload_enabled (463-470)
• dpif_offload_port_mgr_remove (1638-1665)
• dpif_offload_port_mgr_port_dump_start (1733-1758)
• dpif_offload_port_mgr_port_dump_next (1760-1783)
• dpif_offload_port_mgr_port_dump_done (1785-1794)
• dpif_offload_port_mgr_find_by_odp_port (1621-1636)
• dpif_offload_init (393-402)
• dpif_offload_port_mgr_init (1551-1563)
• dpif_offload_port_mgr_traverse_ports (1705-1718)
• dpif_offload_port_mgr_port_count (1720-1724)
• dpif_offload_is_offload_rebalance_policy_enabled (472-479)

lib/dpif.c (1)

• dpif_get_features (555-561)

lib/tc.c (1)

• tc_set_policy (4026-4045)

lib/odp-util.c (1)

• parse_key_and_mask_to_match (7733-7785)

lib/netlink.c (4)

• nl_attr_get (645-650)
• nl_attr_get_size (653-658)
• nl_attr_type (638-642)
• nl_attr_get_odp_port (777-781)

lib/ofpbuf.c (4)

• ofpbuf_use_stack (79-83)
• ofpbuf_init (138-142)
• ofpbuf_uninit (145-153)
• ofpbuf_put (423-433)

lib/flow.c (1)

• flow_get_tunnel_netdev (3719-3739)

lib/netdev-offload.c (1)

• netdev_set_hw_info (55-74)

[coderabbitai]

🛠️ Refactor suggestion

Bug: wrong provider selection for dump types (may fetch/print wrong sets and duplicate work)

non_offloaded and partially-offloaded should not enable offloaded_flows. Partially-offloaded (dp_layer=ovs, offloaded=true) must be fetched via ovs_flows. Suggest:

• ovs_flows: ovs || non_offloaded || partially_offloaded
• offloaded_flows: tc || dpdk || offloaded

Apply this diff:

-    dpif_dump_types->ovs_flows = dump_types->ovs || dump_types->non_offloaded;
-    dpif_dump_types->offloaded_flows = dump_types->tc || dump_types->offloaded
-                                       || dump_types->non_offloaded
-                                       || dump_types->dpdk
-                                       || dump_types->partially_offloaded;
+    dpif_dump_types->ovs_flows =
+        dump_types->ovs || dump_types->non_offloaded || dump_types->partially_offloaded;
+    dpif_dump_types->offloaded_flows =
+        dump_types->tc || dump_types->dpdk || dump_types->offloaded;

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

Suggested change

	dpif_dump_types->offloaded_flows = dump_types->tc || dump_types->offloaded
	|| dump_types->non_offloaded
	|| dump_types->dpdk
	|| dump_types->partially_offloaded;
	}
	dpif_dump_types->ovs_flows =
	dump_types->ovs || dump_types->non_offloaded || dump_types->partially_offloaded;
	dpif_dump_types->offloaded_flows =
	dump_types->tc || dump_types->dpdk || dump_types->offloaded;
	}

🤖 Prompt for AI Agents

In lib/dpctl.c around lines 968 to 972, the current logic incorrectly sets
dpif_dump_types->offloaded_flows when non_offloaded or partially_offloaded are
true, causing wrong provider selection and duplicate work; change the assignment
so offloaded_flows is true only when dump_types->tc || dump_types->dpdk ||
dump_types->offloaded, and set ovs_flows to dump_types->ovs ||
dump_types->non_offloaded || dump_types->partially_offloaded; update the boolean
expressions accordingly so partially_offloaded and non_offloaded do not enable
offloaded_flows but do enable ovs_flows.

[coderabbitai]

🛠️ Refactor suggestion

Memory leak on error path; minor cleanups

• If dpif_offload_stats_get() returns error after allocating/partially filling stats, stats_array must be freed (and per-entry counters freed for 0..n_stats-1).
• Inside the loop, ‘stats’ is never NULL; the check is redundant.

Apply this diff:

@@
-    error = dpif_offload_stats_get(dpif, &stats_array, &n_stats);
-    if (error) {
-        dpctl_error(dpctl_p, error, "retrieving offload statistics");
-        goto close_dpif;
-    }
+    error = dpif_offload_stats_get(dpif, &stats_array, &n_stats);
+    if (error) {
+        dpctl_error(dpctl_p, error, "retrieving offload statistics");
+        if (stats_array) {
+            for (size_t i = 0; i < n_stats; i++) {
+                netdev_free_custom_stats_counters(&stats_array[i]);
+            }
+            free(stats_array);
+        }
+        goto close_dpif;
+    }
@@
-    for (size_t i = 0; i < n_stats; i++) {
-        struct netdev_custom_stats *stats = &stats_array[i];
-
-        if (!stats) {
-            continue;
-        }
+    for (size_t i = 0; i < n_stats; i++) {
+        struct netdev_custom_stats *stats = &stats_array[i];

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

Suggested change

	struct netdev_custom_stats *stats_array;
	struct dpif *dpif;
	size_t n_stats;
	int error;
	size_t i;
	error = opt_dpif_open(argc, argv, dpctl_p, 2, &dpif);
	if (error) {
	return error;
	}
	memset(&stats, 0, sizeof(stats));
	error = dpif_offload_stats_get(dpif, &stats);
	error = dpif_offload_stats_get(dpif, &stats_array, &n_stats);
	if (error) {
	dpctl_error(dpctl_p, error, "retrieving offload statistics");
	goto close_dpif;
	}
	if (stats_array && n_stats) {
	qsort(stats_array, n_stats, sizeof *stats_array, compare_custom_stats);
	}
	dpctl_print(dpctl_p, "HW Offload stats:\n");
	for (i = 0; i < stats.size; i++) {
	dpctl_print(dpctl_p, " %s: %6" PRIu64 "\n",
	stats.counters[i].name, stats.counters[i].value);
	for (size_t i = 0; i < n_stats; i++) {
	struct netdev_custom_stats *stats = &stats_array[i];
	if (!stats) {
	continue;
	}
	if (stats->label) {
	dpctl_print(dpctl_p, " %s:\n", stats->label);
	}
	for (size_t j = 0; j < stats->size; j++) {
	dpctl_print(dpctl_p, " %s: %6" PRIu64 "\n",
	stats->counters[j].name, stats->counters[j].value);
	}
	netdev_free_custom_stats_counters(stats);
	}
	netdev_free_custom_stats_counters(&stats);
	free(stats_array);
	error = dpif_offload_stats_get(dpif, &stats_array, &n_stats);
	if (error) {
	dpctl_error(dpctl_p, error, "retrieving offload statistics");
	if (stats_array) {
	for (size_t i = 0; i < n_stats; i++) {
	netdev_free_custom_stats_counters(&stats_array[i]);
	}
	free(stats_array);
	}
	goto close_dpif;
	}
	if (stats_array && n_stats) {
	qsort(stats_array, n_stats, sizeof *stats_array, compare_custom_stats);
	}
	dpctl_print(dpctl_p, "HW Offload stats:\n");
	for (size_t i = 0; i < n_stats; i++) {
	struct netdev_custom_stats *stats = &stats_array[i];
	if (stats->label) {
	dpctl_print(dpctl_p, " %s:\n", stats->label);
	}
	for (size_t j = 0; j < stats->size; j++) {
	dpctl_print(dpctl_p, " %s: %6" PRIu64 "\n",
	stats->counters[j].name, stats->counters[j].value);
	}
	netdev_free_custom_stats_counters(stats);
	}
	free(stats_array);

[coderabbitai]

⚠️ Potential issue

Bug: use-after-invalidate of flow->mark causes false “last ref” and wrong provider del/free

offload_flow_del() calls mark_to_flow_disassociate(), which sets flow->mark = INVALID_FLOW_MARK, then immediately checks flow_mark_has_no_ref(dp, flow->mark). That check is always against INVALID_FLOW_MARK instead of the original mark, so the provider delete path and mark free can run prematurely or incorrectly.

Fix by preserving the original mark and using it for the “no-ref” test and follow-up:

 static void
 offload_flow_del(struct dp_netdev_pmd_thread *pmd, struct dp_netdev_flow *flow)
 {
-    odp_port_t in_port = flow->flow.in_port.odp_port;
-
-    if (flow->mark == INVALID_FLOW_MARK) {
+    odp_port_t in_port = flow->flow.in_port.odp_port;
+    /* Preserve the original mark before disassociate. */
+    uint32_t orig_mark = flow->mark;
+
+    if (orig_mark == INVALID_FLOW_MARK) {
         return;
     }
 
-    mark_to_flow_disassociate(pmd->dp, flow);
+    mark_to_flow_disassociate(pmd->dp, flow);
 
-    if (flow_mark_has_no_ref(pmd->dp, flow->mark)
-        && dpif_offload_is_offload_enabled()) {
+    if (flow_mark_has_no_ref(pmd->dp, orig_mark)
+        && dpif_offload_is_offload_enabled()) {
         struct dpif_offload_flow_del del = {
             .in_port = in_port,
             .ufid = CONST_CAST(ovs_u128 *, &flow->mega_ufid),
             .stats = NULL,
             .cb_data.callback = offload_flow_del_resume_cb,
         };
-        uint32_t mark;
+        uint32_t returned_mark = INVALID_FLOW_MARK;
         int ret;
 
-        ret = dpif_offload_datapath_flow_del(pmd->dp->full_name, &del, &mark);
+        ret = dpif_offload_datapath_flow_del(pmd->dp->full_name, &del,
+                                             &returned_mark);
         if (ret && ret != EINPROGRESS) {
             VLOG_DBG("Failed removing offload flow ufid "UUID_FMT
                      " from port %d",
                      UUID_ARGS((struct uuid *)&flow->mega_ufid), in_port);
         }
-        offload_flow_del_resume(ret, mark);
+        /* Fall back to orig_mark if provider didn’t return one. */
+        offload_flow_del_resume(ret,
+                                returned_mark != INVALID_FLOW_MARK
+                                    ? returned_mark
+                                    : orig_mark);
     }
 }

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

Suggested change

	offload_flow_del(struct dp_netdev_pmd_thread *pmd, struct dp_netdev_flow *flow)
	{
	struct dp_offload_thread_item *item;
	struct dp_offload_flow_item *flow_offload;
	odp_port_t in_port = flow->flow.in_port.odp_port;
	if (!netdev_is_flow_api_enabled()) {
	if (flow->mark == INVALID_FLOW_MARK) {
	return;
	}
	item = dp_netdev_alloc_flow_offload(pmd->dp, flow, op);
	flow_offload = &item->data->flow;
	flow_offload->match = *match;
	flow_offload->actions = xmalloc(actions_len);
	memcpy(flow_offload->actions, actions, actions_len);
	flow_offload->actions_len = actions_len;
	flow_offload->orig_in_port = flow->orig_in_port;
	mark_to_flow_disassociate(pmd->dp, flow);
	if (flow_mark_has_no_ref(pmd->dp, flow->mark)
	&& dpif_offload_is_offload_enabled()) {
	struct dpif_offload_flow_del del = {
	.in_port = in_port,
	.ufid = CONST_CAST(ovs_u128 *, &flow->mega_ufid),
	.stats = NULL,
	.cb_data.callback = offload_flow_del_resume_cb,
	};
	uint32_t mark;
	int ret;
	item->timestamp = pmd->ctx.now;
	dp_netdev_offload_flow_enqueue(item);
	ret = dpif_offload_datapath_flow_del(pmd->dp->full_name, &del, &mark);
	if (ret && ret != EINPROGRESS) {
	VLOG_DBG("Failed removing offload flow ufid "UUID_FMT
	" from port %d",
	UUID_ARGS((struct uuid *)&flow->mega_ufid), in_port);
	}
	offload_flow_del_resume(ret, mark);
	}
	}
	static void
	offload_flow_del(struct dp_netdev_pmd_thread *pmd, struct dp_netdev_flow *flow)
	{
	odp_port_t in_port = flow->flow.in_port.odp_port;
	/* Preserve the original mark before disassociate. */
	uint32_t orig_mark = flow->mark;
	if (orig_mark == INVALID_FLOW_MARK) {
	return;
	}
	mark_to_flow_disassociate(pmd->dp, flow);
	if (flow_mark_has_no_ref(pmd->dp, orig_mark)
	&& dpif_offload_is_offload_enabled()) {
	struct dpif_offload_flow_del del = {
	.in_port = in_port,
	.ufid = CONST_CAST(ovs_u128 *, &flow->mega_ufid),
	.stats = NULL,
	.cb_data.callback = offload_flow_del_resume_cb,
	};
	uint32_t returned_mark = INVALID_FLOW_MARK;
	int ret;
	ret = dpif_offload_datapath_flow_del(pmd->dp->full_name, &del,
	&returned_mark);
	if (ret && ret != EINPROGRESS) {
	VLOG_DBG("Failed removing offload flow ufid " UUID_FMT
	" from port %d",
	UUID_ARGS((struct uuid *)&flow->mega_ufid), in_port);
	}
	/* Fall back to orig_mark if provider didn’t return one. */
	offload_flow_del_resume(ret,
	returned_mark != INVALID_FLOW_MARK
	? returned_mark
	: orig_mark);
	}
	}

[coderabbitai]

⚠️ Potential issue

Bug: leaking/freeing wrong mark on put error; success check uses truthiness

On error you test flow->mark (which can still be INVALID) instead of the provider-returned flow_mark, risking a leaked mark. On success you check if (flow_mark) which is unsafe if INVALID_FLOW_MARK is 0. Use explicit comparisons to INVALID_FLOW_MARK.

Apply:

 static void
 offload_flow_put_resume(struct dp_netdev *dp,
                         struct dp_netdev_flow *flow, uint32_t flow_mark,
                         int error)
 {
     if (error == EINPROGRESS) {
         return;
     }
 
-    if (!error) {
-        if (flow_mark && flow->mark == INVALID_FLOW_MARK) {
+    if (!error) {
+        if (flow_mark != INVALID_FLOW_MARK &&
+            flow->mark == INVALID_FLOW_MARK) {
             mark_to_flow_associate(dp, flow_mark, flow);
         }
     } else {
         /* On error, no flow should be associated with this flow mark,
          * and we should free it. */
-        if (flow->mark != INVALID_FLOW_MARK) {
-            mark_to_flow_disassociate_all(dp, flow_mark);
-            dpif_offload_free_flow_mark(flow_mark);
-        }
+        if (flow_mark != INVALID_FLOW_MARK) {
+            /* Disassociate any accidental users, then free the id. */
+            mark_to_flow_disassociate_all(dp, flow_mark);
+            dpif_offload_free_flow_mark(flow_mark);
+        }
     }
     dp_netdev_flow_unref(flow);
 }

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

Suggested change

	static void
	offload_flow_put_resume(struct dp_netdev *dp,
	struct dp_netdev_flow *flow, uint32_t flow_mark,
	int error)
	{
	if (error == EINPROGRESS) {
	return;
	}
	if (!error) {
	if (flow_mark && flow->mark == INVALID_FLOW_MARK) {
	mark_to_flow_associate(dp, flow_mark, flow);
	}
	} else {
	/* On error, no flow should be associated with this flow mark,
	* and we should free it. */
	if (flow->mark != INVALID_FLOW_MARK) {
	mark_to_flow_disassociate_all(dp, flow_mark);
	dpif_offload_free_flow_mark(flow_mark);
	}
	}
	dp_netdev_flow_unref(flow);
	}
	static void
	offload_flow_put_resume(struct dp_netdev *dp,
	struct dp_netdev_flow *flow, uint32_t flow_mark,
	int error)
	{
	if (error == EINPROGRESS) {
	return;
	}
	if (!error) {
	if (flow_mark != INVALID_FLOW_MARK &&
	flow->mark == INVALID_FLOW_MARK) {
	mark_to_flow_associate(dp, flow_mark, flow);
	}
	} else {
	/* On error, no flow should be associated with this flow mark,
	* and we should free it. */
	if (flow_mark != INVALID_FLOW_MARK) {
	/* Disassociate any accidental users, then free the id. */
	mark_to_flow_disassociate_all(dp, flow_mark);
	dpif_offload_free_flow_mark(flow_mark);
	}
	}
	dp_netdev_flow_unref(flow);
	}

🤖 Prompt for AI Agents

In lib/dpif-netdev.c around lines 3480 to 3503, the code uses truthiness and
checks flow->mark on error which can be INVALID_FLOW_MARK, risking
freeing/disassociating the wrong mark or leaking the provider-returned
flow_mark; change both success and error branches to use explicit comparisons
against INVALID_FLOW_MARK: on success use if (flow_mark != INVALID_FLOW_MARK) to
call mark_to_flow_associate(dp, flow_mark, flow); on error use if (flow_mark !=
INVALID_FLOW_MARK) to call mark_to_flow_disassociate_all(dp, flow_mark) and
dpif_offload_free_flow_mark(flow_mark). Ensure you no longer rely on flow->mark
in these checks.

[coderabbitai]

🛠️ Refactor suggestion

Missing header for ovsthread_once APIs.

File uses ovsthread_once_*; include the header to avoid implicit declarations on some compilers.

Apply:

 #include "util.h"
+#include "ovs-thread.h"
 #include "uuid.h"

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

Suggested change

	#include "dpif.h"
	#include "dpif-offload-provider.h"
	#include "dpif-offload.h"
	#include "dummy.h"
	#include "netdev-provider.h"
	#include "odp-util.h"
	#include "util.h"
	#include "uuid.h"
	#include "openvswitch/json.h"
	#include "openvswitch/match.h"
	#include "openvswitch/vlog.h"
	#include "dpif.h"
	#include "dpif-offload-provider.h"
	#include "dpif-offload.h"
	#include "dummy.h"
	#include "netdev-provider.h"
	#include "odp-util.h"
	#include "util.h"
	#include "ovs-thread.h"
	#include "uuid.h"
	#include "openvswitch/json.h"
	#include "openvswitch/match.h"
	#include "openvswitch/vlog.h"

🤖 Prompt for AI Agents

In lib/dpif-offload-dummy.c around lines 20 to 32, the file uses
ovsthread_once_* APIs but does not include the ovsthread header, which can lead
to implicit declaration warnings/errors; add #include "openvswitch/ovsthread.h"
(alongside the other openvswitch/* includes) so the ovsthread_once declarations
are available to the compiler.

[coderabbitai]

⚠️ Potential issue

ADD_VF: non-POSIX ‘[[’ usage and invalid “return” at top-level.

Tests run under /bin/sh; ‘[[’ is non-POSIX. Also, ‘return’ outside a function will fail. Use POSIX ‘test’ and ‘exit’.

-    [ USER_PORT=$1
-      PORT_NO=${USER_PORT##*[!0-9]}
-
-      AT_CHECK([[[ "$PORT_NO" -ge 0 ]] && [[ "$PORT_NO" -le 6 ]] || return 66])
+    [ USER_PORT=$1
+      PORT_NO=${USER_PORT##*[!0-9]}
+      AT_CHECK([
+        test -n "$PORT_NO" || exit 66
+        test "$PORT_NO" -ge 0 -a "$PORT_NO" -le 5 || exit 66
+      ])

Note: bounded to 0..5 (six ports) to match the precheck.

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

Suggested change

	m4_define([ADD_VF],
	[ USER_PORT=$1
	PORT_NO=${USER_PORT##*[!0-9]}
	AT_CHECK([[[ "$PORT_NO" -ge 0 ]] && [[ "$PORT_NO" -le 6 ]] || return 66])
	PORT_CFG=$(echo $OVS_RTE_FLOW_VF_PCI_ADDRS | cut -d' ' -f$((PORT_NO+1)))
	PF_PCI=$(ovs_rte_get_pci_id "$PORT_CFG")
	VF_IDX=$(ovs_rte_get_vf_idx "$PORT_CFG")
	REP=$(ovs_rte_get_representor_netdev $PF_PCI $VF_IDX) || return 77
	AT_CHECK([ip link set $REP name $1])
	AT_CHECK([ip link set $1 netns $2])
	[ USER_PORT=$1
	PORT_NO=${USER_PORT##*[!0-9]}
	AT_CHECK([
	test -n "$PORT_NO" || exit 66
	test "$PORT_NO" -ge 0 -a "$PORT_NO" -le 5 || exit 66
	])
	PORT_CFG=$(echo $OVS_RTE_FLOW_VF_PCI_ADDRS | cut -d' ' -f$((PORT_NO+1)))
	PF_PCI=$(ovs_rte_get_pci_id "$PORT_CFG")
	VF_IDX=$(ovs_rte_get_vf_idx "$PORT_CFG")
	REP=$(ovs_rte_get_representor_netdev $PF_PCI $VF_IDX) || return 77
	AT_CHECK([ip link set $REP name $1])
	AT_CHECK([ip link set $1 netns $2])

🤖 Prompt for AI Agents

In tests/system-rte-offloads-macros.at around lines 79–90, the script uses
non-POSIX [[ ... ]] and top-level return statements; replace the conditional
with POSIX tests and use exit instead of return (also tighten the upper bound to
5). Concretely, change the check to use [ "$PORT_NO" -ge 0 ] && [ "$PORT_NO" -le
5 ] || exit 66, and replace any top-level "|| return 77" (after REP assignment)
with "|| exit 77"; keep the rest of the logic intact and ensure POSIX-compatible
quoting/spacing for test commands.

[coderabbitai]

🛠️ Refactor suggestion

ADD_VF: representor lookup failure should skip, but ‘return 77’ won’t work here.

Emit exit 77 to mark the test as skipped.

-      REP=$(ovs_rte_get_representor_netdev $PF_PCI $VF_IDX) || return 77
+      REP=$(ovs_rte_get_representor_netdev "$PF_PCI" "$VF_IDX") || exit 77

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

Suggested change

	REP=$(ovs_rte_get_representor_netdev $PF_PCI $VF_IDX) || return 77
	AT_CHECK([ip link set $REP name $1])
	AT_CHECK([ip link set $1 netns $2])
	REP=$(ovs_rte_get_representor_netdev "$PF_PCI" "$VF_IDX") || exit 77
	AT_CHECK([ip link set $REP name $1])
	AT_CHECK([ip link set $1 netns $2])

🤖 Prompt for AI Agents

In tests/system-rte-offloads-macros.at around lines 87 to 90, the representor
lookup failure currently does "return 77" which does not properly mark the test
as skipped in this script; change the control flow so the script emits exit 77
instead of returning (i.e., replace the "return 77" path with an "exit 77" so
the test runner sees the skip status).

[coderabbitai]

⚠️ Potential issue

Representor detection is incorrect (returns the VF netdev, not the representor).

virtfn$VF_IDX/net lists the VF’s own netdev. We need the uplink representor that peers with the VF, typically identifiable by physfn match and phys_port_name “pfXvfY”.

Replace with physfn+phys_port_name based discovery:

-    VF_NET_DIR="/sys/bus/pci/devices/$PF_PCI/virtfn$VF_IDX/net"
-
-    if [[ ! -d "$VF_NET_DIR" ]]; then
-        echo "ERROR: VF $VF_IDX for PF $PF_PCI does not exist" >&2
-        return 1
-    fi
-
-    for iface in "$VF_NET_DIR"/*; do
-        if [[ -e "$iface" ]]; then
-            basename "$iface"
-            return 0
-        fi
-    done
-
-    echo "ERROR: No representor netdev found for VF $VF_IDX on PF $PF_PCI" >&2
+    PF_DIR="/sys/bus/pci/devices/$PF_PCI"
+    if [ ! -d "$PF_DIR" ]; then
+        echo "ERROR: PF $PF_PCI does not exist" >&2
+        return 1
+    fi
+    for iface in /sys/class/net/*; do
+        [ -d "$iface" ] || continue
+        # Must be a representor of this PF (device/physfn points to PF)
+        if [ -e "$iface/device/physfn" ] && \
+           [ "$(readlink -f "$iface/device/physfn")" = "$PF_DIR" ]; then
+            if [ -r "$iface/phys_port_name" ]; then
+                name=$(cat "$iface/phys_port_name")
+                case "$name" in
+                    pf*vf$VF_IDX)
+                        basename "$iface"
+                        return 0
+                        ;;
+                esac
+            fi
+        fi
+    done
+    echo "ERROR: No representor netdev found for VF $VF_IDX on PF $PF_PCI" >&2
     return 1

[coderabbitai]

🛠️ Refactor suggestion

Make the flow match resilient by stripping dynamic counters.

Matching packets/bytes/used is brittle. Normalize output before comparison and drop counters from the expected text.

Apply this diff:

-OVS_WAIT_UNTIL_EQUAL(
-  [ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
-   | DUMP_DP_IP_CLEAN_SORTED], [dnl
-in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
-in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p0])
+OVS_WAIT_UNTIL_EQUAL(
+  [ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
+   | DUMP_DP_IP_CLEAN_SORTED \
+   | sed -E 's/, packets:[0-9]+, bytes:[0-9]+, used:[0-9.]+s//g'], [dnl
+in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
+in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0])

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

Suggested change

	OVS_WAIT_UNTIL_EQUAL(
	[ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
	| DUMP_DP_IP_CLEAN_SORTED], [dnl
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p0])
	OVS_WAIT_UNTIL_EQUAL(
	[ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
	| DUMP_DP_IP_CLEAN_SORTED \
	| sed -E 's/, packets:[0-9]+, bytes:[0-9]+, used:[0-9.]+s//g'], [dnl
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0])

🤖 Prompt for AI Agents

In tests/system-rte-offloads.at around lines 30 to 35, the test currently
compares raw ovs-appctl dpctl/dump-flows output including dynamic counters
(packets:, bytes:, used:) which makes the match brittle; change the test to
normalize the output before comparison by stripping those dynamic counter fields
from the actual command output (e.g., pipe through sed/awk/perl to remove
"packets:..., bytes:..., used:..." tokens or replace them with fixed
placeholders) and update the expected text to omit the counters so the
OVS_WAIT_UNTIL_EQUAL compares only stable fields (in_port, eth, eth_type, ipv4,
actions).

[coderabbitai]

🛠️ Refactor suggestion

Same robustness fix for the 6-port scenario.

Drop dynamic fields from both actual and expected outputs.

Apply this diff:

-OVS_WAIT_UNTIL_EQUAL(
-  [ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
-   | DUMP_DP_IP_CLEAN_SORTED], [dnl
-in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
-...
-in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p4])
+OVS_WAIT_UNTIL_EQUAL(
+  [ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
+   | DUMP_DP_IP_CLEAN_SORTED \
+   | sed -E 's/, packets:[0-9]+, bytes:[0-9]+, used:[0-9.]+s//g'], [dnl
+in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
+in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
+in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
+in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
+in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
+in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
+in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
+in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
+in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
+in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
+in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
+in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
+in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
+in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
+in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
+in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
+in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
+in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
+in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
+in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
+in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
+in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
+in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
+in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
+in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
+in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
+in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
+in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
+in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
+in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4])

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

Suggested change

	OVS_WAIT_UNTIL_EQUAL(
	[ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
	| DUMP_DP_IP_CLEAN_SORTED], [dnl
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p2
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p3
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p4
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p5
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p0
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p2
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p3
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p4
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p5
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p0
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p3
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p4
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p5
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p0
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p2
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p4
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p5
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p0
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p2
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p3
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p5
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p0
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p1
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p2
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p3
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:196, used:0.0s, actions:ovs-p4])
	OVS_WAIT_UNTIL_EQUAL(
	[ovs-appctl dpctl/dump-flows --names type=dpdk,offloaded \
	| DUMP_DP_IP_CLEAN_SORTED \
	| sed -E 's/, packets:[0-9]+, bytes:[0-9]+, used:[0-9.]+s//g'], [dnl
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
	in_port(ovs-p0),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
	in_port(ovs-p1),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
	in_port(ovs-p2),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4
	in_port(ovs-p3),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
	in_port(ovs-p4),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p5
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p0
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p1
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p2
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p3
	in_port(ovs-p5),eth(macs),eth_type(0x0800),ipv4(frag=no), actions:ovs-p4])

🤖 Prompt for AI Agents

In tests/system-rte-offloads.at around lines 62-95, the test comparison includes
dynamic fields (packets, bytes, used timestamps) that cause flakiness in the
6-port scenario; update the test to remove those dynamic fields from both the
actual dpctl/dump-flows output and the expected literal before comparing—i.e.,
extend or reuse the DUMP_DP_IP_CLEAN_SORTED transformation (or add a new sed/awk
filter) to strip "packets:[0-9]+, bytes:[0-9]+, used:[^,]+" from the actual
output and update the expected block to omit those fields as well so the
OVS_WAIT_UNTIL_EQUAL compares only stable fields like in_port, eth, eth_type,
ipv4 and actions.