* [PATCH v10 net-next 1/3] ptp_mock: Expose ptp_clock_info to external drivers
2026-08-15 6:31 [PATCH v10 net-next 0/3] Implement PTP support in netdevsim Maciek Machnikowski
@ 2026-08-15 6:31 ` Maciek Machnikowski
2026-08-15 6:31 ` [PATCH v10 net-next 2/3] netdevsim: Implement basic ptp support Maciek Machnikowski
2026-08-15 6:31 ` [PATCH v10 net-next 3/3] selftests: drivers/net: Implement ptp4l sync test using netdevsim Maciek Machnikowski
2 siblings, 0 replies; 5+ messages in thread
From: Maciek Machnikowski @ 2026-08-15 6:31 UTC (permalink / raw)
To: netdev
Cc: kuba, maciek, richardcochran, milena.olech, willemdebruijn.kernel,
andrew, vadim.fedorenko, horms
Allow exposing the ptp_clock_info of the ptp_mock to the external drivers.
Convert spinlocks to SLIS to allow gettime to be called from the netdevsim.
This is a prerequisite for implementing ptp support on netdevsim.
Co-developed-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Maciek Machnikowski <maciek@machnikowski.net>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
drivers/ptp/ptp_mock.c | 26 ++++++++++++++++++--------
include/linux/ptp_mock.h | 5 +++++
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/ptp/ptp_mock.c b/drivers/ptp/ptp_mock.c
index 4d66b6147121..7a4e5f3274a6 100644
--- a/drivers/ptp/ptp_mock.c
+++ b/drivers/ptp/ptp_mock.c
@@ -49,15 +49,16 @@ static u64 mock_phc_cc_read(struct cyclecounter *cc)
static int mock_phc_adjfine(struct ptp_clock_info *info, long scaled_ppm)
{
struct mock_phc *phc = info_to_phc(info);
+ unsigned long flags;
s64 adj;
adj = (s64)scaled_ppm << MOCK_PHC_FADJ_SHIFT;
adj = div_s64(adj, MOCK_PHC_FADJ_DENOMINATOR);
- spin_lock(&phc->lock);
+ spin_lock_irqsave(&phc->lock, flags);
timecounter_read(&phc->tc);
phc->cc.mult = MOCK_PHC_CC_MULT + adj;
- spin_unlock(&phc->lock);
+ spin_unlock_irqrestore(&phc->lock, flags);
return 0;
}
@@ -65,10 +66,11 @@ static int mock_phc_adjfine(struct ptp_clock_info *info, long scaled_ppm)
static int mock_phc_adjtime(struct ptp_clock_info *info, s64 delta)
{
struct mock_phc *phc = info_to_phc(info);
+ unsigned long flags;
- spin_lock(&phc->lock);
+ spin_lock_irqsave(&phc->lock, flags);
timecounter_adjtime(&phc->tc, delta);
- spin_unlock(&phc->lock);
+ spin_unlock_irqrestore(&phc->lock, flags);
return 0;
}
@@ -78,10 +80,11 @@ static int mock_phc_settime64(struct ptp_clock_info *info,
{
struct mock_phc *phc = info_to_phc(info);
u64 ns = timespec64_to_ns(ts);
+ unsigned long flags;
- spin_lock(&phc->lock);
+ spin_lock_irqsave(&phc->lock, flags);
timecounter_init(&phc->tc, &phc->cc, ns);
- spin_unlock(&phc->lock);
+ spin_unlock_irqrestore(&phc->lock, flags);
return 0;
}
@@ -89,11 +92,12 @@ static int mock_phc_settime64(struct ptp_clock_info *info,
static int mock_phc_gettime64(struct ptp_clock_info *info, struct timespec64 *ts)
{
struct mock_phc *phc = info_to_phc(info);
+ unsigned long flags;
u64 ns;
- spin_lock(&phc->lock);
+ spin_lock_irqsave(&phc->lock, flags);
ns = timecounter_read(&phc->tc);
- spin_unlock(&phc->lock);
+ spin_unlock_irqrestore(&phc->lock, flags);
*ts = ns_to_timespec64(ns);
@@ -171,5 +175,11 @@ void mock_phc_destroy(struct mock_phc *phc)
}
EXPORT_SYMBOL_GPL(mock_phc_destroy);
+struct ptp_clock_info *mock_phc_get_ptp_info(struct mock_phc *phc)
+{
+ return &phc->info;
+}
+EXPORT_SYMBOL_GPL(mock_phc_get_ptp_info);
+
MODULE_DESCRIPTION("Mock-up PTP Hardware Clock driver");
MODULE_LICENSE("GPL");
diff --git a/include/linux/ptp_mock.h b/include/linux/ptp_mock.h
index 72eb401034d9..e33188dec2b7 100644
--- a/include/linux/ptp_mock.h
+++ b/include/linux/ptp_mock.h
@@ -16,6 +16,7 @@ struct mock_phc;
struct mock_phc *mock_phc_create(struct device *dev);
void mock_phc_destroy(struct mock_phc *phc);
int mock_phc_index(struct mock_phc *phc);
+struct ptp_clock_info *mock_phc_get_ptp_info(struct mock_phc *phc);
#else
@@ -33,6 +34,10 @@ static inline int mock_phc_index(struct mock_phc *phc)
return -1;
}
+static inline struct ptp_clock_info *mock_phc_get_ptp_info(struct mock_phc *phc)
+{
+ return NULL;
+}
#endif
#endif /* _PTP_MOCK_H_ */
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v10 net-next 2/3] netdevsim: Implement basic ptp support
2026-08-15 6:31 [PATCH v10 net-next 0/3] Implement PTP support in netdevsim Maciek Machnikowski
2026-08-15 6:31 ` [PATCH v10 net-next 1/3] ptp_mock: Expose ptp_clock_info to external drivers Maciek Machnikowski
@ 2026-08-15 6:31 ` Maciek Machnikowski
2026-08-18 13:58 ` Simon Horman
2026-08-15 6:31 ` [PATCH v10 net-next 3/3] selftests: drivers/net: Implement ptp4l sync test using netdevsim Maciek Machnikowski
2 siblings, 1 reply; 5+ messages in thread
From: Maciek Machnikowski @ 2026-08-15 6:31 UTC (permalink / raw)
To: netdev
Cc: kuba, maciek, richardcochran, milena.olech, willemdebruijn.kernel,
andrew, vadim.fedorenko, horms
Add support for virtual timestamping inside the netdevsim driver.
The implementation uses two attached ptp_mock clocks, reads the timestamps
of the ones attached either to the netdevsim or its peer and returns
timestamps using standard timestamps APIs.
This implementation enables running ptp4l on netdevsim adapters and
introduces a new ptp selftest.
Co-developed-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Maciek Machnikowski <maciek@machnikowski.net>
---
drivers/net/netdevsim/ethtool.c | 15 +++++
drivers/net/netdevsim/netdev.c | 104 ++++++++++++++++++++++++++++++
drivers/net/netdevsim/netdevsim.h | 1 +
3 files changed, 120 insertions(+)
diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 025ea79879f3..24e7d5592e79 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -200,7 +200,22 @@ static int nsim_get_ts_info(struct net_device *dev,
{
struct netdevsim *ns = netdev_priv(dev);
+ ethtool_op_get_ts_info(dev, info);
+ if (!ns->phc) {
+ info->phc_index = -1;
+ return 0;
+ }
+
info->phc_index = mock_phc_index(ns->phc);
+ if (info->phc_index < 0)
+ return 0;
+
+ info->so_timestamping |= SOF_TIMESTAMPING_TX_HARDWARE |
+ SOF_TIMESTAMPING_RX_HARDWARE |
+ SOF_TIMESTAMPING_RAW_HARDWARE;
+
+ info->tx_types = BIT(HWTSTAMP_TX_OFF) | BIT(HWTSTAMP_TX_ON);
+ info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) | BIT(HWTSTAMP_FILTER_ALL);
return 0;
}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 4e9d7e10b527..e586fd8f3bc3 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -30,6 +30,8 @@
#include <net/rtnetlink.h>
#include <net/udp_tunnel.h>
#include <net/busy_poll.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/timecounter.h>
#include "netdevsim.h"
@@ -122,7 +124,12 @@ static int nsim_forward_skb(struct net_device *tx_dev,
static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
+ struct skb_shared_hwtstamps shhwtstamps = {};
+ struct ptp_clock_info *ptp_info_tx = NULL;
+ struct ptp_clock_info *ptp_info_rx = NULL;
struct netdevsim *ns = netdev_priv(dev);
+ struct timespec64 tx_ts, rx_ts;
+ struct sk_buff *skb_orig = skb;
struct skb_ext *psp_ext = NULL;
struct net_device *peer_dev;
unsigned int len = skb->len;
@@ -164,6 +171,44 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
skb_linearize(skb);
skb_tx_timestamp(skb);
+
+ if (unlikely(READ_ONCE(peer_ns->tstamp_config.rx_filter) !=
+ HWTSTAMP_FILTER_NONE))
+ ptp_info_rx = mock_phc_get_ptp_info(peer_ns->phc);
+
+ if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+ READ_ONCE(ns->tstamp_config.tx_type) == HWTSTAMP_TX_ON)
+ ptp_info_tx = mock_phc_get_ptp_info(ns->phc);
+
+ if (unlikely(ptp_info_tx || ptp_info_rx)) {
+ /* Capture both timestamps closely */
+ if (ptp_info_tx)
+ ptp_info_tx->gettime64(ptp_info_tx, &tx_ts);
+ if (ptp_info_rx)
+ ptp_info_rx->gettime64(ptp_info_rx, &rx_ts);
+
+ /* If TX hardware timestamping is enabled report TX timestamp */
+ if (ptp_info_tx) {
+ shhwtstamps.hwtstamp = timespec64_to_ktime(tx_ts);
+ skb_tstamp_tx(skb_orig, &shhwtstamps);
+ }
+
+ /* Unshare the skb, stamping it must not corrupt cloned copies
+ * and prevents returning single tstamp for Tx and Rx
+ */
+ skb = skb_unshare(skb_orig, GFP_ATOMIC);
+ if (unlikely(!skb)) {
+ if (psp_ext)
+ __skb_ext_put(psp_ext);
+ goto out_drop_cnt;
+ }
+
+ /* If RX hardware timestamping is enabled report RX timestamp */
+ if (ptp_info_rx)
+ skb_hwtstamps(skb)->hwtstamp =
+ timespec64_to_ktime(rx_ts);
+ }
+
if (unlikely(nsim_forward_skb(dev, peer_dev,
skb, rq, psp_ext) == NET_RX_DROP))
goto out_drop_cnt;
@@ -185,6 +230,63 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
+static int nsim_set_ts_config(struct net_device *netdev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack)
+{
+ struct netdevsim *ns = netdev_priv(netdev);
+ int rx_filter;
+
+ if (!ns->phc)
+ return -EOPNOTSUPP;
+
+ switch (config->rx_filter) {
+ case HWTSTAMP_FILTER_NONE:
+ rx_filter = HWTSTAMP_FILTER_NONE;
+ break;
+ case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
+ case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
+ case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
+ case HWTSTAMP_FILTER_PTP_V2_EVENT:
+ case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+ case HWTSTAMP_FILTER_PTP_V2_SYNC:
+ case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
+ case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
+ case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
+ case HWTSTAMP_FILTER_NTP_ALL:
+ case HWTSTAMP_FILTER_ALL:
+ rx_filter = HWTSTAMP_FILTER_ALL;
+ break;
+ default:
+ return -ERANGE;
+ }
+
+ switch (config->tx_type) {
+ case HWTSTAMP_TX_OFF:
+ WRITE_ONCE(ns->tstamp_config.tx_type, HWTSTAMP_TX_OFF);
+ break;
+ case HWTSTAMP_TX_ON:
+ WRITE_ONCE(ns->tstamp_config.tx_type, HWTSTAMP_TX_ON);
+ break;
+ default:
+ return -ERANGE;
+ }
+
+ WRITE_ONCE(ns->tstamp_config.rx_filter, rx_filter);
+ config->rx_filter = rx_filter;
+
+ return 0;
+}
+
+static int nsim_get_ts_config(struct net_device *netdev,
+ struct kernel_hwtstamp_config *config)
+{
+ struct netdevsim *ns = netdev_priv(netdev);
+
+ *config = ns->tstamp_config;
+ return 0;
+}
+
static int nsim_set_rx_mode(struct net_device *dev,
struct netdev_hw_addr_list *uc,
struct netdev_hw_addr_list *mc)
@@ -647,6 +749,8 @@ static const struct net_device_ops nsim_netdev_ops = {
.ndo_vlan_rx_add_vid = nsim_vlan_rx_add_vid,
.ndo_vlan_rx_kill_vid = nsim_vlan_rx_kill_vid,
.net_shaper_ops = &nsim_shaper_ops,
+ .ndo_hwtstamp_get = nsim_get_ts_config,
+ .ndo_hwtstamp_set = nsim_set_ts_config,
};
static const struct net_device_ops nsim_vf_netdev_ops = {
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 64f77f93d937..5cdd1e294446 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -108,6 +108,7 @@ struct netdevsim {
struct net_device *netdev;
struct nsim_dev *nsim_dev;
struct nsim_dev_port *nsim_dev_port;
+ struct kernel_hwtstamp_config tstamp_config;
struct mock_phc *phc;
struct nsim_rq **rq;
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v10 net-next 2/3] netdevsim: Implement basic ptp support
2026-08-15 6:31 ` [PATCH v10 net-next 2/3] netdevsim: Implement basic ptp support Maciek Machnikowski
@ 2026-08-18 13:58 ` Simon Horman
0 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2026-08-18 13:58 UTC (permalink / raw)
To: maciek
Cc: Simon Horman, netdev, kuba, richardcochran, milena.olech,
willemdebruijn.kernel, andrew, vadim.fedorenko
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260815063148.2952-1-maciek@machnikowski.net?part=2
---
--- Patch [2]: [PATCH v10 net-next 2/3] netdevsim: Implement basic ptp support ---
commit ef2b7171af287d8c3a8f5064008830a7dd8f4db3
Author: Maciek Machnikowski <maciek@machnikowski.net>
netdevsim: Implement basic ptp support
Adds virtual hardware timestamping to netdevsim by reading the mock PHC
attached to the transmitting netdevsim and to its peer, reporting a TX
timestamp to the socket and stamping the forwarded skb for the RX side.
Also wires up ndo_hwtstamp_get / ndo_hwtstamp_set and extends
nsim_get_ts_info().
A few questions on the timestamping path below.
> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
> index 025ea79879f3a..24e7d5592e799 100644
> --- a/drivers/net/netdevsim/ethtool.c
> +++ b/drivers/net/netdevsim/ethtool.c
> @@ -200,7 +200,22 @@ static int nsim_get_ts_info(struct net_device *dev,
> {
> struct netdevsim *ns = netdev_priv(dev);
>
> + ethtool_op_get_ts_info(dev, info);
> + if (!ns->phc) {
> + info->phc_index = -1;
> + return 0;
> + }
> +
[Severity: High]
[Finding: main-1-1]
[Sources: opus-5]
This isn't a bug introduced by this patch, but the new !ns->phc guard looks
like it silently fixes a NULL pointer dereference that exists today, without
a mention in the changelog or a Fixes: tag.
Before this patch nsim_get_ts_info() did:
info->phc_index = mock_phc_index(ns->phc);
and nsim_ethtool_init() installs the shared nsim_ethtool_ops (including
.get_ts_info = nsim_get_ts_info) on every netdevsim netdev, while only
nsim_init_netdevsim() calls mock_phc_create(). nsim_init_netdevsim_vf()
just assigns nsim_vf_netdev_ops and registers the netdev, so ns->phc stays
NULL for VF netdevs.
With CONFIG_PTP_1588_CLOCK_MOCK=y, mock_phc_index() is the real function:
drivers/ptp/ptp_mock.c:mock_phc_index() {
return ptp_clock_index(phc->clock);
}
so ethtool -T on a netdevsim VF netdev dereferences NULL. Would it make
sense to split this guard into its own patch with a Fixes: tag so that
stable tooling can pick it up?
Also, adding ethtool_op_get_ts_info() here changes ethtool -T output for all
netdevsim devices by advertising SOF_TIMESTAMPING_TX_SOFTWARE,
SOF_TIMESTAMPING_RX_SOFTWARE and SOF_TIMESTAMPING_SOFTWARE. Could the
changelog mention that behaviour change?
> info->phc_index = mock_phc_index(ns->phc);
> + if (info->phc_index < 0)
> + return 0;
> +
> + info->so_timestamping |= SOF_TIMESTAMPING_TX_HARDWARE |
> + SOF_TIMESTAMPING_RX_HARDWARE |
> + SOF_TIMESTAMPING_RAW_HARDWARE;
> +
> + info->tx_types = BIT(HWTSTAMP_TX_OFF) | BIT(HWTSTAMP_TX_ON);
> + info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) | BIT(HWTSTAMP_FILTER_ALL);
>
> return 0;
> }
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index 4e9d7e10b527e..e586fd8f3bc37 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
[ ... ]
> @@ -164,6 +171,44 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
> skb_linearize(skb);
>
> skb_tx_timestamp(skb);
> +
> + if (unlikely(READ_ONCE(peer_ns->tstamp_config.rx_filter) !=
> + HWTSTAMP_FILTER_NONE))
> + ptp_info_rx = mock_phc_get_ptp_info(peer_ns->phc);
> +
> + if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
> + READ_ONCE(ns->tstamp_config.tx_type) == HWTSTAMP_TX_ON)
> + ptp_info_tx = mock_phc_get_ptp_info(ns->phc);
> +
> + if (unlikely(ptp_info_tx || ptp_info_rx)) {
> + /* Capture both timestamps closely */
> + if (ptp_info_tx)
> + ptp_info_tx->gettime64(ptp_info_tx, &tx_ts);
> + if (ptp_info_rx)
> + ptp_info_rx->gettime64(ptp_info_rx, &rx_ts);
> +
> + /* If TX hardware timestamping is enabled report TX timestamp */
> + if (ptp_info_tx) {
> + shhwtstamps.hwtstamp = timespec64_to_ktime(tx_ts);
> + skb_tstamp_tx(skb_orig, &shhwtstamps);
> + }
[Severity: Medium]
[Finding: gpt-5-6-sol-3-10]
[Sources: gpt-5-6-sol, opus-5]
Can a socket receive two SCM_TSTAMP_SND reports for the same packet here?
The hardware report is emitted after skb_tx_timestamp(), which already
delivered the software one:
include/linux/skbuff.h:skb_tx_timestamp() {
skb_clone_tx_timestamp(skb);
if (skb_shinfo(skb)->tx_flags & (SKBTX_SW_TSTAMP | SKBTX_BPF))
skb_tstamp_tx(skb, NULL);
}
The only suppression of the software report lives in __skb_tstamp_tx():
net/core/skbuff.c:__skb_tstamp_tx() {
if (!hwtstamps && !(tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)
return;
netdevsim never sets SKBTX_IN_PROGRESS anywhere - the new code only tests
SKBTX_HW_TSTAMP - so for a socket that enabled both
SOF_TIMESTAMPING_TX_SOFTWARE and SOF_TIMESTAMPING_TX_HARDWARE without
SOF_TIMESTAMPING_OPT_TX_SWHW both reports are delivered.
Would following the usual driver sequence work here, i.e. test
SKBTX_HW_TSTAMP and set SKBTX_IN_PROGRESS before calling
skb_tx_timestamp()?
> +
> + /* Unshare the skb, stamping it must not corrupt cloned copies
> + * and prevents returning single tstamp for Tx and Rx
> + */
> + skb = skb_unshare(skb_orig, GFP_ATOMIC);
> + if (unlikely(!skb)) {
> + if (psp_ext)
> + __skb_ext_put(psp_ext);
> + goto out_drop_cnt;
> + }
[Severity: Low]
[Finding: main-3-8]
[Sources: opus-5]
Should this skb_unshare() be gated on ptp_info_rx?
The enclosing condition is ptp_info_tx || ptp_info_rx, but the only write
into the skb below is guarded by if (ptp_info_rx). On the TX-only path
(peer rx_filter == HWTSTAMP_FILTER_NONE) every timestamped frame gets copied
by skb_copy() for nothing, which also linearizes the buffer that the
hds_config logic just above deliberately left non-linear, and does not carry
queue_mapping.
If the GFP_ATOMIC copy then fails, the frame is dropped via out_drop_cnt
even though nothing needed to be written into it, and after the TX
timestamp was already handed to the socket by skb_tstamp_tx(skb_orig, ...).
[Severity: Low]
[Finding: main-5-12]
[Sources: opus-5]
Does skb_unshare() cover the case this comment describes?
include/linux/skbuff.h:skb_unshare() {
might_sleep_if(gfpflags_allow_blocking(pri));
if (skb_cloned(skb)) {
...
}
The copy is made only for clones, so an skb with skb->users > 1 and
cloned == 0 is returned unchanged. pktgen does
refcount_add(burst, &pkt_dev->skb->users) and resubmits the same skb, so the
skb_hwtstamps(skb) write below touches skb_shared_info that the other
reference holder still owns, and that the peer's NAPI path may read
concurrently after nsim_forward_skb() -> nsim_napi_rx() queues it on
rq->skb_queue. Should the driver check skb_shared() as well, or stamp a
private copy?
> +
> + /* If RX hardware timestamping is enabled report RX timestamp */
> + if (ptp_info_rx)
> + skb_hwtstamps(skb)->hwtstamp =
> + timespec64_to_ktime(rx_ts);
> + }
> +
[Severity: Critical]
[Finding: main-6-13]
[Sources: opus-5]
Can this write corrupt kernel memory for AF_XDP TX skbs?
skb_hwtstamps() returns a pointer into a union in skb_shared_info:
include/linux/skbuff.h:struct skb_shared_info {
union {
struct skb_shared_hwtstamps hwtstamps;
struct xsk_tx_metadata_compl xsk_meta;
};
struct xsk_tx_metadata_compl has a single member, u64 *tx_timestamp, and the
AF_XDP destructor dereferences it with only a NULL check:
net/xdp/xsk.c:xsk_destruct_skb() {
struct xsk_tx_metadata_compl *compl = &skb_shinfo(skb)->xsk_meta;
if (compl->tx_timestamp) {
/* sw completion timestamp, not a real one */
*compl->tx_timestamp = ktime_get_tai_fast_ns();
}
Every generic-xmit AF_XDP skb carries that destructor
(xsk_skb_init_misc() sets skb->destructor = xsk_destruct_skb), and it is
freshly allocated and not cloned, so skb_unshare() above returns the same
skb with the destructor and the overwritten shinfo intact.
__dev_forward_skb() -> skb_scrub_packet() does not orphan the skb or reset
skb_shinfo(), so the value survives to the destructor.
The RX branch is gated only on the peer's setting:
if (unlikely(READ_ONCE(peer_ns->tstamp_config.rx_filter) !=
HWTSTAMP_FILTER_NONE))
ptp_info_rx = mock_phc_get_ptp_info(peer_ns->phc);
so any AF_XDP frame forwarded while the peer has RX timestamping enabled
gets a non-NULL, PHC-derived value in xsk_meta.tx_timestamp, and the
destructor performs an 8-byte write to that address. The mock PHC value is
settable from userspace through clock_settime() on /dev/ptpN via
mock_phc_settime64().
Real NICs avoid this because they only stamp skbs they allocated for RX.
Since netdevsim reuses the TX skb as the RX skb, should it orphan the skb
(clearing skb->destructor) or stamp a private copy before touching
skb_shinfo()->hwtstamps?
> if (unlikely(nsim_forward_skb(dev, peer_dev,
> skb, rq, psp_ext) == NET_RX_DROP))
> goto out_drop_cnt;
> @@ -185,6 +230,63 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
> return NETDEV_TX_OK;
> }
>
> +static int nsim_set_ts_config(struct net_device *netdev,
> + struct kernel_hwtstamp_config *config,
> + struct netlink_ext_ack *extack)
> +{
> + struct netdevsim *ns = netdev_priv(netdev);
> + int rx_filter;
> +
> + if (!ns->phc)
> + return -EOPNOTSUPP;
> +
[ ... ]
> +static int nsim_get_ts_config(struct net_device *netdev,
> + struct kernel_hwtstamp_config *config)
> +{
> + struct netdevsim *ns = netdev_priv(netdev);
> +
> + *config = ns->tstamp_config;
> + return 0;
> +}
[Severity: Low]
[Finding: gpt-5-6-sol-2-6]
[Sources: gpt-5-6-sol, opus-5]
Should nsim_get_ts_config() carry the same !ns->phc check as
nsim_set_ts_config()?
drivers/net/Kconfig allows netdevsim without the mock clock:
depends on PTP_1588_CLOCK_MOCK || PTP_1588_CLOCK_MOCK=n
and with CONFIG_PTP_1588_CLOCK_MOCK=n the stub is used:
include/linux/ptp_mock.h {
static inline struct mock_phc *mock_phc_create(struct device *dev)
{
return NULL;
}
nsim_init_netdevsim() only rejects IS_ERR(), so ns->phc stays NULL. In that
configuration SIOCGHWTSTAMP succeeds and reports tx_type =
HWTSTAMP_TX_OFF / rx_filter = HWTSTAMP_FILTER_NONE, i.e. "hardware
timestamping present but disabled", while nsim_get_ts_info() reports
phc_index = -1 with no hardware so_timestamping and every set attempt is
rejected with -EOPNOTSUPP. Other converted drivers keep the get/set pair
symmetric here.
[ ... ]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v10 net-next 3/3] selftests: drivers/net: Implement ptp4l sync test using netdevsim
2026-08-15 6:31 [PATCH v10 net-next 0/3] Implement PTP support in netdevsim Maciek Machnikowski
2026-08-15 6:31 ` [PATCH v10 net-next 1/3] ptp_mock: Expose ptp_clock_info to external drivers Maciek Machnikowski
2026-08-15 6:31 ` [PATCH v10 net-next 2/3] netdevsim: Implement basic ptp support Maciek Machnikowski
@ 2026-08-15 6:31 ` Maciek Machnikowski
2 siblings, 0 replies; 5+ messages in thread
From: Maciek Machnikowski @ 2026-08-15 6:31 UTC (permalink / raw)
To: netdev
Cc: kuba, maciek, richardcochran, milena.olech, willemdebruijn.kernel,
andrew, vadim.fedorenko, horms
Add PTP synchronization test using ptp4l and netdevsim.
The test uses the NetDrvEpEnv to link a local netdevsim
device to a remote endpoint, runs ptp4l as leader and follower
on the two ends, and waits for the follower to report the
synchronized state (s2).
Signed-off-by: Maciek Machnikowski <maciek@machnikowski.net>
---
tools/testing/selftests/drivers/net/Makefile | 1 +
tools/testing/selftests/drivers/net/config | 1 +
tools/testing/selftests/drivers/net/ptp.py | 84 ++++++++++++++++++++
3 files changed, 86 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/ptp.py
diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index d5bf4cb638a8..18bb7c693b69 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -19,6 +19,7 @@ TEST_PROGS := \
netpoll_basic.py \
ping.py \
psp.py \
+ ptp.py \
queues.py \
ring_reconfig.py \
shaper.py \
diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config
index b6989c7d3d9d..7be30a72e119 100644
--- a/tools/testing/selftests/drivers/net/config
+++ b/tools/testing/selftests/drivers/net/config
@@ -21,5 +21,6 @@ CONFIG_NET_SCH_INGRESS=y
CONFIG_NET_SCH_PRIO=m
CONFIG_PPP=y
CONFIG_PPPOE=y
+CONFIG_PTP_1588_CLOCK_MOCK=y
CONFIG_VLAN_8021Q=m
CONFIG_XDP_SOCKETS=y
diff --git a/tools/testing/selftests/drivers/net/ptp.py b/tools/testing/selftests/drivers/net/ptp.py
new file mode 100755
index 000000000000..57ea77659b11
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/ptp.py
@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# By Maciek Machnikowski <maciek@machnikowski.net> (c) 2026,
+
+"""
+Test suite for PTP sync using ptp4l.
+
+Start a ptp4l leader and follower and check that the follower locks onto the
+leader (state s2)
+"""
+
+import time
+
+from lib.py import (
+ NetDrvEpEnv,
+ bkg,
+ fd_read_timeout,
+ ksft_exit,
+ ksft_pr,
+ ksft_run,
+ ksft_true,
+)
+
+PTP4L_SYNC_TIMEOUT = 40
+
+
+def _poll_follower_sync(follower, timeout):
+ """Read the follower stdout pipe until ptp4l reports sync state s2.
+
+ Returns a tuple (synced, output) where output is the text read so far.
+ The synchronized ptp4l log line looks like this:
+ ptp4l[18374.558]: master offset 844 s2 freq +822 path delay 521
+ """
+ fd_file = follower.proc.stdout
+ fd = fd_file.fileno()
+ buf = b""
+ deadline = time.monotonic() + timeout
+ while time.monotonic() < deadline:
+ if b" s2 " in buf:
+ break
+ if follower.proc.poll() is not None:
+ chunk = fd_file.read()
+ if chunk:
+ buf += chunk
+ break
+ try:
+ remaining = deadline - time.monotonic()
+ buf += fd_read_timeout(fd, max(0, min(1, remaining)))
+ except TimeoutError:
+ continue
+ return b" s2 " in buf, buf.decode("utf-8", "replace")
+
+
+def ptp_sync_test(cfg):
+ """Verify ptp4l leader/follower synchronization reaches state s2."""
+ cfg.require_cmd("ptp4l", remote=True)
+
+ leader_cmd = f"ptp4l -i {cfg.remote_ifname} -m -2"
+ follower_cmd = f"ptp4l -i {cfg.ifname} -m -s -2"
+
+ with bkg(leader_cmd, host=cfg.remote), \
+ bkg(follower_cmd) as follower:
+ synced, output = _poll_follower_sync(follower, PTP4L_SYNC_TIMEOUT)
+
+ if synced:
+ return
+
+ ksft_pr(f"ptp4l follower did not reach locked state (s2) within "
+ f"{PTP4L_SYNC_TIMEOUT}s")
+ tail = output.strip().split("\n")[-10:]
+ ksft_pr("Follower log (last 10 lines): " + " | ".join(tail))
+ ksft_true(False, "PTP sync timeout")
+
+
+def main():
+ """Run ksft tests."""
+ with NetDrvEpEnv(__file__) as cfg:
+ ksft_run([ptp_sync_test], args=(cfg, ))
+ ksft_exit()
+
+
+if __name__ == "__main__":
+ main()
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread