From 08e77d041c2d680c530321381dea8da232832c31 Mon Sep 17 00:00:00 2001 From: roshan-ku Date: Mon, 5 Oct 2026 14:58:56 +0530 Subject: [PATCH 1/7] feat: add hardware PTP timing with transmitter-as-grandmaster mode The deployment switch has PTP enabled but its grandmaster has no GPS, so its absolute time is not a usable reference. The transmitter must own the reference clock instead. MTL's built-in PTP client is slave-only -- mt_ptp.c only ever transmits PTP_DELAY_REQ and never sends Announce/Sync -- so MTL cannot elect itself grandmaster. The role is therefore split: ptp4l serves the NIC's PTP Hardware Clock to the receivers, and dvledtx feeds MTL that same PHC through mtl_init_params.ptp_get_time_fn with pacing forced to ST21_TX_PACING_WAY_PTP. TX pacing, RTP timestamps and the clock the receivers lock to then all resolve to one oscillator, independent of the switch. Add a ptp.mode option selecting between the existing external-grandmaster behaviour ("slave") and the new local-PHC reference ("grandmaster"), alongside phc_interface / phc_device / phc_interval_ms. New util/ptp_clock serves the PHC to MTL. A /dev/ptpN read costs microseconds, far too slow for the per-packet pacing path, so the PHC is sampled on a background thread and served as a frequency-corrected linear model over CLOCK_MONOTONIC_RAW, published through a lock-free slot ring. PHC references from config are validated against /dev/ptpN or an interface name before reaching open(). Grandmaster mode requires the direct MTL TX build: a function pointer has no AVOption equivalent and the mtl_st20p plugin owns the mtl_init() call, so the muxer build rejects it at config load rather than transmitting on the wrong clock. Also fix a latent bug on the muxer path: ptp.enable set only the MTL_FLAG_PTP_* flags and never pacing_way, leaving mtl_init_params.pacing at AUTO, which resolves to TSC. Slave mode therefore never actually used PTP pacing there. Add scripts/ptp_grandmaster.sh (ptp4l with masterOnly 1 and priority1 64, so BMCA stays on this host against switch grandmasters that advertise 128), a sample config, a README section and unit tests. Verified: both build variants compile warning-clean; the PHC time source tracks a live Intel E610 PHC to a 256 ns residual with a 38930 ppb frequency correction. --- README.md | 174 +++++++++- config/tx_fullhd_ptp_grandmaster.json | 36 +++ include/app_context.h | 27 +- include/core/session_manager.h | 4 + include/util/config_reader.h | 11 +- include/util/ptp_clock.h | 56 ++++ meson.build | 3 +- scripts/ptp_grandmaster.sh | 73 +++++ src/ffmpeg/ffmpeg_tx.c | 24 +- src/mtl/mtl_tx.c | 55 +++- src/util/config_reader.c | 92 +++++- src/util/ptp_clock.c | 442 ++++++++++++++++++++++++++ tests/meson.build | 1 + tests/test_config_reader.c | 137 ++++++++ 14 files changed, 1099 insertions(+), 36 deletions(-) create mode 100644 config/tx_fullhd_ptp_grandmaster.json create mode 100644 include/util/ptp_clock.h create mode 100755 scripts/ptp_grandmaster.sh create mode 100644 src/util/ptp_clock.c diff --git a/README.md b/README.md index 12bfc22..76b17ad 100644 --- a/README.md +++ b/README.md @@ -195,9 +195,13 @@ dvledtx uses a JSON config file with the following sections: | | `scale_height` | (Optional) Output height after scaling | | | `fps` | Frames per second (25, 30, 50, 60) | | | `fmt` | Pixel format (see [Supported Formats](#supported-formats)) | -| **ptp** | `enable` | (Optional) Enable MTL's built-in PTP client and PTP-paced TX. Default `false` (TSC-based pacing). See [PTP Timing](#ptp-timing) | -| | `pi` | (Optional) Use the PI controller for the built-in PTP client (physical function NICs only). Default `false` | -| | `unicast` | (Optional) Send `PTP_DELAY_REQ` messages to a unicast address instead of multicast. Default `false` | +| **ptp** | `enable` | (Optional) Enable hardware PTP timing and PTP-paced TX. Default `false` (TSC-based pacing). See [PTP Timing](#ptp-timing) | +| | `mode` | (Optional) `slave` (default) to lock to an external grandmaster, or `grandmaster` to use this host's NIC PHC as the reference clock | +| | `pi` | (Optional, `slave` only) Use the PI controller for the built-in PTP client (physical function NICs only). Default `false` | +| | `unicast` | (Optional, `slave` only) Send `PTP_DELAY_REQ` messages to a unicast address instead of multicast. Default `false` | +| | `phc_interface` | (Optional, `grandmaster` only) Kernel interface whose PHC `ptp4l` serves, e.g. `enp4s0` | +| | `phc_device` | (Optional, `grandmaster` only) PHC character device, e.g. `/dev/ptp2`. Takes precedence over `phc_interface`; omit both to auto-select the host's only PHC | +| | `phc_interval_ms` | (Optional, `grandmaster` only) PHC resample period in ms, `100`–`10000`. Default `1000` | | **hwaccel** | `hwaccel` | (Optional) Decode the input stream on the GPU via VA-API. Default `false` (CPU decode). Falls back to the CPU automatically when the GPU cannot decode the stream. See [Hardware Decode (VA-API)](#hardware-decode-va-api) | | **tx_sessions[]** | `nic_index` | (Optional) Index into `interfaces[]` selecting which NIC this session uses (default: `0`) | | | `udp_port` | UDP port for the session | @@ -232,27 +236,177 @@ Multiple sessions can be defined in `tx_sessions` to transmit different crop reg #### PTP Timing -ST 2110 senders normally derive their TX pacing from a PTP (IEEE 1588) clock. dvledtx exposes MTL's built-in PTP client through an optional top-level `ptp` block: +ST 2110 senders derive their TX pacing and RTP timestamps from a PTP (IEEE 1588) clock. dvledtx +exposes two PTP roles through an optional top-level `ptp` block, selected by `mode`: + +| `mode` | Reference clock | Use when | +|---|---|---| +| `slave` (default) | An external PTP grandmaster on the media LAN | A dedicated grandmaster appliance or a GPS-locked boundary-clock switch is present | +| `grandmaster` | This transmitter's own NIC PHC | There is no trustworthy external time reference — e.g. the switch has PTP enabled but no GPS, so its grandmaster time is not true time | + +**PTP is disabled by default.** When the block is absent — or `enable` is `false` — MTL falls back to +TSC-based pacing, which derives timing from the CPU's invariant timestamp counter and requires no +external clock. + +##### Slave mode — external grandmaster ```json "ptp": { "enable": true, + "mode": "slave", "pi": true, "unicast": false } ``` -**PTP is disabled by default.** When the block is absent — or `enable` is `false` — MTL falls back to TSC-based pacing, which derives timing from the CPU's invariant timestamp counter and requires no external clock. This is the recommended setting for lab and standalone use. +This enables MTL's built-in PTP client (`MTL_FLAG_PTP_ENABLE`, plus `MTL_FLAG_PTP_PI` and +`MTL_FLAG_PTP_UNICAST_ADDR`) and switches TX pacing to `ST21_TX_PACING_WAY_PTP`. MTL's PTP client is +*slave-only* — it only ever sends `PTP_DELAY_REQ` and never `Announce`/`Sync` — so with no +grandmaster on the link the pacing clock never locks and transmission stalls. + +##### Grandmaster mode — this transmitter is the reference clock + +```json +"ptp": { + "enable": true, + "mode": "grandmaster", + "phc_interface": "enp4s0", + "phc_interval_ms": 1000 +} +``` + +Because MTL cannot be a PTP master, the grandmaster role is split: **`ptp4l` serves the NIC's PTP +Hardware Clock (PHC) to the receivers, and dvledtx feeds MTL that same PHC** through +`mtl_init_params.ptp_get_time_fn`. TX pacing, RTP timestamps and the clock the receivers lock to are +then all the same clock, with no dependency on the switch's time. + +**Prerequisites** + +- `linuxptp` installed on the TX host: `sudo apt install linuxptp` +- The direct MTL TX build: `bash scripts/build.sh -Denable_mtl_tx=true` +- A NIC port **on the media segment** that is bound to the kernel driver and has a PHC. Check with: + + ```bash + ethtool -T + ``` + + It qualifies if `PTP Hardware Clock:` shows an index (not `none`) and the capabilities list + includes both `hardware-transmit` and `hardware-receive`. `hardware-transmit` is what makes the + master role possible — the NIC must timestamp the `Sync` frames it sends. Most Intel (`igb`, + `igc`, `ixgbe`, `ice`), Mellanox and Broadcom server NICs qualify; many consumer NICs do not. + +**Choose a topology.** A port bound to `vfio-pci` has no netdev and no `/dev/ptpN`, so `ptp4l` +cannot run on it. Either option below solves that: + +| Topology | When to use | Setup | +|---|---|---| +| **SR-IOV on the TX NIC** | Only one NIC port is available on the media segment | PF stays in the kernel for `ptp4l`, dvledtx uses the VFs | +| **Separate PTP port** | A spare media-segment port exists | `ptp4l` owns port A; dvledtx keeps port B fully bound to DPDK and reads port A's PHC | + +The second works because MTL never touches the TX port's own clock in this mode — it uses whatever +`ptp_get_time_fn` returns. Both receivers and TX pacing then follow the PTP port's oscillator. + +**Setup — SR-IOV topology** + +1. Create trusted VFs. This keeps the PF bound to the kernel (so it retains its netdev and PHC), + brings it UP, and binds the VFs to `vfio-pci`: + + ```bash + sudo bash /script/nicctl.sh create_tvf 0000:04:00.0 4 # VF count, default 6 + sudo bash /script/nicctl.sh list 0000:04:00.0 # note the VF BDFs + ``` + +2. Point `interfaces[].name` in the config at the **VF** BDFs (e.g. `0000:04:10.0`), and + `ptp.phc_interface` at the **PF** netdev (e.g. `enp4s0`). + +3. Start the grandmaster on the PF: + + ```bash + sudo bash scripts/ptp_grandmaster.sh enp4s0 64 + ``` -Enable PTP only when a **PTP grandmaster is present on the network** (a dedicated appliance or a boundary-clock switch). MTL's built-in PTP client is *slave-only* — it cannot elect itself grandmaster — so with `enable: true` and no grandmaster on the link the TX pacing clock never locks and transmission stalls. + The script runs `ptp4l` with `masterOnly 1` and `priority1 64`, which keeps BMCA on this host — + switch grandmasters normally advertise `priority1 128`, so a lower value wins. `masterOnly` + additionally stops `ptp4l` from ever slaving to the switch. -When enabled, the startup log reports the active settings and the pacing mode chosen by MTL: +4. Leave it running and start dvledtx as usual. + +For the separate-port topology, skip step 1 and run `ptp4l` on the spare port instead; the TX NIC +stays entirely under DPDK. + +**Verifying it works** + +On a **receiver**, confirm which clock actually won the election: + +```bash +pmc -u -b 0 'GET PARENT_DATA_SET' +``` + +`grandmasterIdentity` must be the TX NIC's clock identity — derived from its MAC with `fffe` +inserted in the middle, e.g. MAC `f8:02:78:24:91:bc` → `f80278.fffe.2491bc`. If it names the switch +instead, BMCA went the wrong way; see troubleshooting below. + +On the **transmitter**, `ptp4l` should settle into master state and dvledtx should report a small, +stable PHC residual: + +``` +ptp4l[...]: selected local clock f80278.fffe.2491bc as best master +ptp4l[...]: assuming the grand master role +[INFO ] PTP clock: /dev/ptp2 offset=-53 ns freq=28134 ppb samples=16 +``` + +A residual that keeps growing, or repeated `stepping local model` warnings, means the PHC is being +stepped externally — usually a second `ptp4l`/`phc2sys` instance fighting for the same clock. + +**Other notes** + +- `pi` and `unicast` apply only to the built-in PTP client and are ignored in grandmaster mode. +- `phc_interval_ms` (default `1000`, range `100`–`10000`) is how often the PHC is sampled. The PHC is + *not* read per packet — a `/dev/ptpN` read costs microseconds — so dvledtx samples it on a + background thread and serves MTL a frequency-corrected model over `CLOCK_MONOTONIC_RAW`. The + startup and periodic logs report the residual offset and frequency correction: + + ``` + [INFO ] PTP clock: using /dev/ptp2 as TX time source (resample 1000 ms, start 1791191418286827491 ns) + [INFO ] PTP clock: /dev/ptp2 offset=-53 ns freq=28134 ppb samples=16 + ``` + +- The free-running PHC of a grandmaster without GPS does not carry true time. Receivers only need to + agree with the transmitter, which they do; do not use these timestamps as a wall-clock reference. + +**The grandmaster PHC and the system clock are independent.** The NIC driver seeds the PHC from +`CLOCK_REALTIME` when the interface is bound and it free-runs from there — typically tens of ppm, so +it drifts seconds per day away from the host's wall clock. This is harmless for video (every +receiver inherits the same offset) but makes application logs, which use the system clock, hard to +correlate with RTP timestamps. To tie them together, steer the **system clock from the +grandmaster** — the reverse of the usual `phc2sys` direction: + +```bash +sudo phc2sys -s enp4s0 -c CLOCK_REALTIME -O 0 -w -m +``` + +**Troubleshooting** + +| Symptom | Cause | Fix | +|---|---|---| +| `pmc` on the RX names the switch as grandmaster | The switch is configured as a forced/always-master GM and refuses to slave, so it wins BMCA | Raise the switch's `priority1` above the value passed to the script, or put the port into transparent/boundary-clock mode | +| `cannot open /dev/ptpN (Permission denied)` | dvledtx is not running as root; `/dev/ptp*` is `root:root 0600` | Run as root, or grant access to the PHC device | +| `interface 'X' has no PTP hardware clock` | The port is bound to `vfio-pci`, or the NIC has no PHC | Keep the PTP port on the kernel driver; verify with `ethtool -T` | +| `ptp.mode 'grandmaster' requires the direct MTL TX build` | Built without `-Denable_mtl_tx=true` | Rebuild with the direct MTL TX path | +| Receivers lock, but video drifts over hours | TX and RX are not actually on the same grandmaster | Re-check `pmc` on every receiver, not just one | + +##### Startup logs ``` -[INFO ] PTP enabled: pi_controller=1 unicast_delay_req=0 +[INFO ] PTP enabled: mode=slave pi_controller=1 unicast_delay_req=0 MTL: ... tv_attach(0), pacing way: ptp ``` +``` +[INFO ] PTP enabled: mode=grandmaster phc=enp4s0 resample=1000 ms +[INFO ] MTL init: PTP grandmaster mode, time source /dev/ptp2, pacing=ptp +``` + With PTP disabled the log instead shows: ``` @@ -260,7 +414,9 @@ With PTP disabled the log instead shows: MTL: ... tv_attach(0), pacing way: tsc ``` -> Requires an MTL/FFmpeg build that exposes the `ptp_enable`, `ptp_pi` and `ptp_unicast` AVOptions on the `mtl_st20p` muxer. If they are missing, dvledtx logs a warning and continues without built-in PTP rather than failing. +> Slave mode on the `mtl_st20p` muxer build requires an MTL/FFmpeg build that exposes the +> `ptp_enable`, `ptp_pi`, `ptp_unicast` and `pacing_way` AVOptions. If they are missing, dvledtx logs +> a warning and continues without built-in PTP rather than failing. #### Hardware Decode (VA-API) diff --git a/config/tx_fullhd_ptp_grandmaster.json b/config/tx_fullhd_ptp_grandmaster.json new file mode 100644 index 0000000..3cda45f --- /dev/null +++ b/config/tx_fullhd_ptp_grandmaster.json @@ -0,0 +1,36 @@ +{ + "log_file": "dvledtx.log", + "interfaces": [ + { + "nic_index": 0, + "name": "0000:04:10.0", + "sip": "192.168.50.29", + "dip": "239.168.85.20" + } + ], + "video": { + "width": 1920, + "height": 1080, + "tx_url": "bbb_sunflower_1080p_30fps_normal.mp4" + }, + "tx_video": { + "scale_width": 1920, + "scale_height": 1080, + "fps": 30, + "fmt": "yuv422p10le" + }, + "ptp": { + "enable": true, + "mode": "grandmaster", + "phc_interface": "enp4s0", + "phc_interval_ms": 1000 + }, + "tx_sessions": [ + { + "nic_index": 0, + "udp_port": 20000, + "payload_type": 96, + "crop": { "x": 0, "y": 0, "w": 1920, "h": 1080 } + } + ] +} diff --git a/include/app_context.h b/include/app_context.h index 377e35e..e96ceee 100644 --- a/include/app_context.h +++ b/include/app_context.h @@ -25,6 +25,12 @@ struct nic_config { uint8_t dip_addr[IP_ADDR_BYTES]; /* destination IP binary */ }; +/* PTP timing role of this transmitter (top-level "ptp" JSON object, "mode"). */ +enum dvledtx_ptp_mode { + DVLEDTX_PTP_MODE_SLAVE = 0, /* lock to an external PTP grandmaster */ + DVLEDTX_PTP_MODE_GRANDMASTER, /* this host's NIC PHC is the reference clock */ +}; + /* Per-session network and crop parameters (populated from JSON tx_sessions[]) */ struct tx_session_net { uint16_t udp_port; @@ -69,12 +75,21 @@ struct dvledtx_context { bool force_dhcp; int test_time_s; - /* PTP hardware timing (built-in MTL PTP client) for both the direct MTL TX - * pipeline and the FFmpeg mtl_st20p muxer path. See MTL_FLAG_PTP_* in - * mtl_api.h; the FFmpeg path passes these through as AVOptions in ffmpeg_tx.c. */ - bool ptp_enable; /* enable built-in PTP client + PTP-paced TX */ - bool ptp_pi; /* use PI controller for built-in PTP (PF only) */ - bool ptp_unicast; /* use unicast address for PTP_DELAY_REQ message */ + /* PTP hardware timing. Two modes, selected by ptp_mode: + * - SLAVE: MTL's built-in PTP client locks to an external grandmaster. + * Works on both the direct MTL TX pipeline and the FFmpeg mtl_st20p + * muxer path (passed through as AVOptions in ffmpeg_tx.c). + * - GRANDMASTER: this host owns the reference clock. ptp4l serves the + * NIC PHC to the receivers and MTL is fed that same PHC through + * mtl_init_params.ptp_get_time_fn. Direct MTL TX path only, because a + * function pointer cannot be passed through an AVOption. + * See MTL_FLAG_PTP_* in mtl_api.h and src/util/ptp_clock.c. */ + bool ptp_enable; /* enable hardware PTP timing + PTP-paced TX */ + int ptp_mode; /* enum dvledtx_ptp_mode */ + bool ptp_pi; /* use PI controller for built-in PTP (PF only, slave mode) */ + bool ptp_unicast; /* use unicast address for PTP_DELAY_REQ message (slave mode) */ + char ptp_phc[64]; /* grandmaster mode: "/dev/ptpN" or kernel iface name */ + int ptp_phc_interval_ms; /* grandmaster mode: PHC resample period (0 = default) */ /* Per-session network + crop config — dynamically allocated, st20p_sessions elements */ struct tx_session_net* session_net; diff --git a/include/core/session_manager.h b/include/core/session_manager.h index 43cb0dd..9be05f1 100644 --- a/include/core/session_manager.h +++ b/include/core/session_manager.h @@ -26,6 +26,7 @@ // Forward declarations struct dvledtx_context; +struct ptp_clock; /* * Shared decode context - ONE decoder feeds ALL N TX sessions. @@ -138,6 +139,9 @@ typedef struct session_manager_s { mtl_handle mtl; /* MTL library instance — owns all st20p_tx sessions */ + /* PTP grandmaster mode only: local NIC PHC feeding mtl_init_params.ptp_get_time_fn */ + struct ptp_clock* ptp_clk; + bool running; } session_manager_t; diff --git a/include/util/config_reader.h b/include/util/config_reader.h index c2938b7..8a03f16 100644 --- a/include/util/config_reader.h +++ b/include/util/config_reader.h @@ -47,12 +47,17 @@ struct dvledtx_config { /* optional log file path (empty = console only) */ char log_file[256]; - /* PTP hardware timing (built-in MTL PTP client), from the optional top-level - * "ptp" JSON object. All default to false when the object or a key is absent, - * in which case MTL falls back to TSC-based TX pacing. */ + /* PTP hardware timing, from the optional top-level "ptp" JSON object. + * Disabled by default, in which case MTL falls back to TSC-based TX pacing. + * ptp_mode selects between slaving to an external grandmaster ("slave", + * the default) and acting as the grandmaster off the local NIC PHC + * ("grandmaster"). See enum dvledtx_ptp_mode in app_context.h. */ bool ptp_enable; + int ptp_mode; bool ptp_pi; bool ptp_unicast; + char ptp_phc[64]; /* grandmaster mode: "/dev/ptpN" or kernel iface name */ + int ptp_phc_interval_ms; /* grandmaster mode: PHC resample period in ms */ /* tx_sessions array — dynamically allocated */ int session_count; diff --git a/include/util/ptp_clock.h b/include/util/ptp_clock.h new file mode 100644 index 0000000..70e3899 --- /dev/null +++ b/include/util/ptp_clock.h @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright 2026 Intel Corporation + */ + +#pragma once +#include +#include + +/* + * ptp_clock — local NIC PTP Hardware Clock (PHC) time source for MTL. + * + * Used for the "grandmaster" PTP mode, where this TX host owns the reference + * clock instead of slaving to an external grandmaster. ptp4l runs as PTP + * master on the NIC's kernel interface and disciplines/serves that NIC's PHC + * to the receivers; dvledtx feeds the very same PHC to MTL through + * mtl_init_params.ptp_get_time_fn so the TX pacing, RTP timestamps and the + * grandmaster the receivers lock to are all the same clock. + * + * MTL calls the time function from the dataplane tasklet, far too often for a + * per-call /dev/ptpN read (~1-2 us each, a driver register access). The module + * therefore samples the PHC on a low-rate background thread and serves time + * from a frequency-corrected linear model over CLOCK_MONOTONIC_RAW, so the + * fast path is a single vDSO clock_gettime() plus arithmetic. + */ + +struct ptp_clock; + +/* True when `ref` is a syntactically valid PHC reference — "/dev/ptpN" or a + * kernel interface name. Does not check that the device exists; used to reject + * config values before they reach open(). */ +bool ptp_clock_valid_device_ref(const char* ref); + +/* Open the PHC and start the sampling thread. + * device — "/dev/ptpN", a kernel interface name ("enp4s0"), or "" to + * auto-select the only PHC present on the host. + * interval_ms — PHC resample period; 0 selects the default (1000 ms). + * Returns NULL on error (device not found, no permission, PHC not readable). */ +struct ptp_clock* ptp_clock_open(const char* device, unsigned int interval_ms); + +/* Stop the sampling thread and release the handle. Safe with NULL. */ +void ptp_clock_close(struct ptp_clock* clk); + +/* Current PHC time in nanoseconds (TAI, as served by the local grandmaster). + * Signature matches mtl_init_params.ptp_get_time_fn; priv is the handle. */ +uint64_t ptp_clock_get_time_ns(void* priv); + +/* Resolved device path, for logging. Returns "" for a NULL handle. */ +const char* ptp_clock_device(const struct ptp_clock* clk); + +/* Sampling health snapshot. Any out pointer may be NULL. + * offset_ns — last measured error between the served model and the PHC + * freq_ppb — current model frequency correction vs CLOCK_MONOTONIC_RAW + * samples — number of PHC samples accepted so far + * Returns 0 on success, -1 for a NULL handle. */ +int ptp_clock_stats(const struct ptp_clock* clk, int64_t* offset_ns, + int64_t* freq_ppb, uint64_t* samples); diff --git a/meson.build b/meson.build index a85c8b2..564c7f2 100644 --- a/meson.build +++ b/meson.build @@ -42,7 +42,8 @@ executable('dvledtx', 'src/ffmpeg/ffmpeg_frame_handler.c', 'src/ffmpeg/ffmpeg_tx.c', 'src/mtl/mtl_tx.c', - 'src/util/logger.c' + 'src/util/logger.c', + 'src/util/ptp_clock.c' ], include_directories: include_directories('include'), dependencies: all_deps, diff --git a/scripts/ptp_grandmaster.sh b/scripts/ptp_grandmaster.sh new file mode 100755 index 0000000..ff2a3b7 --- /dev/null +++ b/scripts/ptp_grandmaster.sh @@ -0,0 +1,73 @@ +#!/bin/bash +# SPDX-License-Identifier: BSD-3-Clause +# Copyright 2026 Intel Corporation + +# Run ptp4l as the PTP grandmaster for a dvledtx deployment. +# +# Use when the media switch has no usable time reference (its grandmaster has +# no GPS, so its absolute time is wrong) and the transmitter must own the +# reference clock instead. ptp4l disciplines and serves this host's NIC PHC to +# the receivers; dvledtx reads the same PHC through the "ptp" config block with +# "mode": "grandmaster", so TX pacing and RTP timestamps come from the clock the +# receivers lock to. +# +# Usage: sudo bash scripts/ptp_grandmaster.sh [priority1] +# e.g. sudo bash scripts/ptp_grandmaster.sh enp4s0 64 +# +# must still be owned by the kernel driver — a port bound to +# vfio-pci has no netdev and no /dev/ptpN. With a DPDK transmitter, keep the PF +# in the kernel and give dvledtx SR-IOV VFs. + +set -e + +IFACE="${1:?usage: $0 [priority1]}" +PRIORITY1="${2:-64}" # lower wins BMCA; switch grandmasters usually sit at 128 + +if [[ ! -d "/sys/class/net/${IFACE}" ]]; then + echo "error: interface ${IFACE} not found (bound to a userspace driver?)" >&2 + exit 1 +fi + +if ! command -v ptp4l >/dev/null 2>&1; then + echo "error: ptp4l not found — install linuxptp (apt install linuxptp)" >&2 + exit 1 +fi + +PHC_INDEX="$(ethtool -T "${IFACE}" 2>/dev/null | awk '/PTP Hardware Clock:/ {print $4}')" +if [[ -z "${PHC_INDEX}" || "${PHC_INDEX}" == "none" || "${PHC_INDEX}" == "-1" ]]; then + echo "error: ${IFACE} has no PTP hardware clock" >&2 + exit 1 +fi + +CONF="$(mktemp /tmp/ptp4l-gm-XXXXXX.conf)" +trap 'rm -f "${CONF}"' EXIT + +# priority1 below the switch's keeps BMCA on this host; masterOnly stops ptp4l +# from ever slaving to the switch's GPS-less grandmaster. +cat >"${CONF}" </dev/null 2>&1 || \ + echo "warn: could not seed ${IFACE} PHC from the system clock; continuing free-running" + +exec ptp4l -f "${CONF}" -i "${IFACE}" -m diff --git a/src/ffmpeg/ffmpeg_tx.c b/src/ffmpeg/ffmpeg_tx.c index ba93325..54ec8d1 100644 --- a/src/ffmpeg/ffmpeg_tx.c +++ b/src/ffmpeg/ffmpeg_tx.c @@ -134,20 +134,42 @@ int open_ffmpeg_tx(struct st20p_tx_ctx* ctx) { * when the installed MTL FFmpeg plugin carries the PTP patch; if it doesn't, * av_opt_set_int() returns AVERROR_OPTION_NOT_FOUND, which we treat as a * benign "PTP not supported by this build" case so the app works whether or - * not the MTL plugin has the PTP patch applied. */ + * not the MTL plugin has the PTP patch applied. + * + * Grandmaster mode cannot work here: it needs mtl_init_params.ptp_get_time_fn, + * a function pointer with no AVOption equivalent, and the plugin owns the + * mtl_init() call on this path. It requires -Denable_mtl_tx=true. */ + if (ctx->app->ptp_enable && ctx->app->ptp_mode == DVLEDTX_PTP_MODE_GRANDMASTER) { + LOG_ERROR("ST20P TX(%d): ptp.mode 'grandmaster' is not supported by the " + "mtl_st20p muxer path — rebuild with -Denable_mtl_tx=true", ctx->idx); + avformat_free_context(ctx->out_fmt_ctx); ctx->out_fmt_ctx = NULL; + return -1; + } if (ctx->app->ptp_enable) { static const char* const ptp_opts[] = {"ptp_enable", "ptp_pi", "ptp_unicast"}; const int ptp_vals[] = {1, ctx->app->ptp_pi ? 1 : 0, ctx->app->ptp_unicast ? 1 : 0}; + bool ptp_opts_ok = true; for (size_t i = 0; i < FF_ARRAY_ELEMS(ptp_opts); i++) { ret = av_opt_set_int(ctx->out_fmt_ctx->priv_data, ptp_opts[i], ptp_vals[i], 0); if (ret == AVERROR_OPTION_NOT_FOUND) { LOG_INFO("ST20P TX(%d): PTP AVOptions unsupported by installed MTL plugin " "(no PTP patch); continuing without built-in PTP", ctx->idx); + ptp_opts_ok = false; break; } if (ret < 0) LOG_WARN("ST20P TX(%d): av_opt_set_int %s failed (ret=%d)", ctx->idx, ptp_opts[i], ret); } + /* ptp_enable only sets MTL_FLAG_PTP_*; without this the plugin leaves + * mtl_init_params.pacing at AUTO and the TX still paces off the TSC. */ + if (ptp_opts_ok) { + ret = av_opt_set(ctx->out_fmt_ctx->priv_data, "pacing_way", "ptp", 0); + if (ret == AVERROR_OPTION_NOT_FOUND) + LOG_WARN("ST20P TX(%d): mtl_st20p muxer has no pacing_way AVOption; " + "TX will keep TSC pacing despite PTP being enabled", ctx->idx); + else if (ret < 0) + LOG_WARN("ST20P TX(%d): av_opt_set pacing_way=ptp failed (ret=%d)", ctx->idx, ret); + } } /* Multi-NIC: mtl_init()/DPDK EAL is only ever initialised once per diff --git a/src/mtl/mtl_tx.c b/src/mtl/mtl_tx.c index 8117b21..2ba43b3 100644 --- a/src/mtl/mtl_tx.c +++ b/src/mtl/mtl_tx.c @@ -30,6 +30,7 @@ #include "app_context.h" #include "core/session_manager.h" #include "util/logger.h" +#include "util/ptp_clock.h" #include #include #include @@ -199,14 +200,40 @@ void mtl_copy_crop_to_frame(struct st_frame* dst, const AVFrame* src, * * mtl_tx_uninit() — release the MTL library instance. */ -/* PTP hardware timing (built-in MTL PTP client) — used by both the direct MTL TX - * pipeline and the FFmpeg avdevice TX path via mtl_tx_init(). See MTL_FLAG_PTP_* in mtl_api.h. */ +/* PTP hardware timing — used by both the direct MTL TX pipeline and the FFmpeg + * avdevice TX path via mtl_tx_init(). See MTL_FLAG_PTP_* in mtl_api.h. */ static void mtl_tx_ptp_sync_notify_cb(void* priv, struct mtl_ptp_sync_notify_meta* meta) { (void)priv; LOG_INFO("PTP sync: master_utc_offset=%d delta=%" PRId64 "ns", meta->master_utc_offset, meta->delta); } +/* Grandmaster mode: the local NIC PHC served by ptp4l is the reference clock. + * MTL's built-in PTP client is slave-only (it only ever sends PTP_DELAY_REQ and + * never Announce/Sync), so it is deliberately NOT enabled here — it would make + * MTL lock to whatever grandmaster is on the link instead of to us. Instead the + * PHC is handed to MTL as ptp_get_time_fn, which mt_dev.c wires in ahead of its + * own sources, and pacing is switched to ST21_TX_PACING_WAY_PTP so the video + * transmitter paces off mt_get_ptp_time() rather than the TSC. */ +static int mtl_tx_ptp_grandmaster_init(struct mtl_init_params* mtl_params, + session_manager_t* manager, + struct dvledtx_context* app) { + manager->ptp_clk = ptp_clock_open(app->ptp_phc, (unsigned int)app->ptp_phc_interval_ms); + if (manager->ptp_clk == NULL) { + LOG_ERROR("PTP grandmaster mode: no usable PHC time source. The NIC that " + "runs ptp4l must keep its kernel interface (use SR-IOV VFs for " + "the DPDK ports), and dvledtx needs read access to /dev/ptpN"); + return -1; + } + + mtl_params->priv = manager->ptp_clk; + mtl_params->ptp_get_time_fn = ptp_clock_get_time_ns; + mtl_params->pacing = ST21_TX_PACING_WAY_PTP; + LOG_INFO("MTL init: PTP grandmaster mode, time source %s, pacing=ptp", + ptp_clock_device(manager->ptp_clk)); + return 0; +} + int mtl_tx_init(session_manager_t* manager, struct dvledtx_context* app) { struct mtl_init_params mtl_params; memset(&mtl_params, 0, sizeof(mtl_params)); @@ -215,18 +242,24 @@ int mtl_tx_init(session_manager_t* manager, struct dvledtx_context* app) { mtl_params.num_ports = app->nic_count; if (app->ptp_enable) { - mtl_params.flags |= MTL_FLAG_PTP_ENABLE; - mtl_params.pacing = ST21_TX_PACING_WAY_PTP; - if (app->ptp_pi) mtl_params.flags |= MTL_FLAG_PTP_PI; - if (app->ptp_unicast) mtl_params.flags |= MTL_FLAG_PTP_UNICAST_ADDR; - mtl_params.ptp_sync_notify = mtl_tx_ptp_sync_notify_cb; - LOG_INFO("MTL init: PTP enabled (pi=%d unicast=%d)", app->ptp_pi, app->ptp_unicast); + if (app->ptp_mode == DVLEDTX_PTP_MODE_GRANDMASTER) { + if (mtl_tx_ptp_grandmaster_init(&mtl_params, manager, app) < 0) return -1; + } else { + mtl_params.flags |= MTL_FLAG_PTP_ENABLE; + mtl_params.pacing = ST21_TX_PACING_WAY_PTP; + if (app->ptp_pi) mtl_params.flags |= MTL_FLAG_PTP_PI; + if (app->ptp_unicast) mtl_params.flags |= MTL_FLAG_PTP_UNICAST_ADDR; + mtl_params.ptp_sync_notify = mtl_tx_ptp_sync_notify_cb; + LOG_INFO("MTL init: PTP slave mode (pi=%d unicast=%d)", app->ptp_pi, app->ptp_unicast); + } } /* Count sessions assigned to each NIC for queue allocation */ int* sessions_per_nic = calloc((size_t)app->nic_count, sizeof(int)); if (sessions_per_nic == NULL) { LOG_ERROR("Failed to allocate sessions_per_nic array"); + ptp_clock_close(manager->ptp_clk); + manager->ptp_clk = NULL; return -1; } for (int i = 0; i < app->st20p_sessions; i++) { @@ -254,6 +287,8 @@ int mtl_tx_init(session_manager_t* manager, struct dvledtx_context* app) { manager->mtl = mtl_init(&mtl_params); if (!manager->mtl) { LOG_ERROR("Failed to initialise MTL library"); + ptp_clock_close(manager->ptp_clk); + manager->ptp_clk = NULL; return -1; } LOG_INFO("MTL library initialised successfully (%d port(s))", app->nic_count); @@ -265,6 +300,10 @@ void mtl_tx_uninit(session_manager_t* manager) { mtl_uninit(manager->mtl); manager->mtl = NULL; } + /* Closed after mtl_uninit: MTL may still call ptp_get_time_fn while tearing + * down its tasklets. */ + ptp_clock_close(manager->ptp_clk); + manager->ptp_clk = NULL; } /* ========================================================================= diff --git a/src/util/config_reader.c b/src/util/config_reader.c index e7c7c1c..6db253f 100644 --- a/src/util/config_reader.c +++ b/src/util/config_reader.c @@ -6,6 +6,7 @@ #include "util/config_reader.h" #include "app_context.h" #include "util/logger.h" +#include "util/ptp_clock.h" #include #include #include @@ -428,13 +429,18 @@ int parse_tx_config(const char* config_file, struct dvledtx_config* config) { if (hwaccel_val >= 0) config->hwaccel = (hwaccel_val != 0); } - /* PTP hardware timing (built-in MTL PTP client). - * Disabled by default: PTP-paced TX requires a PTP grandmaster on the - * network, and without one the TX pacing clock never locks. Enable via the + /* PTP hardware timing. + * Disabled by default: with "mode":"slave" PTP-paced TX requires a PTP + * grandmaster on the network, and without one the TX pacing clock never + * locks. With "mode":"grandmaster" this host's own NIC PHC is the + * reference and no external grandmaster is needed. Enable via the * optional top-level "ptp" block. */ config->ptp_enable = false; + config->ptp_mode = DVLEDTX_PTP_MODE_SLAVE; config->ptp_pi = false; config->ptp_unicast = false; + config->ptp_phc[0] = '\0'; + config->ptp_phc_interval_ms = 0; const char* ptp_obj = find_object(json, buf_end, "ptp"); if (ptp_obj != NULL) { const char* ptp_end = find_object_end(ptp_obj, buf_end); @@ -443,6 +449,32 @@ int parse_tx_config(const char* config_file, struct dvledtx_config* config) { b = extract_json_bool(ptp_obj, ptp_end, "enable"); if (b >= 0) config->ptp_enable = (b != 0); b = extract_json_bool(ptp_obj, ptp_end, "pi"); if (b >= 0) config->ptp_pi = (b != 0); b = extract_json_bool(ptp_obj, ptp_end, "unicast"); if (b >= 0) config->ptp_unicast = (b != 0); + + char mode[32] = {0}; + if (extract_json_string(ptp_obj, ptp_end, "mode", mode, sizeof(mode)) != NULL && + mode[0] != '\0') { + if (strcmp(mode, "grandmaster") == 0) { + config->ptp_mode = DVLEDTX_PTP_MODE_GRANDMASTER; + } else if (strcmp(mode, "slave") == 0) { + config->ptp_mode = DVLEDTX_PTP_MODE_SLAVE; + } else { + LOG_ERROR("Invalid ptp.mode '%s' (expected \"slave\" or \"grandmaster\")", mode); + free(json); + return -1; + } + } + + /* Grandmaster time source: "/dev/ptpN" or the kernel interface that + * owns the PHC. Either key populates the same field. */ + extract_json_string(ptp_obj, ptp_end, "phc_device", config->ptp_phc, + sizeof(config->ptp_phc)); + if (config->ptp_phc[0] == '\0') + extract_json_string(ptp_obj, ptp_end, "phc_interface", config->ptp_phc, + sizeof(config->ptp_phc)); + + config->ptp_phc_interval_ms = + extract_json_int(ptp_obj, ptp_end, "phc_interval_ms"); + if (config->ptp_phc_interval_ms < 0) config->ptp_phc_interval_ms = 0; } /* --- tx_sessions array --- */ @@ -630,6 +662,30 @@ int validate_tx_config(const struct dvledtx_config* config) { } regfree(&bdf_regex); + /* PTP block validation */ + if (config->ptp_mode == DVLEDTX_PTP_MODE_GRANDMASTER) { + if (!config->ptp_enable) { + LOG_WARN("ptp.mode is 'grandmaster' but ptp.enable is false — " + "PTP is off and TX falls back to TSC pacing"); + } + if (config->ptp_pi || config->ptp_unicast) { + LOG_WARN("ptp.pi / ptp.unicast only apply to the built-in PTP client " + "(mode 'slave') and are ignored in grandmaster mode"); + } + if (config->ptp_phc[0] != '\0' && !ptp_clock_valid_device_ref(config->ptp_phc)) { + LOG_ERROR("invalid ptp.phc_device/phc_interface '%s' " + "(expected '/dev/ptpN' or a kernel interface name)", + config->ptp_phc); + return -1; + } + if (config->ptp_phc_interval_ms != 0 && + (config->ptp_phc_interval_ms < 100 || config->ptp_phc_interval_ms > 10000)) { + LOG_ERROR("ptp.phc_interval_ms %d is out of range (100..10000, or 0 for default)", + config->ptp_phc_interval_ms); + return -1; + } + } + /* Video resolution validation */ if (config->width == 0 || config->height == 0) { LOG_ERROR("video width/height must be non-zero"); @@ -939,15 +995,35 @@ int load_and_apply_config(struct dvledtx_context* app, const char* config_file) app->log_file[sizeof(app->log_file) - 1] = '\0'; } - /* PTP hardware timing (built-in MTL PTP client) */ + /* PTP hardware timing */ app->ptp_enable = config.ptp_enable; + app->ptp_mode = config.ptp_mode; app->ptp_pi = config.ptp_pi; app->ptp_unicast = config.ptp_unicast; - if (app->ptp_enable) - LOG_INFO("PTP enabled: pi_controller=%d unicast_delay_req=%d", - app->ptp_pi, app->ptp_unicast); - else + app->ptp_phc_interval_ms = config.ptp_phc_interval_ms; + strncpy(app->ptp_phc, config.ptp_phc, sizeof(app->ptp_phc) - 1); + app->ptp_phc[sizeof(app->ptp_phc) - 1] = '\0'; + if (!app->ptp_enable) LOG_INFO("PTP disabled (default TSC-based TX pacing)"); + else if (app->ptp_mode == DVLEDTX_PTP_MODE_GRANDMASTER) + LOG_INFO("PTP enabled: mode=grandmaster phc=%s resample=%d ms", + app->ptp_phc[0] ? app->ptp_phc : "", + app->ptp_phc_interval_ms); + else + LOG_INFO("PTP enabled: mode=slave pi_controller=%d unicast_delay_req=%d", + app->ptp_pi, app->ptp_unicast); + +#ifndef ENABLE_MTL_TX + /* On the mtl_st20p muxer path the plugin owns the mtl_init() call and + * ptp_get_time_fn has no AVOption equivalent, so the PHC could never reach + * MTL. Fail here rather than transmitting on the wrong clock. */ + if (app->ptp_enable && app->ptp_mode == DVLEDTX_PTP_MODE_GRANDMASTER) { + LOG_ERROR("ptp.mode 'grandmaster' requires the direct MTL TX build; " + "rebuild with -Denable_mtl_tx=true"); + dvledtx_config_free(&config); + return -1; + } +#endif LOG_INFO("Config loaded: %s (%d NIC(s), %d session(s))", config_file, config.nic_count, config.session_count); diff --git a/src/util/ptp_clock.c b/src/util/ptp_clock.c new file mode 100644 index 0000000..5155cce --- /dev/null +++ b/src/util/ptp_clock.c @@ -0,0 +1,442 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright 2026 Intel Corporation + */ + +/* + * ptp_clock.c — NIC PTP Hardware Clock (PHC) time source for MTL. + * + * See include/util/ptp_clock.h for the role this plays in "grandmaster" PTP + * mode. + * + * Model + * ----- + * A background thread samples the PHC every interval_ms and maintains + * + * phc_ns(t) ~= anchor_phc + (mono_ns(t) - anchor_mono) * rate + * + * where mono_ns is CLOCK_MONOTONIC_RAW. Each sample re-anchors on the + * *estimated* value rather than the measured one, and folds the measured + * error into `rate` for the next interval, so the served clock stays + * continuous and monotonic instead of stepping once per second. A measured + * error larger than PTP_STEP_NS (ptp4l stepping the PHC, or a resumed host) + * is applied as an explicit step, since slewing it would take minutes. + * + * The model is published through a small ring of slots indexed by an atomic + * counter: readers never block and never take a lock on the dataplane path, + * and with a >=100 ms publish interval a reader would have to stall for + * PTP_MODEL_SLOTS intervals to observe a slot being rewritten. + */ + +#include "util/ptp_clock.h" +#include "util/logger.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* Turn a /dev/ptpN descriptor into a POSIX dynamic clock id (linux/ptp_clock.h + * documents this encoding; it is not exported by glibc). */ +#define PTP_FD_TO_CLOCKID(fd) ((~(clockid_t)(fd) << 3) | 3) + +#define PTP_DEFAULT_INTERVAL_MS 1000u +#define PTP_MIN_INTERVAL_MS 100u +#define PTP_MAX_INTERVAL_MS 10000u + +#define PTP_MODEL_SLOTS 8u +#define PTP_SAMPLE_TRIES 5 +#define PTP_SAMPLE_MAX_LAT_NS 50000ull /* reject a PHC read slower than 50us */ +#define PTP_STEP_NS 1000000ll /* >1ms error: step instead of slew */ +#define PTP_MAX_RATE_DEV 0.001 /* clamp model rate to +/-1000 ppm */ +#define PTP_RATE_EMA 0.25 /* smoothing for the measured rate */ +#define PTP_LOG_PERIOD_S 30 + +struct ptp_model { + uint64_t anchor_mono_ns; + uint64_t anchor_phc_ns; + double rate; /* PHC nanoseconds per CLOCK_MONOTONIC_RAW nanosecond */ +}; + +struct ptp_clock { + int fd; + clockid_t clkid; + char device[64]; + unsigned int interval_ms; + + /* Published model: readers load `gen`, then read slot gen % PTP_MODEL_SLOTS. */ + _Atomic uint64_t gen; + struct ptp_model slots[PTP_MODEL_SLOTS]; + + /* Sampler thread */ + pthread_t thread; + bool thread_started; + _Atomic bool stop; + + /* Sampler-private servo state */ + double base_rate; + uint64_t prev_mono_ns; + uint64_t prev_phc_ns; + bool have_model; + + /* Statistics (published for ptp_clock_stats) */ + _Atomic int64_t stat_offset_ns; + _Atomic int64_t stat_freq_ppb; + _Atomic uint64_t stat_samples; +}; + +static inline uint64_t ts_to_ns(const struct timespec* ts) { + return (uint64_t)ts->tv_sec * 1000000000ull + (uint64_t)ts->tv_nsec; +} + +static inline uint64_t mono_now_ns(void) { + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC_RAW, &ts); + return ts_to_ns(&ts); +} + +/* ------------------------------------------------------------------ */ +/* Device resolution */ +/* ------------------------------------------------------------------ */ + +/* "/dev/ptp" followed by digits only — the value comes from a config file, so + * reject anything that could point the open() somewhere else. */ +static bool is_ptp_device_path(const char* s) { + const char* prefix = "/dev/ptp"; + size_t plen = strlen(prefix); + if (strncmp(s, prefix, plen) != 0) return false; + if (s[plen] == '\0') return false; + for (const char* p = s + plen; *p != '\0'; p++) + if (!isdigit((unsigned char)*p)) return false; + return true; +} + +static bool is_iface_name(const char* s) { + size_t len = strlen(s); + if (len == 0 || len >= IFNAMSIZ) return false; + for (const char* p = s; *p != '\0'; p++) { + if (!isalnum((unsigned char)*p) && *p != '-' && *p != '_' && *p != '.') + return false; + } + return true; +} + +bool ptp_clock_valid_device_ref(const char* ref) { + if (ref == NULL) return false; + return is_ptp_device_path(ref) || is_iface_name(ref); +} + +/* PHC index backing a kernel network interface, or -1. */ +static int phc_index_for_iface(const char* iface) { + struct ethtool_ts_info info; + struct ifreq ifr; + + memset(&info, 0, sizeof(info)); + memset(&ifr, 0, sizeof(ifr)); + info.cmd = ETHTOOL_GET_TS_INFO; + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", iface); + ifr.ifr_data = (char*)&info; + + int sock = socket(AF_INET, SOCK_DGRAM, 0); + if (sock < 0) return -1; + int ret = ioctl(sock, SIOCETHTOOL, &ifr); + close(sock); + if (ret < 0) return -1; + return info.phc_index; +} + +/* Exactly one /dev/ptpN on the host — otherwise the choice is ambiguous and + * the operator must name it. Returns the index or -1. */ +static int phc_index_autodetect(void) { + DIR* dir = opendir("/sys/class/ptp"); + if (dir == NULL) return -1; + + int found = -1; + int count = 0; + struct dirent* ent; + while ((ent = readdir(dir)) != NULL) { + if (strncmp(ent->d_name, "ptp", 3) != 0) continue; + char* endp = NULL; + long idx = strtol(ent->d_name + 3, &endp, 10); + if (endp == NULL || *endp != '\0' || idx < 0) continue; + count++; + found = (int)idx; + } + closedir(dir); + + return count == 1 ? found : -1; +} + +static int resolve_device(const char* device, char* out, size_t out_size) { + if (device == NULL || device[0] == '\0') { + int idx = phc_index_autodetect(); + if (idx < 0) { + LOG_ERROR("PTP clock: cannot auto-select a PHC (zero or multiple present); " + "set ptp.phc_device or ptp.phc_interface"); + return -1; + } + snprintf(out, out_size, "/dev/ptp%d", idx); + return 0; + } + + if (is_ptp_device_path(device)) { + snprintf(out, out_size, "%s", device); + return 0; + } + + if (is_iface_name(device)) { + int idx = phc_index_for_iface(device); + if (idx < 0) { + LOG_ERROR("PTP clock: interface '%s' has no PTP hardware clock " + "(not present, or bound to a userspace driver)", device); + return -1; + } + snprintf(out, out_size, "/dev/ptp%d", idx); + return 0; + } + + LOG_ERROR("PTP clock: invalid phc device '%s' " + "(expected '/dev/ptpN' or a kernel interface name)", device); + return -1; +} + +/* ------------------------------------------------------------------ */ +/* Model publish / load */ +/* ------------------------------------------------------------------ */ + +static void model_publish(struct ptp_clock* clk, const struct ptp_model* m) { + uint64_t gen = atomic_load_explicit(&clk->gen, memory_order_relaxed); + clk->slots[(gen + 1) % PTP_MODEL_SLOTS] = *m; + atomic_store_explicit(&clk->gen, gen + 1, memory_order_release); +} + +/* ------------------------------------------------------------------ */ +/* Sampling */ +/* ------------------------------------------------------------------ */ + +/* Best-of-N paired read of CLOCK_MONOTONIC_RAW and the PHC, keeping the pair + * with the shortest PHC read window and midpointing it. */ +static int phc_sample(clockid_t clkid, uint64_t* mono_ns, uint64_t* phc_ns) { + uint64_t best_lat = UINT64_MAX; + + for (int i = 0; i < PTP_SAMPLE_TRIES; i++) { + struct timespec t0, phc, t1; + if (clock_gettime(CLOCK_MONOTONIC_RAW, &t0) != 0) return -1; + if (clock_gettime(clkid, &phc) != 0) return -1; + if (clock_gettime(CLOCK_MONOTONIC_RAW, &t1) != 0) return -1; + + uint64_t a = ts_to_ns(&t0), b = ts_to_ns(&t1); + if (b < a) continue; + uint64_t lat = b - a; + if (lat < best_lat) { + best_lat = lat; + *mono_ns = a + lat / 2; + *phc_ns = ts_to_ns(&phc); + } + } + + return (best_lat <= PTP_SAMPLE_MAX_LAT_NS) ? 0 : -1; +} + +static inline double clamp_rate(double rate) { + if (rate < 1.0 - PTP_MAX_RATE_DEV) return 1.0 - PTP_MAX_RATE_DEV; + if (rate > 1.0 + PTP_MAX_RATE_DEV) return 1.0 + PTP_MAX_RATE_DEV; + return rate; +} + +static void sampler_update(struct ptp_clock* clk, uint64_t mono_ns, uint64_t phc_ns) { + struct ptp_model next; + + if (!clk->have_model) { + next.anchor_mono_ns = mono_ns; + next.anchor_phc_ns = phc_ns; + next.rate = clk->base_rate; + clk->have_model = true; + atomic_store_explicit(&clk->stat_offset_ns, 0, memory_order_relaxed); + } else { + const struct ptp_model* cur = + &clk->slots[atomic_load_explicit(&clk->gen, memory_order_relaxed) % PTP_MODEL_SLOTS]; + uint64_t est = cur->anchor_phc_ns + + (uint64_t)llround((double)(mono_ns - cur->anchor_mono_ns) * cur->rate); + int64_t err = (int64_t)(phc_ns - est); + uint64_t elapsed = mono_ns - clk->prev_mono_ns; + + atomic_store_explicit(&clk->stat_offset_ns, err, memory_order_relaxed); + + if (llabs(err) > PTP_STEP_NS || elapsed == 0) { + LOG_WARN("PTP clock: stepping local model by %" PRId64 " ns (PHC jump or stall)", err); + next.anchor_mono_ns = mono_ns; + next.anchor_phc_ns = phc_ns; + next.rate = clk->base_rate; + } else { + double measured = (double)(phc_ns - clk->prev_phc_ns) / (double)elapsed; + if (fabs(measured - 1.0) < PTP_MAX_RATE_DEV) + clk->base_rate += PTP_RATE_EMA * (measured - clk->base_rate); + clk->base_rate = clamp_rate(clk->base_rate); + + /* Re-anchor on the estimate, not the measurement, so the served clock is + * continuous; the error is absorbed over the next interval instead. */ + next.anchor_mono_ns = mono_ns; + next.anchor_phc_ns = est; + next.rate = clamp_rate(clk->base_rate + (double)err / (double)elapsed); + } + } + + atomic_store_explicit(&clk->stat_freq_ppb, + (int64_t)llround((next.rate - 1.0) * 1e9), memory_order_relaxed); + model_publish(clk, &next); + + clk->prev_mono_ns = mono_ns; + clk->prev_phc_ns = phc_ns; + atomic_fetch_add_explicit(&clk->stat_samples, 1, memory_order_relaxed); +} + +static void* sampler_thread(void* arg) { + struct ptp_clock* clk = (struct ptp_clock*)arg; + unsigned int since_log_ms = 0; + unsigned int consecutive_failures = 0; + + while (!atomic_load_explicit(&clk->stop, memory_order_acquire)) { + uint64_t mono_ns = 0, phc_ns = 0; + if (phc_sample(clk->clkid, &mono_ns, &phc_ns) == 0) { + consecutive_failures = 0; + sampler_update(clk, mono_ns, phc_ns); + } else if (++consecutive_failures == 5) { + LOG_WARN("PTP clock: %s unreadable or read latency too high; " + "TX timing is coasting on the last known rate", clk->device); + } + + since_log_ms += clk->interval_ms; + if (since_log_ms >= PTP_LOG_PERIOD_S * 1000u) { + since_log_ms = 0; + LOG_INFO("PTP clock: %s offset=%" PRId64 " ns freq=%" PRId64 " ppb samples=%" PRIu64, + clk->device, + atomic_load_explicit(&clk->stat_offset_ns, memory_order_relaxed), + atomic_load_explicit(&clk->stat_freq_ppb, memory_order_relaxed), + atomic_load_explicit(&clk->stat_samples, memory_order_relaxed)); + } + + struct timespec req = {.tv_sec = clk->interval_ms / 1000, + .tv_nsec = (long)(clk->interval_ms % 1000) * 1000000L}; + nanosleep(&req, NULL); + } + + return NULL; +} + +/* ------------------------------------------------------------------ */ +/* Public API */ +/* ------------------------------------------------------------------ */ + +struct ptp_clock* ptp_clock_open(const char* device, unsigned int interval_ms) { + char path[64]; + if (resolve_device(device, path, sizeof(path)) != 0) return NULL; + + if (interval_ms == 0) interval_ms = PTP_DEFAULT_INTERVAL_MS; + if (interval_ms < PTP_MIN_INTERVAL_MS) interval_ms = PTP_MIN_INTERVAL_MS; + if (interval_ms > PTP_MAX_INTERVAL_MS) interval_ms = PTP_MAX_INTERVAL_MS; + + struct ptp_clock* clk = calloc(1, sizeof(*clk)); + if (clk == NULL) { + LOG_ERROR("PTP clock: allocation failed"); + return NULL; + } + + clk->fd = open(path, O_RDONLY | O_CLOEXEC); + if (clk->fd < 0) { + LOG_ERROR("PTP clock: cannot open %s (%s)", path, strerror(errno)); + free(clk); + return NULL; + } + + clk->clkid = PTP_FD_TO_CLOCKID(clk->fd); + clk->interval_ms = interval_ms; + clk->base_rate = 1.0; + snprintf(clk->device, sizeof(clk->device), "%s", path); + atomic_init(&clk->gen, 0); + atomic_init(&clk->stop, false); + atomic_init(&clk->stat_offset_ns, 0); + atomic_init(&clk->stat_freq_ppb, 0); + atomic_init(&clk->stat_samples, 0); + + uint64_t mono_ns = 0, phc_ns = 0; + if (phc_sample(clk->clkid, &mono_ns, &phc_ns) != 0) { + LOG_ERROR("PTP clock: %s is not readable as a POSIX clock", path); + close(clk->fd); + free(clk); + return NULL; + } + clk->slots[0] = (struct ptp_model){ + .anchor_mono_ns = mono_ns, .anchor_phc_ns = phc_ns, .rate = 1.0}; + clk->have_model = true; + clk->prev_mono_ns = mono_ns; + clk->prev_phc_ns = phc_ns; + atomic_store_explicit(&clk->stat_samples, 1, memory_order_relaxed); + + if (pthread_create(&clk->thread, NULL, sampler_thread, clk) != 0) { + LOG_ERROR("PTP clock: cannot start sampler thread (%s)", strerror(errno)); + close(clk->fd); + free(clk); + return NULL; + } + clk->thread_started = true; + + LOG_INFO("PTP clock: using %s as TX time source (resample %u ms, start %" PRIu64 " ns)", + path, interval_ms, phc_ns); + return clk; +} + +void ptp_clock_close(struct ptp_clock* clk) { + if (clk == NULL) return; + + if (clk->thread_started) { + atomic_store_explicit(&clk->stop, true, memory_order_release); + pthread_join(clk->thread, NULL); + } + if (clk->fd >= 0) close(clk->fd); + free(clk); +} + +uint64_t ptp_clock_get_time_ns(void* priv) { + struct ptp_clock* clk = (struct ptp_clock*)priv; + if (clk == NULL) return 0; + + uint64_t gen = atomic_load_explicit(&clk->gen, memory_order_acquire); + const struct ptp_model* m = &clk->slots[gen % PTP_MODEL_SLOTS]; + + uint64_t mono = mono_now_ns(); + if (mono <= m->anchor_mono_ns) return m->anchor_phc_ns; + return m->anchor_phc_ns + (uint64_t)((double)(mono - m->anchor_mono_ns) * m->rate); +} + +const char* ptp_clock_device(const struct ptp_clock* clk) { + return clk != NULL ? clk->device : ""; +} + +int ptp_clock_stats(const struct ptp_clock* clk, int64_t* offset_ns, + int64_t* freq_ppb, uint64_t* samples) { + if (clk == NULL) return -1; + if (offset_ns != NULL) + *offset_ns = atomic_load_explicit(&clk->stat_offset_ns, memory_order_relaxed); + if (freq_ppb != NULL) + *freq_ppb = atomic_load_explicit(&clk->stat_freq_ppb, memory_order_relaxed); + if (samples != NULL) + *samples = atomic_load_explicit(&clk->stat_samples, memory_order_relaxed); + return 0; +} diff --git a/tests/meson.build b/tests/meson.build index aa6aa60..30b1b38 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -34,6 +34,7 @@ test_config_reader = executable('test_config_reader', 'test_config_reader.c', '../src/util/config_reader.c', '../src/util/logger.c', + '../src/util/ptp_clock.c', ], include_directories: test_inc, dependencies: test_deps, diff --git a/tests/test_config_reader.c b/tests/test_config_reader.c index ba8f445..0fdc2a0 100644 --- a/tests/test_config_reader.c +++ b/tests/test_config_reader.c @@ -23,6 +23,7 @@ #include #include "util/config_reader.h" +#include "util/ptp_clock.h" #include "app_context.h" /* -------------------------------------------------------------------------- @@ -532,6 +533,134 @@ static void test_parse_ptp_key_without_colon_keeps_default(void **state) dvledtx_config_free(&cfg); } +/* ========================================================================== + * PTP "mode" / grandmaster time source + * ========================================================================== */ + +/* Absent "mode" keeps the historical behaviour: slave to an external GM. */ +static void test_parse_ptp_mode_defaults_to_slave(void **state) +{ + (void)state; + char *path = write_ptp_config("\"ptp\": {\"enable\":true},"); + assert_non_null(path); + + struct dvledtx_config cfg; + int ret = parse_tx_config(path, &cfg); + unlink(path); free(path); + assert_int_equal(ret, 0); + assert_int_equal(cfg.ptp_mode, DVLEDTX_PTP_MODE_SLAVE); + assert_string_equal(cfg.ptp_phc, ""); + assert_int_equal(cfg.ptp_phc_interval_ms, 0); + dvledtx_config_free(&cfg); +} + +static void test_parse_ptp_mode_grandmaster_with_interface(void **state) +{ + (void)state; + char *path = write_ptp_config( + "\"ptp\": {\"enable\":true,\"mode\":\"grandmaster\"," + "\"phc_interface\":\"enp4s0\",\"phc_interval_ms\":500},"); + assert_non_null(path); + + struct dvledtx_config cfg; + int ret = parse_tx_config(path, &cfg); + unlink(path); free(path); + assert_int_equal(ret, 0); + assert_true(cfg.ptp_enable); + assert_int_equal(cfg.ptp_mode, DVLEDTX_PTP_MODE_GRANDMASTER); + assert_string_equal(cfg.ptp_phc, "enp4s0"); + assert_int_equal(cfg.ptp_phc_interval_ms, 500); + dvledtx_config_free(&cfg); +} + +/* phc_device takes precedence over phc_interface — both land in ptp_phc. */ +static void test_parse_ptp_phc_device_preferred_over_interface(void **state) +{ + (void)state; + char *path = write_ptp_config( + "\"ptp\": {\"enable\":true,\"mode\":\"grandmaster\"," + "\"phc_device\":\"/dev/ptp3\",\"phc_interface\":\"enp4s0\"},"); + assert_non_null(path); + + struct dvledtx_config cfg; + int ret = parse_tx_config(path, &cfg); + unlink(path); free(path); + assert_int_equal(ret, 0); + assert_string_equal(cfg.ptp_phc, "/dev/ptp3"); + dvledtx_config_free(&cfg); +} + +static void test_parse_ptp_mode_unknown_is_rejected(void **state) +{ + (void)state; + char *path = write_ptp_config("\"ptp\": {\"enable\":true,\"mode\":\"master\"},"); + assert_non_null(path); + + struct dvledtx_config cfg; + int ret = parse_tx_config(path, &cfg); + unlink(path); free(path); + assert_int_equal(ret, -1); +} + +/* The PHC reference comes from a config file, so anything that is not a + * /dev/ptpN path or a kernel interface name must be refused before open(). */ +static void test_ptp_clock_device_ref_validation(void **state) +{ + (void)state; + assert_true(ptp_clock_valid_device_ref("/dev/ptp0")); + assert_true(ptp_clock_valid_device_ref("/dev/ptp11")); + assert_true(ptp_clock_valid_device_ref("enp4s0")); + assert_true(ptp_clock_valid_device_ref("eth0.100")); + + assert_false(ptp_clock_valid_device_ref(NULL)); + assert_false(ptp_clock_valid_device_ref("")); + assert_false(ptp_clock_valid_device_ref("/dev/ptp")); + assert_false(ptp_clock_valid_device_ref("/dev/ptp0x")); + assert_false(ptp_clock_valid_device_ref("/dev/../etc/passwd")); + assert_false(ptp_clock_valid_device_ref("../../dev/ptp0")); + assert_false(ptp_clock_valid_device_ref("eth0; reboot")); +} + +static void test_validate_grandmaster_rejects_bad_phc_ref(void **state) +{ + (void)state; + struct dvledtx_config cfg; + fill_valid_config(&cfg); + cfg.ptp_enable = true; + cfg.ptp_mode = DVLEDTX_PTP_MODE_GRANDMASTER; + snprintf(cfg.ptp_phc, sizeof(cfg.ptp_phc), "/etc/shadow"); + assert_int_equal(validate_tx_config(&cfg), -1); + dvledtx_config_free(&cfg); +} + +static void test_validate_grandmaster_rejects_out_of_range_interval(void **state) +{ + (void)state; + struct dvledtx_config cfg; + fill_valid_config(&cfg); + cfg.ptp_enable = true; + cfg.ptp_mode = DVLEDTX_PTP_MODE_GRANDMASTER; + cfg.ptp_phc_interval_ms = 50; + assert_int_equal(validate_tx_config(&cfg), -1); + + cfg.ptp_phc_interval_ms = 20000; + assert_int_equal(validate_tx_config(&cfg), -1); + dvledtx_config_free(&cfg); +} + +static void test_validate_grandmaster_accepts_valid_phc(void **state) +{ + (void)state; + struct dvledtx_config cfg; + fill_valid_config(&cfg); + cfg.ptp_enable = true; + cfg.ptp_mode = DVLEDTX_PTP_MODE_GRANDMASTER; + snprintf(cfg.ptp_phc, sizeof(cfg.ptp_phc), "/dev/ptp0"); + cfg.ptp_phc_interval_ms = 1000; + assert_int_equal(validate_tx_config(&cfg), 0); + dvledtx_config_free(&cfg); +} + /* ========================================================================== * Optional "hwaccel" flag (hardware decode) * @@ -2095,6 +2224,14 @@ int main(void) cmocka_unit_test(test_parse_ptp_numeric_value_falls_back_to_default), cmocka_unit_test(test_parse_ptp_string_value_falls_back_to_default), cmocka_unit_test(test_parse_ptp_key_without_colon_keeps_default), + cmocka_unit_test(test_parse_ptp_mode_defaults_to_slave), + cmocka_unit_test(test_parse_ptp_mode_grandmaster_with_interface), + cmocka_unit_test(test_parse_ptp_phc_device_preferred_over_interface), + cmocka_unit_test(test_parse_ptp_mode_unknown_is_rejected), + cmocka_unit_test(test_ptp_clock_device_ref_validation), + cmocka_unit_test(test_validate_grandmaster_rejects_bad_phc_ref), + cmocka_unit_test(test_validate_grandmaster_rejects_out_of_range_interval), + cmocka_unit_test(test_validate_grandmaster_accepts_valid_phc), cmocka_unit_test(test_parse_hwaccel_absent_defaults_false), cmocka_unit_test(test_parse_hwaccel_true), cmocka_unit_test(test_parse_hwaccel_false), From 6f8bdd5a18951f95ea8033f73969e042ffea0ae8 Mon Sep 17 00:00:00 2001 From: roshan-ku Date: Tue, 6 Oct 2026 11:31:04 +0530 Subject: [PATCH 2/7] fix(ptp): correct ptp4l config generation and document the VF queue ceiling Found while bringing the feature up end to end on an E610. scripts/ptp_grandmaster.sh: - linuxptp 4.0 renamed masterOnly to serverOnly and warns on the old spelling. Detect the major version from `ptp4l -v` and emit the right key, keeping masterOnly for linuxptp 3.x. - The script ends in `exec ptp4l`, which replaces the shell, so the `trap ... EXIT` that was meant to clean up the mktemp config could never fire and every invocation leaked a file in /tmp. Write a fixed /tmp/ptp4l-gm-.conf that overwrites instead of accumulating. README: document that sessions per VF are bounded by the NIC's TX queues. MTL requests sessions+2, silently clamps to what the device provides, and only fails later with "fail to find free tx queue". An E610 VF has 4 TX queues and MTL reserves one, so 3 sessions per VF is the ceiling. Add two configs covering both sides of that limit: 3 sessions on a single VF (at the ceiling) and 4 sessions spread one-per-VF (scaling past it). Validated on an E610: all sessions report "pacing way: ptp", the PHC servo settles to tens of ns with a steady ~40 ppm correction, and frame get/succ/put match with drop 0. --- README.md | 8 ++++++ config/tx_ptp_gm_3session.json | 48 +++++++++++++++++++++++++++++++ config/tx_ptp_gm_4session.json | 52 ++++++++++++++++++++++++++++++++++ scripts/ptp_grandmaster.sh | 17 ++++++++--- 4 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 config/tx_ptp_gm_3session.json create mode 100644 config/tx_ptp_gm_4session.json diff --git a/README.md b/README.md index 76b17ad..eabb82a 100644 --- a/README.md +++ b/README.md @@ -334,6 +334,14 @@ The second works because MTL never touches the TX port's own clock in this mode For the separate-port topology, skip step 1 and run `ptp4l` on the spare port instead; the TX NIC stays entirely under DPDK. +> **Sessions per VF are limited by the NIC's TX queues.** MTL requests `sessions + 2` TX queues but +> silently clamps to what the device provides, then fails later with the unhelpful +> `mt_dev_get_tx_queue(0), fail to find free tx queue`. An Intel E610 VF exposes 4 TX queues and MTL +> reserves one for system traffic, so **3 sessions per VF is the ceiling**. Spread further sessions +> across additional VFs — one session per VF, as in `config/tx_ptp_gm_4session.json`. Check the +> startup log: `MTL init: port[0]=... tx_queues=5` is the request, +> `dev_config_port(0), tx_q(4 ...)` is what the hardware actually gave. + **Verifying it works** On a **receiver**, confirm which clock actually won the election: diff --git a/config/tx_ptp_gm_3session.json b/config/tx_ptp_gm_3session.json new file mode 100644 index 0000000..10cedd8 --- /dev/null +++ b/config/tx_ptp_gm_3session.json @@ -0,0 +1,48 @@ +{ + "log_file": "dvledtx_ptp_3session.log", + "interfaces": [ + { + "nic_index": 0, + "name": "0000:04:10.0", + "sip": "192.168.50.29", + "dip": "239.168.85.20" + } + ], + "video": { + "width": 1920, + "height": 1080, + "tx_url": "bbb_sunflower_1080p_30fps_normal.mp4" + }, + "tx_video": { + "scale_width": 1920, + "scale_height": 1080, + "fps": 30, + "fmt": "yuv422p10le" + }, + "ptp": { + "enable": true, + "mode": "grandmaster", + "phc_interface": "enp4s0", + "phc_interval_ms": 1000 + }, + "tx_sessions": [ + { + "nic_index": 0, + "udp_port": 20000, + "payload_type": 96, + "crop": { "x": 0, "y": 0, "w": 640, "h": 1080 } + }, + { + "nic_index": 0, + "udp_port": 20002, + "payload_type": 96, + "crop": { "x": 640, "y": 0, "w": 640, "h": 1080 } + }, + { + "nic_index": 0, + "udp_port": 20004, + "payload_type": 96, + "crop": { "x": 1280, "y": 0, "w": 640, "h": 1080 } + } + ] +} diff --git a/config/tx_ptp_gm_4session.json b/config/tx_ptp_gm_4session.json new file mode 100644 index 0000000..8fae016 --- /dev/null +++ b/config/tx_ptp_gm_4session.json @@ -0,0 +1,52 @@ +{ + "log_file": "dvledtx_ptp_4session.log", + "interfaces": [ + { "nic_index": 0, "name": "0000:04:10.0", "sip": "192.168.50.29", "dip": "239.168.85.20" }, + { "nic_index": 1, "name": "0000:04:10.2", "sip": "192.168.50.30", "dip": "239.168.85.20" }, + { "nic_index": 2, "name": "0000:04:10.4", "sip": "192.168.50.31", "dip": "239.168.85.20" }, + { "nic_index": 3, "name": "0000:04:10.6", "sip": "192.168.50.32", "dip": "239.168.85.20" } + ], + "video": { + "width": 1920, + "height": 1080, + "tx_url": "bbb_sunflower_1080p_30fps_normal.mp4" + }, + "tx_video": { + "scale_width": 1920, + "scale_height": 1080, + "fps": 30, + "fmt": "yuv422p10le" + }, + "ptp": { + "enable": true, + "mode": "grandmaster", + "phc_interface": "enp4s0", + "phc_interval_ms": 1000 + }, + "tx_sessions": [ + { + "nic_index": 0, + "udp_port": 20000, + "payload_type": 96, + "crop": { "x": 0, "y": 0, "w": 960, "h": 540 } + }, + { + "nic_index": 1, + "udp_port": 20002, + "payload_type": 96, + "crop": { "x": 960, "y": 0, "w": 960, "h": 540 } + }, + { + "nic_index": 2, + "udp_port": 20004, + "payload_type": 96, + "crop": { "x": 0, "y": 540, "w": 960, "h": 540 } + }, + { + "nic_index": 3, + "udp_port": 20006, + "payload_type": 96, + "crop": { "x": 960, "y": 540, "w": 960, "h": 540 } + } + ] +} diff --git a/scripts/ptp_grandmaster.sh b/scripts/ptp_grandmaster.sh index ff2a3b7..dd1a394 100755 --- a/scripts/ptp_grandmaster.sh +++ b/scripts/ptp_grandmaster.sh @@ -39,10 +39,19 @@ if [[ -z "${PHC_INDEX}" || "${PHC_INDEX}" == "none" || "${PHC_INDEX}" == "-1" ]] exit 1 fi -CONF="$(mktemp /tmp/ptp4l-gm-XXXXXX.conf)" -trap 'rm -f "${CONF}"' EXIT +# Fixed path, not mktemp: the final exec replaces this shell so an EXIT trap +# would never fire, and repeated runs would leak a file each time. +CONF="/tmp/ptp4l-gm-${IFACE}.conf" -# priority1 below the switch's keeps BMCA on this host; masterOnly stops ptp4l +# linuxptp 4.0 renamed masterOnly to serverOnly and warns on the old spelling. +PTP4L_MAJOR="$(ptp4l -v 2>&1 | head -1 | sed -E 's/[^0-9]*([0-9]+).*/\1/')" +if [[ -n "${PTP4L_MAJOR}" && "${PTP4L_MAJOR}" -ge 4 ]]; then + SERVER_ONLY_KEY="serverOnly" +else + SERVER_ONLY_KEY="masterOnly" +fi + +# priority1 below the switch's keeps BMCA on this host; serverOnly stops ptp4l # from ever slaving to the switch's GPS-less grandmaster. cat >"${CONF}" < Date: Tue, 6 Oct 2026 12:17:57 +0530 Subject: [PATCH 3/7] fix(ptp): use L2 transport for the grandmaster DPDK's ixgbe_timesync_enable only installs an ethertype 0x88F7 filter, with no UDP-319 timestamping. ptp4l defaults to network_transport UDPv4, so an MTL receiver could never hardware-timestamp our Sync messages. Verified on an E610: over UDPv4 the receiver's PTP never locks; on L2 it reaches SLAVE state and converges to 31 ns rms. --- scripts/ptp_grandmaster.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/ptp_grandmaster.sh b/scripts/ptp_grandmaster.sh index dd1a394..3774b61 100755 --- a/scripts/ptp_grandmaster.sh +++ b/scripts/ptp_grandmaster.sh @@ -53,12 +53,19 @@ fi # priority1 below the switch's keeps BMCA on this host; serverOnly stops ptp4l # from ever slaving to the switch's GPS-less grandmaster. +# +# network_transport L2 is required, not cosmetic: DPDK's ixgbe timesync only +# installs an ethertype 0x88F7 filter (ixgbe_timesync_enable), so a receiver +# using MTL's built-in PTP client can only hardware-timestamp L2 PTP. Over the +# UDPv4 default the receiver falls back to software timestamps, or fails with +# ptp_timesync_read_rx_time err -22. cat >"${CONF}" < Date: Tue, 6 Oct 2026 13:59:26 +0530 Subject: [PATCH 4/7] docs(ptp): explain why the grandmaster must use L2 transport aa56dff added network_transport L2 to the script but documented it nowhere, so anyone writing their own ptp4l config would hit the failure it fixes: DPDK's ixgbe timesync installs only an ethertype 0x88F7 filter and never timestamps PTP over UDP, so a UDPv4 grandmaster runs fine locally while no MTL receiver can ever lock to it. Also note that the script emits serverOnly on linuxptp 4.0+, matching the version detection already in the script. --- README.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index eabb82a..ddc1b8d 100644 --- a/README.md +++ b/README.md @@ -327,7 +327,14 @@ The second works because MTL never touches the TX port's own clock in this mode The script runs `ptp4l` with `masterOnly 1` and `priority1 64`, which keeps BMCA on this host — switch grandmasters normally advertise `priority1 128`, so a lower value wins. `masterOnly` - additionally stops `ptp4l` from ever slaving to the switch. + additionally stops `ptp4l` from ever slaving to the switch. On linuxptp 4.0+ the script emits + `serverOnly` instead, which is the same setting under its current name. + + It also forces `network_transport L2`. This is required, not a preference: DPDK's `ixgbe` + timestamping installs only an ethertype `0x88F7` filter and never timestamps PTP carried over + UDP, so an MTL receiver can never hardware-timestamp a UDPv4 grandmaster. `ptp4l` defaults to + UDPv4, so a hand-rolled config will appear to run correctly on this host while no receiver is + able to lock to it. 4. Leave it running and start dvledtx as usual. From 218cd10d832dfd3ec9ec96aaee42fe0b69e5cbdb Mon Sep 17 00:00:00 2001 From: roshan-ku Date: Tue, 6 Oct 2026 14:21:57 +0530 Subject: [PATCH 5/7] fix(tests): link ptp_clock into targets that build mtl_tx, extend PTP tests The unit test job failed to build. mtl_tx.c now calls ptp_clock_open, ptp_clock_get_time_ns, ptp_clock_device and ptp_clock_close, but ptp_clock.c was only added to the test_config_reader target, so test_mtl_tx, test_mtl_tx_stub and test_session_manager all failed to link with undefined references. scripts/test.sh sends meson and ninja output to /dev/null, so CI showed only "Setting up test build..." followed by exit 1 with no diagnostic. Extend the PTP coverage while here: - explicit mode "slave", and a negative phc_interval_ms clamping to 0 - empty phc reference accepted, since that selects the host's only PHC - interval boundaries 100/10000 accepted and 99/10001 rejected - slave mode ignores the phc fields rather than validating them - interface name length boundary at IFNAMSIZ - ptp_clock_open on a missing device fails, and a NULL handle is safe through every entry point 151 tests pass locally against cmocka 1.1.7. --- tests/meson.build | 3 + tests/test_config_reader.c | 128 +++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/tests/meson.build b/tests/meson.build index 30b1b38..0cc6f84 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -102,6 +102,7 @@ test_session_manager = executable('test_session_manager', 'test_session_manager.c', '../src/core/session_manager.c', '../src/mtl/mtl_tx.c', + '../src/util/ptp_clock.c', '../src/util/logger.c', ], include_directories: test_inc, @@ -179,6 +180,7 @@ if mtl_dep.found() sources: [ 'test_mtl_tx.c', '../src/mtl/mtl_tx.c', + '../src/util/ptp_clock.c', '../src/util/logger.c', ], include_directories: [test_inc, include_directories('/usr/local/include/mtl')], @@ -194,6 +196,7 @@ if mtl_dep.found() sources: [ 'test_mtl_tx_stub.c', '../src/mtl/mtl_tx.c', + '../src/util/ptp_clock.c', '../src/util/logger.c', ], include_directories: [test_inc, include_directories('/usr/local/include/mtl')], diff --git a/tests/test_config_reader.c b/tests/test_config_reader.c index 0fdc2a0..f1a38a4 100644 --- a/tests/test_config_reader.c +++ b/tests/test_config_reader.c @@ -661,6 +661,126 @@ static void test_validate_grandmaster_accepts_valid_phc(void **state) dvledtx_config_free(&cfg); } +/* "slave" is accepted explicitly, not only as the absent-key default. */ +static void test_parse_ptp_mode_slave_explicit(void **state) +{ + (void)state; + char *path = write_ptp_config( + "\"ptp\": {\"enable\":true,\"mode\":\"slave\"},"); + assert_non_null(path); + + struct dvledtx_config cfg; + int ret = parse_tx_config(path, &cfg); + unlink(path); free(path); + assert_int_equal(ret, 0); + assert_int_equal(cfg.ptp_mode, DVLEDTX_PTP_MODE_SLAVE); + dvledtx_config_free(&cfg); +} + +/* A negative interval is clamped to 0 ("use the default") rather than being + * carried through to the validator as a nonsense value. */ +static void test_parse_ptp_negative_interval_clamped_to_zero(void **state) +{ + (void)state; + char *path = write_ptp_config( + "\"ptp\": {\"enable\":true,\"mode\":\"grandmaster\"," + "\"phc_interval_ms\":-500},"); + assert_non_null(path); + + struct dvledtx_config cfg; + int ret = parse_tx_config(path, &cfg); + unlink(path); free(path); + assert_int_equal(ret, 0); + assert_int_equal(cfg.ptp_phc_interval_ms, 0); + dvledtx_config_free(&cfg); +} + +/* An empty phc reference is legal: the module auto-selects the host's only + * PHC, so validation must not demand an explicit device. */ +static void test_validate_grandmaster_accepts_empty_phc_for_autoselect(void **state) +{ + (void)state; + struct dvledtx_config cfg; + fill_valid_config(&cfg); + cfg.ptp_enable = true; + cfg.ptp_mode = DVLEDTX_PTP_MODE_GRANDMASTER; + cfg.ptp_phc[0] = '\0'; + cfg.ptp_phc_interval_ms = 0; + assert_int_equal(validate_tx_config(&cfg), 0); + dvledtx_config_free(&cfg); +} + +/* 100..10000 inclusive, or 0. The edges are where an off-by-one would hide. */ +static void test_validate_grandmaster_interval_boundaries(void **state) +{ + (void)state; + struct dvledtx_config cfg; + fill_valid_config(&cfg); + cfg.ptp_enable = true; + cfg.ptp_mode = DVLEDTX_PTP_MODE_GRANDMASTER; + snprintf(cfg.ptp_phc, sizeof(cfg.ptp_phc), "enp4s0"); + + cfg.ptp_phc_interval_ms = 100; + assert_int_equal(validate_tx_config(&cfg), 0); + cfg.ptp_phc_interval_ms = 10000; + assert_int_equal(validate_tx_config(&cfg), 0); + + cfg.ptp_phc_interval_ms = 99; + assert_int_equal(validate_tx_config(&cfg), -1); + cfg.ptp_phc_interval_ms = 10001; + assert_int_equal(validate_tx_config(&cfg), -1); + dvledtx_config_free(&cfg); +} + +/* The PHC checks are grandmaster-only. In slave mode the fields are unused, + * so stale values left in the config must not fail an otherwise valid run. */ +static void test_validate_slave_mode_ignores_phc_fields(void **state) +{ + (void)state; + struct dvledtx_config cfg; + fill_valid_config(&cfg); + cfg.ptp_enable = true; + cfg.ptp_mode = DVLEDTX_PTP_MODE_SLAVE; + snprintf(cfg.ptp_phc, sizeof(cfg.ptp_phc), "/etc/shadow"); + cfg.ptp_phc_interval_ms = 999999; + assert_int_equal(validate_tx_config(&cfg), 0); + dvledtx_config_free(&cfg); +} + +/* Interface names are bounded by IFNAMSIZ (16 incl. NUL), so 15 characters is + * the longest acceptable name. */ +static void test_ptp_clock_device_ref_length_boundary(void **state) +{ + (void)state; + char name[64]; + + memset(name, 'a', 15); name[15] = '\0'; + assert_true(ptp_clock_valid_device_ref(name)); + + memset(name, 'a', 16); name[16] = '\0'; + assert_false(ptp_clock_valid_device_ref(name)); +} + +/* Opening a PHC that does not exist must fail cleanly, and the NULL handle + * must then be safe to pass to every other entry point — MTL calls the time + * function from the dataplane with whatever priv it was given. */ +static void test_ptp_clock_open_nonexistent_device_fails(void **state) +{ + (void)state; + struct ptp_clock *clk = ptp_clock_open("/dev/ptp99", 1000); + assert_null(clk); + ptp_clock_close(clk); +} + +static void test_ptp_clock_null_handle_is_safe(void **state) +{ + (void)state; + assert_string_equal(ptp_clock_device(NULL), ""); + assert_int_equal((int)ptp_clock_get_time_ns(NULL), 0); + assert_int_equal(ptp_clock_stats(NULL, NULL, NULL, NULL), -1); + ptp_clock_close(NULL); +} + /* ========================================================================== * Optional "hwaccel" flag (hardware decode) * @@ -2232,6 +2352,14 @@ int main(void) cmocka_unit_test(test_validate_grandmaster_rejects_bad_phc_ref), cmocka_unit_test(test_validate_grandmaster_rejects_out_of_range_interval), cmocka_unit_test(test_validate_grandmaster_accepts_valid_phc), + cmocka_unit_test(test_parse_ptp_mode_slave_explicit), + cmocka_unit_test(test_parse_ptp_negative_interval_clamped_to_zero), + cmocka_unit_test(test_validate_grandmaster_accepts_empty_phc_for_autoselect), + cmocka_unit_test(test_validate_grandmaster_interval_boundaries), + cmocka_unit_test(test_validate_slave_mode_ignores_phc_fields), + cmocka_unit_test(test_ptp_clock_device_ref_length_boundary), + cmocka_unit_test(test_ptp_clock_open_nonexistent_device_fails), + cmocka_unit_test(test_ptp_clock_null_handle_is_safe), cmocka_unit_test(test_parse_hwaccel_absent_defaults_false), cmocka_unit_test(test_parse_hwaccel_true), cmocka_unit_test(test_parse_hwaccel_false), From d55ff2bed2797ed34cc84022f0f1a7e8a9e56f9c Mon Sep 17 00:00:00 2001 From: roshan-ku Date: Tue, 6 Oct 2026 14:31:19 +0530 Subject: [PATCH 6/7] test(ptp): cover the PHC servo and published time model ptp_clock.c sat at 20% line coverage: the config-level tests reached validation and the NULL paths, but the servo that the whole feature depends on was untested. sampler_update() already takes the two timestamps as plain integers -- the syscalls live in its callers -- so the servo can be driven with synthetic samples and no PHC hardware. The test includes ptp_clock.c directly to reach the static functions, as test_main.c does for main.c, and wraps clock_gettime so the served time is deterministic. Covers first-sample anchoring, convergence on a 40 ppm drift, the step-on-jump threshold, the stalled-clock guard against a divide by zero, rate clamping, continuity across a re-anchor, monotonicity under noisy samples, and the slot ring wrapping. Verified non-vacuous by mutation: re-anchoring on the raw measurement instead of the estimate, making clamp_rate a no-op, and raising the step threshold each fail the suite. The first revision of the clamp test did not catch its mutant -- a 1 s discrepancy takes the step path and never reaches the rate calculation -- so it now uses 0.9 ms over 100 ms, which stays under the step threshold and implies +9000 ppm. --- tests/meson.build | 15 ++ tests/test_ptp_clock.c | 366 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 381 insertions(+) create mode 100644 tests/test_ptp_clock.c diff --git a/tests/meson.build b/tests/meson.build index 0cc6f84..6f7749f 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -54,6 +54,21 @@ test_logger = executable('test_logger', ) test('logger', test_logger) +# ptp_clock.c is included directly by the test to reach its static servo +# functions; clock_gettime is wrapped to make the served time deterministic. +test_ptp_clock = executable('test_ptp_clock', + sources: [ + 'test_ptp_clock.c', + '../src/util/logger.c', + ], + include_directories: test_inc, + dependencies: [m_dep, pthread_dep, cmocka_dep], + c_args: test_c_args, + link_args: ['-Wl,--wrap=clock_gettime'], + install: false +) +test('ptp_clock', test_ptp_clock) + test_ffmpeg_decoder = executable('test_ffmpeg_decoder', sources: [ 'test_ffmpeg_decoder.c', diff --git a/tests/test_ptp_clock.c b/tests/test_ptp_clock.c new file mode 100644 index 0000000..c8563cc --- /dev/null +++ b/tests/test_ptp_clock.c @@ -0,0 +1,366 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright 2026 Intel Corporation + * + * Unit tests for the src/util/ptp_clock.c servo and published time model. + * + * The module samples the NIC PHC on a background thread and serves MTL a + * frequency-corrected linear model over CLOCK_MONOTONIC_RAW. The sampling + * syscalls live in phc_sample()/sampler_thread(), while sampler_update() takes + * the two timestamps as plain integers — so the servo itself can be driven + * with synthetic samples and no PHC hardware. + * + * Covers: + * clamp_rate() — +/-1000 ppm ceiling + * sampler_update() — first-sample anchoring, EMA rate tracking, + * step-on-jump, stalled-clock guard, rate clamp + * model_publish() — generation counter and slot ring wrap + * ptp_clock_get_time_ns() — served time, continuity across re-anchor + * ptp_clock_stats() — offset / freq / sample counters + * + * src/util/ptp_clock.c is included directly to reach its static functions and + * the opaque struct ptp_clock, as tests/test_main.c does for src/main.c. + * clock_gettime is wrapped so the served time is deterministic. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include + +/* ── Deterministic CLOCK_MONOTONIC_RAW ──────────────────────────────────── */ + +static bool g_fake_mono_enabled = false; +static uint64_t g_fake_mono_ns = 0; + +int __real_clock_gettime(clockid_t clk_id, struct timespec *tp); + +int __wrap_clock_gettime(clockid_t clk_id, struct timespec *tp) +{ + if (g_fake_mono_enabled && clk_id == CLOCK_MONOTONIC_RAW && tp != NULL) { + tp->tv_sec = (time_t)(g_fake_mono_ns / 1000000000ull); + tp->tv_nsec = (long)(g_fake_mono_ns % 1000000000ull); + return 0; + } + return __real_clock_gettime(clk_id, tp); +} + +static void fake_mono_set(uint64_t ns) +{ + g_fake_mono_enabled = true; + g_fake_mono_ns = ns; +} + +#include "../src/util/ptp_clock.c" + +/* ── Helpers ────────────────────────────────────────────────────────────── */ + +#define NS_PER_S 1000000000ull + +static void clk_reset(struct ptp_clock *clk) +{ + memset(clk, 0, sizeof(*clk)); + clk->fd = -1; + clk->base_rate = 1.0; + g_fake_mono_enabled = false; + g_fake_mono_ns = 0; +} + +static const struct ptp_model *current_model(const struct ptp_clock *clk) +{ + uint64_t gen = atomic_load_explicit(&clk->gen, memory_order_acquire); + return &clk->slots[gen % PTP_MODEL_SLOTS]; +} + +/* Served time at an arbitrary monotonic instant. */ +static uint64_t served_at(struct ptp_clock *clk, uint64_t mono_ns) +{ + fake_mono_set(mono_ns); + return ptp_clock_get_time_ns(clk); +} + +/* ── clamp_rate ─────────────────────────────────────────────────────────── */ + +static void test_clamp_rate_limits_to_one_thousand_ppm(void **state) +{ + (void)state; + assert_true(clamp_rate(1.0) == 1.0); + assert_true(clamp_rate(2.0) == 1.0 + PTP_MAX_RATE_DEV); + assert_true(clamp_rate(0.0) == 1.0 - PTP_MAX_RATE_DEV); + + /* A value inside the band is passed through untouched. */ + assert_true(clamp_rate(1.0 + PTP_MAX_RATE_DEV / 2) == 1.0 + PTP_MAX_RATE_DEV / 2); +} + +/* ── First sample ───────────────────────────────────────────────────────── */ + +/* With no prior model the first sample anchors exactly on the PHC: the served + * time must equal the sampled PHC value at that instant, with no slewing. */ +static void test_first_sample_anchors_on_phc(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + sampler_update(&clk, 1 * NS_PER_S, 5 * NS_PER_S); + + assert_true(clk.have_model); + assert_int_equal((int)atomic_load(&clk.stat_samples), 1); + assert_int_equal((int)atomic_load(&clk.stat_offset_ns), 0); + assert_int_equal((int)served_at(&clk, 1 * NS_PER_S), (int)(5 * NS_PER_S)); + + /* One second later the model advances at rate 1.0. */ + assert_int_equal((int)(served_at(&clk, 2 * NS_PER_S) - 5 * NS_PER_S), + (int)NS_PER_S); +} + +/* Before the anchor instant the model clamps rather than extrapolating + * backwards, so the served clock never runs below the anchor. */ +static void test_served_time_before_anchor_clamps(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + sampler_update(&clk, 10 * NS_PER_S, 100 * NS_PER_S); + + assert_int_equal((int)(served_at(&clk, 9 * NS_PER_S) - 100 * NS_PER_S), 0); + assert_int_equal((int)(served_at(&clk, 10 * NS_PER_S) - 100 * NS_PER_S), 0); +} + +/* ── Rate tracking ──────────────────────────────────────────────────────── */ + +/* A PHC running 40 ppm fast must be tracked, so the served model converges on + * the real rate instead of accumulating error. This is the drift the feature + * exists to cancel. */ +static void test_servo_converges_on_constant_drift(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + const double drift = 1.00004; /* +40 ppm */ + uint64_t mono = 1 * NS_PER_S; + uint64_t phc = 1 * NS_PER_S; + + sampler_update(&clk, mono, phc); + for (int i = 0; i < 40; i++) { + mono += NS_PER_S; + phc += (uint64_t)(NS_PER_S * drift); + sampler_update(&clk, mono, phc); + } + + int64_t freq_ppb = 0, offset_ns = 0; + uint64_t samples = 0; + assert_int_equal(ptp_clock_stats(&clk, &offset_ns, &freq_ppb, &samples), 0); + assert_int_equal((int)samples, 41); + + /* Converged to ~+40000 ppb, and the residual error per interval is small. */ + assert_in_range(freq_ppb, 38000, 42000); + assert_true(llabs(offset_ns) < 100000); /* < 100 us */ +} + +/* The model rate is bounded even when the implied correction is large, so a + * noisy PHC cannot make the served clock run away. The error must stay under + * the step threshold, otherwise the step path runs instead of the slew path: + * 0.9 ms of error over a 100 ms interval implies +9000 ppm, which must be + * clamped to +1000. */ +static void test_rate_is_clamped_against_wild_sample(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + const uint64_t t0 = 1 * NS_PER_S; + sampler_update(&clk, t0, t0); + sampler_update(&clk, t0 + 100000000ull, t0 + 100000000ull + 900000ull); + + const struct ptp_model *m = current_model(&clk); + assert_true(m->rate <= 1.0 + PTP_MAX_RATE_DEV); + assert_true(m->rate >= 1.0 - PTP_MAX_RATE_DEV); + + int64_t freq_ppb = 0; + ptp_clock_stats(&clk, NULL, &freq_ppb, NULL); + assert_in_range(llabs(freq_ppb), 0, 1000000); +} + +/* ── Step handling ──────────────────────────────────────────────────────── */ + +/* An error beyond PTP_STEP_NS means the PHC jumped (ptp4l stepped it, or the + * sampler stalled). Slewing a jump that large would take minutes, so the model + * re-anchors directly on the new PHC value. */ +static void test_large_error_steps_the_model(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + sampler_update(&clk, 1 * NS_PER_S, 1 * NS_PER_S); + sampler_update(&clk, 2 * NS_PER_S, 2 * NS_PER_S); + + /* +10 ms jump, well over the 1 ms step threshold. */ + const uint64_t jumped = 3 * NS_PER_S + 10000000ull; + sampler_update(&clk, 3 * NS_PER_S, jumped); + + const struct ptp_model *m = current_model(&clk); + assert_int_equal((int)(m->anchor_phc_ns - 3 * NS_PER_S), 10000000); + assert_true(m->rate == clk.base_rate); + assert_int_equal((int)(served_at(&clk, 3 * NS_PER_S) - 3 * NS_PER_S), 10000000); +} + +/* Two samples at the same monotonic instant would divide by zero in the rate + * calculation; the stalled-clock guard must take the step path instead. */ +static void test_zero_elapsed_does_not_divide_by_zero(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + sampler_update(&clk, 5 * NS_PER_S, 5 * NS_PER_S); + sampler_update(&clk, 5 * NS_PER_S, 5 * NS_PER_S); + + const struct ptp_model *m = current_model(&clk); + assert_true(isfinite(m->rate)); + assert_true(m->rate == clk.base_rate); + assert_int_equal((int)atomic_load(&clk.stat_samples), 2); +} + +/* ── Continuity ─────────────────────────────────────────────────────────── */ + +/* A small error is absorbed over the following interval rather than jumped: + * the model re-anchors on its own estimate, so the served clock stays + * continuous. A backwards step here would corrupt RTP timestamps. */ +static void test_small_error_is_absorbed_continuously(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + sampler_update(&clk, 1 * NS_PER_S, 1 * NS_PER_S); + sampler_update(&clk, 2 * NS_PER_S, 2 * NS_PER_S); + + uint64_t before = served_at(&clk, 3 * NS_PER_S); + + /* 100 us of error — below the 1 ms step threshold. */ + sampler_update(&clk, 3 * NS_PER_S, 3 * NS_PER_S + 100000ull); + + uint64_t after = served_at(&clk, 3 * NS_PER_S); + + /* Served time does not jump to the raw PHC value at the sample instant. */ + assert_int_equal((int)(after - before), 0); + assert_int_equal((int)atomic_load(&clk.stat_offset_ns), 100000); +} + +/* The served clock must never run backwards across a sequence of noisy + * samples — monotonicity is what the pacing path depends on. */ +static void test_served_time_never_goes_backwards(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + uint64_t mono = 1 * NS_PER_S; + uint64_t phc = 1 * NS_PER_S; + sampler_update(&clk, mono, phc); + + uint64_t prev = served_at(&clk, mono); + const int64_t noise[] = {0, 50000, -40000, 120000, -90000, 10000, -5000}; + + for (unsigned i = 0; i < sizeof(noise) / sizeof(noise[0]); i++) { + mono += NS_PER_S; + phc = (uint64_t)((int64_t)(phc + NS_PER_S) + noise[i]); + sampler_update(&clk, mono, phc); + + uint64_t now = served_at(&clk, mono); + assert_true(now >= prev); + prev = now; + } +} + +/* ── Model ring ─────────────────────────────────────────────────────────── */ + +/* Readers pick a slot by generation counter, so publishing more updates than + * there are slots must keep serving the newest model. */ +static void test_model_ring_wraps_and_serves_latest(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + + uint64_t mono = 1 * NS_PER_S; + uint64_t phc = 1 * NS_PER_S; + sampler_update(&clk, mono, phc); + + for (unsigned i = 0; i < PTP_MODEL_SLOTS * 3; i++) { + mono += NS_PER_S; + phc += NS_PER_S; + sampler_update(&clk, mono, phc); + } + + uint64_t gen = atomic_load(&clk.gen); + assert_int_equal((int)gen, (int)(PTP_MODEL_SLOTS * 3 + 1)); + + const struct ptp_model *m = current_model(&clk); + assert_int_equal((int)(m->anchor_mono_ns / NS_PER_S), (int)(mono / NS_PER_S)); + assert_int_equal((int)(served_at(&clk, mono) / NS_PER_S), (int)(phc / NS_PER_S)); +} + +/* ── Stats ──────────────────────────────────────────────────────────────── */ + +static void test_stats_accept_null_out_pointers(void **state) +{ + (void)state; + struct ptp_clock clk; + clk_reset(&clk); + sampler_update(&clk, 1 * NS_PER_S, 1 * NS_PER_S); + + assert_int_equal(ptp_clock_stats(&clk, NULL, NULL, NULL), 0); + + uint64_t samples = 0; + assert_int_equal(ptp_clock_stats(&clk, NULL, NULL, &samples), 0); + assert_int_equal((int)samples, 1); +} + +/* ── Device reference parsing ───────────────────────────────────────────── */ + +/* resolve_device() turns a config value into a /dev/ptpN path. The explicit + * path form must pass through untouched, without touching the filesystem. */ +static void test_resolve_device_passes_through_explicit_path(void **state) +{ + (void)state; + char out[64] = {0}; + assert_int_equal(resolve_device("/dev/ptp7", out, sizeof(out)), 0); + assert_string_equal(out, "/dev/ptp7"); +} + +static void test_resolve_device_rejects_garbage(void **state) +{ + (void)state; + char out[64] = {0}; + assert_int_not_equal(resolve_device("/etc/shadow", out, sizeof(out)), 0); + assert_int_not_equal(resolve_device("bad name!", out, sizeof(out)), 0); +} + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_clamp_rate_limits_to_one_thousand_ppm), + cmocka_unit_test(test_first_sample_anchors_on_phc), + cmocka_unit_test(test_served_time_before_anchor_clamps), + cmocka_unit_test(test_servo_converges_on_constant_drift), + cmocka_unit_test(test_rate_is_clamped_against_wild_sample), + cmocka_unit_test(test_large_error_steps_the_model), + cmocka_unit_test(test_zero_elapsed_does_not_divide_by_zero), + cmocka_unit_test(test_small_error_is_absorbed_continuously), + cmocka_unit_test(test_served_time_never_goes_backwards), + cmocka_unit_test(test_model_ring_wraps_and_serves_latest), + cmocka_unit_test(test_stats_accept_null_out_pointers), + cmocka_unit_test(test_resolve_device_passes_through_explicit_path), + cmocka_unit_test(test_resolve_device_rejects_garbage), + }; + return cmocka_run_group_tests(tests, NULL, NULL); +} From 675f3ef004e2ad81201e376f380bd77f79e742ad Mon Sep 17 00:00:00 2001 From: roshan-ku Date: Fri, 9 Oct 2026 14:09:39 +0530 Subject: [PATCH 7/7] fix(ptp): correct the SR-IOV session ceiling and drop the 4-VF config The README told users to spread sessions one-per-VF "as in config/tx_ptp_gm_4session.json". That config cannot work: MTL segfaults on any port after the first once PTP frames arrive, so following the advice crashed immediately. mt_ptp_init() allocates a PTP instance only for port 0 unless a port reports offload timestamping, which ixgbe VFs do not, leaving impl->ptp[i] NULL for ports 1+. cni_rx_handle() then calls mt_ptp_parse() for ethertype 0x88F7 with no NULL check, though the UDP path just below it is guarded. ptp4l runs on the PF and the NIC's internal switch replicates its L2 multicast to every VF, so the second VF dies on the first PTP frame. Measured on an E610: 3 sessions on 1 VF runs clean, while 4 sessions on 2 VFs and 8 on 4 VFs both SIGSEGV in mt_ptp_parse. It reproduces with ptp.enable false and stops only when no PTP traffic is present, so it is independent of this feature. Re-confirmed against a DPDK built with MTL's documented patches. Remove the config, document the real ceiling of 3 sessions on a single VF, and record the one-line mt_cni.c guard that lifts it. Add a startup warning for multi-interface grandmaster configs so the failure is explained rather than a bare segfault. --- README.md | 41 ++++++++++++++++++++++----- config/tx_ptp_gm_4session.json | 52 ---------------------------------- src/util/config_reader.c | 9 ++++++ tests/test_config_reader.c | 28 ++++++++++++++++++ 4 files changed, 71 insertions(+), 59 deletions(-) delete mode 100644 config/tx_ptp_gm_4session.json diff --git a/README.md b/README.md index ddc1b8d..2b38ae9 100644 --- a/README.md +++ b/README.md @@ -341,13 +341,40 @@ The second works because MTL never touches the TX port's own clock in this mode For the separate-port topology, skip step 1 and run `ptp4l` on the spare port instead; the TX NIC stays entirely under DPDK. -> **Sessions per VF are limited by the NIC's TX queues.** MTL requests `sessions + 2` TX queues but -> silently clamps to what the device provides, then fails later with the unhelpful -> `mt_dev_get_tx_queue(0), fail to find free tx queue`. An Intel E610 VF exposes 4 TX queues and MTL -> reserves one for system traffic, so **3 sessions per VF is the ceiling**. Spread further sessions -> across additional VFs — one session per VF, as in `config/tx_ptp_gm_4session.json`. Check the -> startup log: `MTL init: port[0]=... tx_queues=5` is the request, -> `dev_config_port(0), tx_q(4 ...)` is what the hardware actually gave. +> **In the SR-IOV topology, 3 sessions on a single VF is the practical ceiling.** Two separate +> limits combine to produce it, and only the first is obvious. +> +> *The VF's TX queues.* MTL requests `sessions + 2` TX queues but silently clamps to what the device +> provides, then fails later with the unhelpful `mt_dev_get_tx_queue(0), fail to find free tx queue`. +> An Intel E610 VF exposes 4 TX queues however many VFs you create, and MTL reserves one for system +> traffic — so **3 sessions per VF**. Check the startup log: `MTL init: port[0]=... tx_queues=5` is +> the request, `dev_config_port(0), tx_q(4 ...)` is what the hardware actually gave. +> +> *A NULL dereference in MTL, which stops you using a second VF.* `mt_ptp_init()` allocates a PTP +> instance only for port 0 unless a port reports offload timestamping, which ixgbe VFs do not — so +> `impl->ptp[i]` is NULL for ports 1 and up. `cni_rx_handle()` then calls `mt_ptp_parse(ptp, ...)` +> for ethertype `0x88F7` with no NULL check (the UDP path a few lines below *is* guarded). Because +> `ptp4l` runs on the PF, the NIC's internal switch replicates its L2 multicast to every VF, so the +> second VF takes a `SIGSEGV` in `mt_ptp_parse` as soon as the first PTP frame lands. +> +> This is independent of dvledtx: it reproduces with `ptp.enable: false`, and disappears only when +> no PTP traffic is on the wire. Until it is fixed upstream, keep `interfaces[]` to a **single VF** +> and at most 3 sessions. Creating extra VFs is harmless — only *using* a second one crashes. +> +> The fix is one line in MTL's `lib/src/mt_cni.c`, mirroring the guard its own UDP path already has: +> +> ```c +> case RTE_ETHER_TYPE_1588: +> if (!ptp) break; /* port has no PTP instance */ +> ptp_hdr = rte_pktmbuf_mtod_offset(m, struct mt_ptp_header*, hdr_offset); +> mt_ptp_parse(ptp, ptp_hdr, vlan, MT_PTP_L2, m->timesync, NULL); +> break; +> ``` +> +> With that guard in place the ceiling becomes 3 sessions per VF, so 4 VFs give 12. +> +> Note this limit applies **only** when PTP forces the SR-IOV topology. Without PTP, dvledtx owns the +> PF, which has ample queues — 8 sessions on one PF port is known to work. **Verifying it works** diff --git a/config/tx_ptp_gm_4session.json b/config/tx_ptp_gm_4session.json deleted file mode 100644 index 8fae016..0000000 --- a/config/tx_ptp_gm_4session.json +++ /dev/null @@ -1,52 +0,0 @@ -{ - "log_file": "dvledtx_ptp_4session.log", - "interfaces": [ - { "nic_index": 0, "name": "0000:04:10.0", "sip": "192.168.50.29", "dip": "239.168.85.20" }, - { "nic_index": 1, "name": "0000:04:10.2", "sip": "192.168.50.30", "dip": "239.168.85.20" }, - { "nic_index": 2, "name": "0000:04:10.4", "sip": "192.168.50.31", "dip": "239.168.85.20" }, - { "nic_index": 3, "name": "0000:04:10.6", "sip": "192.168.50.32", "dip": "239.168.85.20" } - ], - "video": { - "width": 1920, - "height": 1080, - "tx_url": "bbb_sunflower_1080p_30fps_normal.mp4" - }, - "tx_video": { - "scale_width": 1920, - "scale_height": 1080, - "fps": 30, - "fmt": "yuv422p10le" - }, - "ptp": { - "enable": true, - "mode": "grandmaster", - "phc_interface": "enp4s0", - "phc_interval_ms": 1000 - }, - "tx_sessions": [ - { - "nic_index": 0, - "udp_port": 20000, - "payload_type": 96, - "crop": { "x": 0, "y": 0, "w": 960, "h": 540 } - }, - { - "nic_index": 1, - "udp_port": 20002, - "payload_type": 96, - "crop": { "x": 960, "y": 0, "w": 960, "h": 540 } - }, - { - "nic_index": 2, - "udp_port": 20004, - "payload_type": 96, - "crop": { "x": 0, "y": 540, "w": 960, "h": 540 } - }, - { - "nic_index": 3, - "udp_port": 20006, - "payload_type": 96, - "crop": { "x": 960, "y": 540, "w": 960, "h": 540 } - } - ] -} diff --git a/src/util/config_reader.c b/src/util/config_reader.c index 6db253f..797386c 100644 --- a/src/util/config_reader.c +++ b/src/util/config_reader.c @@ -684,6 +684,15 @@ int validate_tx_config(const struct dvledtx_config* config) { config->ptp_phc_interval_ms); return -1; } + /* MTL gives ports >0 no PTP instance and parses 0x88F7 without a NULL check, + * so a second port segfaults once ptp4l's multicast reaches it. */ + if (config->ptp_enable && config->nic_count > 1) { + LOG_WARN("ptp.mode 'grandmaster' with %d interfaces: MTL crashes on the " + "second and later ports when PTP frames arrive. Use a single VF " + "(max 3 sessions) until MTL's mt_cni.c guards mt_ptp_parse against " + "a NULL ptp instance. See the SR-IOV notes in README.md.", + config->nic_count); + } } /* Video resolution validation */ diff --git a/tests/test_config_reader.c b/tests/test_config_reader.c index f1a38a4..b668cc1 100644 --- a/tests/test_config_reader.c +++ b/tests/test_config_reader.c @@ -761,6 +761,33 @@ static void test_ptp_clock_device_ref_length_boundary(void **state) assert_false(ptp_clock_valid_device_ref(name)); } +/* Multiple interfaces in grandmaster mode warns but still validates, so the + * config keeps working once MTL guards mt_ptp_parse. */ +static void test_validate_grandmaster_multi_nic_warns_but_passes(void **state) +{ + (void)state; + struct dvledtx_config cfg; + fill_valid_config(&cfg); + + cfg.interface_name = realloc(cfg.interface_name, 2 * sizeof(*cfg.interface_name)); + cfg.interface_sip = realloc(cfg.interface_sip, 2 * sizeof(*cfg.interface_sip)); + cfg.interface_dip = realloc(cfg.interface_dip, 2 * sizeof(*cfg.interface_dip)); + assert_non_null(cfg.interface_name); + assert_non_null(cfg.interface_sip); + assert_non_null(cfg.interface_dip); + snprintf(cfg.interface_name[1], sizeof(cfg.interface_name[1]), "0000:06:00.1"); + snprintf(cfg.interface_sip[1], sizeof(cfg.interface_sip[1]), "192.168.50.30"); + snprintf(cfg.interface_dip[1], sizeof(cfg.interface_dip[1]), "239.168.85.20"); + cfg.nic_cap = 2; + cfg.nic_count = 2; + + cfg.ptp_enable = true; + cfg.ptp_mode = DVLEDTX_PTP_MODE_GRANDMASTER; + snprintf(cfg.ptp_phc, sizeof(cfg.ptp_phc), "enp4s0"); + assert_int_equal(validate_tx_config(&cfg), 0); + dvledtx_config_free(&cfg); +} + /* Opening a PHC that does not exist must fail cleanly, and the NULL handle * must then be safe to pass to every other entry point — MTL calls the time * function from the dataplane with whatever priv it was given. */ @@ -2358,6 +2385,7 @@ int main(void) cmocka_unit_test(test_validate_grandmaster_interval_boundaries), cmocka_unit_test(test_validate_slave_mode_ignores_phc_fields), cmocka_unit_test(test_ptp_clock_device_ref_length_boundary), + cmocka_unit_test(test_validate_grandmaster_multi_nic_warns_but_passes), cmocka_unit_test(test_ptp_clock_open_nonexistent_device_fails), cmocka_unit_test(test_ptp_clock_null_handle_is_safe), cmocka_unit_test(test_parse_hwaccel_absent_defaults_false),