daniel-p-carvalho opened a new pull request, #3779:
URL: https://github.com/apache/nuttx-apps/pull/3779
## Summary
* Why change is necessary (fix, update, new feature)?
`apps/netutils/ptpd` had never been exercised against a real,
GPS-disciplined PTP Grandmaster over a Layer 2 (`AF_PACKET`)
transport with `-B` (BMCA) and `-E` (E2E delay request-response)
enabled together. Doing so on real hardware (STM32F4Discovery)
against a real Grandmaster (Toradex Colibri iMX7 running
`linuxptp`'s `ptp4l`, IEC/IEEE 61850-9-3 profile) uncovered four
independent, real bugs: the client's local-clock drift estimate
diverged instead of converging, `Delay_Req` never left the wire at
all, the `Delay_Req` that did leave the wire (after the first fix)
used the wrong `logMessageInterval`/`PTP version` field values and
was silently discarded by the Grandmaster, and a diagnostic-only
error path swallowed the real cause of an early daemon failure.
* What functional part of the code is being changed?
`apps/netutils/ptpd/ptpd.c`, `apps/netutils/ptpd/ptpv2.h`,
`apps/system/ptpd/ptpd_main.c` — the E2E PTP client path
(`-B -E`, `AF_PACKET`/`-2` transport).
* How does the change exactly work (what will change and how)?
Four commits, each independently verified (see Testing below and
each commit's own message for the detailed root cause):
1. `fix BMCA compilation typos and define ETHERTYPE_PTP` — fixes
two typos (`state->conifg`, `state->n_identity`) in the BMCA
announce-processing path and defines the `ETHERTYPE_PTP` macro
used by `ptpd.c`/`ptpv2.h` but never defined anywhere in NuttX.
2. `fix drift divergence, post-jump bootstrap and error
propagation` — fixes the local-clock drift estimator in
`ptp_update_local_clock()`: a bogus first-sample drift
computation right after the initial clock jump, a feedback bug
where `ptp_adjtime()` silently ignores the `ppb` argument for
`CLOCK_REALTIME` (so the previous cycle's assumed correction was
never actually fully applied, corrupting the next drift
estimate), an outlier-avoidance heuristic that could repeatedly
re-apply a stale correction, and a `Delay_Req` bootstrap
deadlock (`can_send_delayreq` never became true because it
required the *uncompensated* offset to already be under 100 µs
— unreachable under software timestamping). Also propagates the
real `ptp_initialize_state()` failure code out of `ptpd_start()`
instead of a hardcoded `0`, which made `do_ptpd_start()` print a
misleading `"ERROR: ptpd_start() failed:0"` even on a clean stop.
3. `fix Delay_Req transmission over AF_PACKET` — on `AF_PACKET`
`SOCK_RAW` sockets, `msg_name` must be `NULL` (destination is in
the Ethernet header already); `ptp_send_delay_req()` was passing
a `struct sockaddr_in` through unconditionally, so `sendmsg()`
failed immediately with `-EAFNOSUPPORT` and `Delay_Req` never
reached the wire. Also corrects the multicast destination MAC
(was using the IEEE 1588 Annex F *peer-delay* address
`01:80:C2:00:00:0E` for a general/non-peer message; the correct
address is `01:1B:19:00:00:00`), byte-swaps `ether_type`, and
initializes `delayreq_interval` to a sane 1 second default.
4. `add diagnostic logging for received PTP frames and Delay_Resp
validation` — adds `ptpinfo()`/`ptpwarn()` calls (gated by the
existing `CONFIG_DEBUG_PTP_INFO`/`_WARN` symbols, zero cost when
disabled) at points that previously failed silently: an
unrecognized L2 protocol, a domain mismatch, and a `Delay_Resp`
rejected by the source/requester identity check. These were
essential to diagnosing the above on real hardware and are kept
for future maintainers.
* Related NuttX Apps Issue / Pull Request reference if applicable.
None yet.
* Dependencies (if change depends on another change).
None — self-contained within `apps/`.
## Impact
* Is new feature added? Is existing feature changed? NO / YES (please
describe if yes).
NO new feature. Existing E2E client behavior (`-B -E`) is fixed —
it was non-functional (diverging clock, no delay measurement) in
this exact combination before this change.
* Impact on user (will user need to adapt to change)? NO.
* Impact on build (will build process change)? NO.
* Impact on hardware (will arch(s) / board(s) / driver(s) change)? NO.
Pure `apps/` change, no driver or arch code touched. Validated on
STM32F4Discovery (see Testing) but not board/arch-specific.
* Impact on documentation (is update required / provided)? NO.
Bug fixes restoring documented/intended behavior; no user-facing
Kconfig or API semantics changed.
* Impact on security (any sort of implications)? NO.
* Impact on compatibility (backward/forward/interoperability)? NO.
No Kconfig symbol renamed or removed. Behavior only becomes
correct where it was previously broken (E2E mode with `-B -E`
together); other modes/flags are unaffected.
* Anything else to consider or add?
Per Contributing Guideline 1.7.9: these four commits are bundled
in one PR because none of them alone restores working E2E delay
measurement — the drift fix, the AF_PACKET transmit fix, and the
message-field fix are each necessary but not sufficient on their
own, verified individually (see Testing). This is not a Breaking
Change.
## Testing
I confirm that changes are verified on local setup and works as intended:
* Build Host(s): Ubuntu 26.04.1 LTS, x86_64, `arm-none-eabi-gcc`
(Arm GNU Toolchain 15.2.Rel1) 15.2.1.
* Target(s): `arm` (STM32F407VG), board:config
`stm32f4discovery-ext:ethraw` (out-of-tree custom board, see
`uFieldStack-NuttX` repo), real hardware — not QEMU/simulator.
* Grandmaster: real GPS/PPS-disciplined PTP Grandmaster (Toradex
Colibri iMX7 + X-NUCLEO-GNSS1A1, `linuxptp` `ptp4l`, IEC/IEEE
61850-9-3 Power Utility Profile, `network_transport L2`,
`delay_mechanism E2E` for this test), reachable over a dedicated
L2 Ethernet segment (not through a consumer router/AP — a
household Wi-Fi router in the path was found to intermittently
break the `Delay_Req`/`Delay_Resp` round trip; this is an
infrastructure constraint of the test setup, not a NuttX/`ptp4l`
bug, and is unrelated to the code changes in this PR).
* Command: `ptpd -2 -s -S -r -B -E -i eth0 &` then periodic
`ptpd -t <pid>` status queries.
Testing logs before change (local-clock drift diverging, path delay
never measured):
```
# t ~= 345s uptime
last_delta_ns: 3200490
last_adjtime_ns: 1064646
drift_ppb: 2225900
path_delay_ns: 0
# t ~= 474s uptime (~129s later) - offset nearly doubled, not converging
last_delta_ns: 5930659
last_adjtime_ns: 3821792
drift_ppb: 2253711
path_delay_ns: 0
```
Testing logs after change (drift converges, path delay measured and
stable across a ~150s window):
```
# t1
clock_source_valid: 1
last_delta_ns: -4045660
last_adjtime_ns: -4128664
drift_ppb: -85572
path_delay_ns: 1363121
# t2 (~150s later)
last_delta_ns: 1957282
last_adjtime_ns: 1880534
drift_ppb: -79122
path_delay_ns: 1376805
```
`path_delay_ns` stable within ~1% across the interval; `drift_ppb`
stayed in a bounded band and did not diverge (contrast with "before",
where it grew unbounded). `clock_source_valid` and the decoded
Grandmaster identity/clock-quality fields were also cross-checked
against the Grandmaster's own real configuration and matched exactly
in every run.
## PR verification Self-Check
* [x] This PR introduces only one functional change (E2E delay
measurement over AF_PACKET works correctly end-to-end; see
Impact note on the 1.7.9 bundling justification).
* [x] I have updated all required description fields above.
* [ ] My PR is still work in progress (not ready for review).
* [x] My PR is ready for review and can be safely merged into a codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
