Skip to content

feat: enhance collector robustness, add rate metrics and CDM breakouts - #3

Merged
pradiptapks merged 1 commit into
mainfrom
feat/dpdk-filters
Jun 8, 2026
Merged

feat: enhance collector robustness, add rate metrics and CDM breakouts#3
pradiptapks merged 1 commit into
mainfrom
feat/dpdk-filters

Conversation

@pradiptapks

Copy link
Copy Markdown
Contributor

Comprehensive enhancement of tool-dpdk for multi-host OVS-DPDK, testpmd, and TRex topologies.

Collector (dpdk-collect, dpdk_telemetry_client.py, dpdk-start):

  • Retry socket discovery indefinitely until SIGTERM instead of a fixed timeout. The gap between rickshaw's start-tools and server-start phases can exceed the old 180s default, causing the collector to exit before testpmd creates its telemetry socket. The --connect-timeout parameter (default 30s) now controls the per-cycle retry interval.

  • Use the negotiated max_output_len from the DPDK handshake for recv() calls instead of the hardcoded 16384. Prevents truncated JSON on NICs with many xstats (e.g., ConnectX-5/6 with 200+ counters).

  • Log the discovered socket path on successful connection to aid debugging in multi-host setups.

  • Store per-port metadata (PCI address, driver, MAC, MTU, queue counts) from /ethdev/info in the JSONL header as port_info.

Post-processor (dpdk-post-process):

  • Compute delta rates between consecutive samples: rx-pps, tx-pps, rx-Gbps, tx-Gbps, rx-missed-sec. These give correct steady-state rates scoped to the measurement period, excluding binary search trial traffic. Uses actual timestamp deltas for interval-agnostic computation. Handles counter wraps gracefully.

  • Add direction breakout using the existing CDM 'direction' field. All xstats with rx_/tx_ prefixes are normalized: the prefix is stripped and direction=rx/tx is added as a CDM label. Enables --breakout direction to compare RX vs TX in a single query.

  • Add PCI address as 'device' label using the existing CDM field. Colons replaced with dots (0000:4b:00.0 -> 0000.4b.00.0).

  • Normalize per-queue xstats with direction + queue labels: xstat-rx_q0_packets -> xstat-q_packets (direction=rx, queue=0). Enables --breakout direction,queue for per-queue analysis. Requires CDM schema update to add 'queue' field before OpenSearch will accept these documents.

  • Core metrics (rx-packets, tx-packets, rx-pps, tx-pps, etc.) remain unchanged with direction embedded in the type name.

This PR is depending the CDM PR: perftool-incubator/CommonDataModel#188

@pradiptapks
pradiptapks requested a review from k-rister June 4, 2026 12:59
@pradiptapks pradiptapks self-assigned this Jun 4, 2026
@pradiptapks pradiptapks added the bug Something isn't working label Jun 4, 2026
@pradiptapks pradiptapks added the enhancement New feature or request label Jun 4, 2026
@k-rister

k-rister commented Jun 4, 2026

Copy link
Copy Markdown
Contributor
  • Compute delta rates between consecutive samples: rx-pps, tx-pps, rx-Gbps, tx-Gbps, rx-missed-sec. These give correct steady-state rates scoped to the measurement period, excluding binary search trial traffic. Uses actual timestamp deltas for interval-agnostic computation. Handles counter wraps gracefully.

@atheurer is this actually necessary? Wouldn't the existing CDM query logic take care of this?

Comprehensive enhancement of tool-dpdk for multi-host OVS-DPDK,
testpmd, and TRex topologies.

Collector (dpdk-collect, dpdk_telemetry_client.py, dpdk-start):

- Retry socket discovery indefinitely until SIGTERM instead of a
  fixed timeout. The gap between rickshaw's start-tools and
  server-start phases can exceed the old 180s default, causing the
  collector to exit before testpmd creates its telemetry socket.
  The --connect-timeout parameter (default 30s) now controls the
  per-cycle retry interval.

- Use the negotiated max_output_len from the DPDK handshake for
  recv() calls instead of the hardcoded 16384. Prevents truncated
  JSON on NICs with many xstats (e.g., ConnectX-5/6 with 200+
  counters).

- Log the discovered socket path on successful connection to aid
  debugging in multi-host setups.

- Store per-port metadata (PCI address, driver, MAC, MTU, queue
  counts) from /ethdev/info in the JSONL header as port_info.

Post-processor (dpdk-post-process):

- Compute delta rates between consecutive samples: rx-pps, tx-pps,
  rx-Gbps, tx-Gbps, rx-missed-sec. These give correct steady-state
  rates scoped to the measurement period, excluding binary search
  trial traffic. Uses actual timestamp deltas for interval-agnostic
  computation. Handles counter wraps gracefully.

