Re: [RFC v2 0/6] introduce PTP protocol library and software relay

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 29 Apr 2026 03:58:31 +0530
Rajesh Kumar <rajesh3.kumar@intel.com> wrote:

> This series introduces a new DPDK library (lib/ptp) for IEEE 1588-2019
> PTP protocol packet processing and a companion example application
> (ptp_tap_relay_sw) that demonstrates its usage.
> 
> Motivation
> ----------
> Several DPDK applications need to classify and manipulate PTP packets
> (e.g. ptpclient, ptp_tap_relay, custom Transparent Clocks). Today each
> application re-implements its own PTP header parsing and correctionField
> handling. A shared library avoids duplication and provides a tested,
> standards-compliant foundation.


It would be great to have this, though not sure who would use it now.

AI review spotted some things.

Reviewed v2 of the PTP library series. Findings below.
Patch 1/6 (lib/ptp):

RTE_PTP_ETHERTYPE 0x88F7 duplicates RTE_ETHER_TYPE_1588 already in lib/net/rte_ether.h. Use the existing constant or alias to it.
Comment on the version byte says "reserved (4) | versionPTP (4)"; IEEE 1588-2019 redefined the upper nibble as minorVersionPTP.

Patch 2/6 (prog guide):

The guide declares rte_ptp_hdr_get(const struct rte_mbuf *m) but the header has it non-const. Fix the doc, or split into const and non-const variants since the function returns a writable interior pointer.

Patch 3/6 (ptp_tap_relay_sw):

The standalone Makefile is missing CFLAGS += -DALLOW_EXPERIMENTAL_API. All three rte_ptp_* APIs the relay uses are __rte_experimental, so make-based builds will warn or fail under -Werror. meson is fine because meson.build sets allow_experimental_apis = true. Match examples/ptpclient/Makefile.
relay_burst() calls rte_ptp_classify() and then rte_ptp_hdr_get() on every event packet, parsing it twice. The prog guide added in 2/6 explicitly tells callers to use rte_ptp_hdr_get() once and then rte_ptp_msg_type(hdr). The reference example should follow its own guidance.
Minor: stats.corrections is updated via the global while the other counters are passed in by pointer. Inconsistent.

Patch 4/6 (sample app guide):

Limitations section claims L2-only and PTPv2-only, but the relay uses lib/ptp classification which handles VLAN/QinQ/UDPv4/UDPv6 with no version filter. Reword to "tested with L2 PTP" or drop the restriction.

Patch 5/6 (test_ptp.c):

Several correction tests dereference the rte_ptp_hdr_get() return without TEST_ASSERT_NOT_NULL: test_ptp_correction_zero, test_ptp_correction_known, and the four test_ptp_add_correction_* tests. test_ptp_two_step and test_ptp_seq_id get this right.
No coverage for IPv4 with options (IHL > 5). The library's offset += ihl path is exercised by no test.

Patch 6/6: no issues.