-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: 29 April 2026 09:07 PM
To: Kumar, Rajesh3 <[email protected]>
Cc: [email protected]; Richardson, Bruce <[email protected]>; Singh, Aman
Deep <[email protected]>
Subject: Re: [RFC v2 0/6] introduce PTP protocol library and software relay
On Wed, 29 Apr 2026 03:58:31 +0530
Rajesh Kumar <[email protected]> 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.
>>>>Fixed.