- Add direction breakout using the existing CDM 'direction' field.
  All xstats with rx_/tx_ prefixes are normalized: the prefix is
  stripped and direction=rx/tx is added as a CDM label. Enables
  --breakout direction to compare RX vs TX in a single query.

- Add PCI address as 'device' label using the existing CDM field.
  Colons replaced with dots (0000:4b:00.0 -> 0000.4b.00.0).

- Normalize per-queue xstats with direction + queue labels:
  xstat-rx_q0_packets -> xstat-q_packets (direction=rx, queue=0).
  Enables --breakout direction,queue for per-queue analysis.
  Requires CDM schema update to add 'queue' field before OpenSearch
  will accept these documents.

- Core metrics (rx-packets, tx-packets, rx-pps, tx-pps, etc.)
  remain unchanged with direction embedded in the type name.

Co-authored-by: Cursor <cursoragent@cursor.com>
@atheurer

atheurer commented Jun 5, 2026

Copy link
Copy Markdown
  • Compute delta rates between consecutive samples: rx-pps, tx-pps, rx-Gbps, tx-Gbps, rx-missed-sec. These give correct steady-state rates scoped to the measurement period, excluding binary search trial traffic. Uses actual timestamp deltas for interval-agnostic computation. Handles counter wraps gracefully.

@atheurer is this actually necessary? Wouldn't the existing CDM query logic take care of this?

@k-rister
I don't think it does. We have many tools which get metrics like total_bytes (uperf) and then have to subtract the previous total_bytes to get a delta, then divide by seconds (looking at the current and previous timestamps) to make it a throughput metric. Some tools' native output already do this, but some tools/benchmarks do not.

I suppose it's possible to enhance toolbox CDM libs to make this easier, but I think we would have to use a new function similar to log_sample, something like convert_and_log_sample, where the args would be the count of something, like total_bytes, and the timestamp, and then this function would need to remember the previous call to convert_and_log_sample, so it could do the math for us.

@k-rister

k-rister commented Jun 5, 2026

Copy link
Copy Markdown
Contributor
  • Compute delta rates between consecutive samples: rx-pps, tx-pps, rx-Gbps, tx-Gbps, rx-missed-sec. These give correct steady-state rates scoped to the measurement period, excluding binary search trial traffic. Uses actual timestamp deltas for interval-agnostic computation. Handles counter wraps gracefully.

@atheurer is this actually necessary? Wouldn't the existing CDM query logic take care of this?

@k-rister I don't think it does. We have many tools which get metrics like total_bytes (uperf) and then have to subtract the previous total_bytes to get a delta, then divide by seconds (looking at the current and previous timestamps) to make it a throughput metric. Some tools' native output already do this, but some tools/benchmarks do not.

I suppose it's possible to enhance toolbox CDM libs to make this easier, but I think we would have to use a new function similar to log_sample, something like convert_and_log_sample, where the args would be the count of something, like total_bytes, and the timestamp, and then this function would need to remember the previous call to convert_and_log_sample, so it could do the math for us.

What you are referring to are metrics that are simply counts of things but the metrics mentioned here are already in "rate" form such as rx-pps, tx-pps, rx-Gbps, etc. so it doesn't need that kind of handling, correct?

@atheurer

atheurer commented Jun 5, 2026

Copy link
Copy Markdown

@k-rister yes those do not need that type of handling, but compute_rate only affects "ibytes" and "obytes"

@pradiptapks

Copy link
Copy Markdown
Contributor Author

@k-rister @atheurer Good catch on the naming : the stored types (rx-pps, rx-Gbps, etc.) are rates, but DPDK only gives cumulative counters. CDM query aggregates stored values; it doesn't compute counter deltas over time (same reason uperf derives Gbps in post-process).

compute_rate() only special-cases ibytes/obytes → Gbps; everything else is delta / delta_sec. Rate samples use begin/end per collector interval so binary-search trial traffic doesn't skew measurement-period throughput.

Cumulative metrics (rx-packets, etc.) are still emitted unchanged alongside the rate types.

@k-rister Does the above address your concern? If so, I'd appreciate a review when you have a chance. The dependent CDM schema change (CommonDataModel#188 for the queue field) is already merged.

@pradiptapks
pradiptapks merged commit 334c0ca into main Jun 8, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from Queued to Done in Crucible Tracking Jun 8, 2026
@pradiptapks
pradiptapks deleted the feat/dpdk-filters branch June 8, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants