Re: [PATCH v2 net-next 6/7] net: dsa: netc: add PTP two-step timestamping support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lazy garbage collection of timed-out TX SKBs permanently leaks sockets and network namespaces.
--

--- Patch [6]: [PATCH v2 net-next 6/7] net: dsa: netc: add PTP two-step timestamping support ---
commit b5b95ff6c72f97e31680784516eda771a0e7104f
Author: Wei Fang <wei.fang@nxp.com>
Date:   Sat Aug 8 03:21:45 2026 +0000

    net: dsa: netc: add PTP two-step timestamping support

This commit adds two-step TX timestamping and RX timestamping support for the
NETC switch. It installs ingress port filter table rules to redirect RX PTP
frames to the CPU port and handles TX timestamp requests and responses via
switch tags.

> diff --git a/drivers/net/dsa/netc/netc_ptp.c b/drivers/net/dsa/netc/netc_ptp.c
> new file mode 100644
> index 0000000000000..1384a6f31d1c1
> --- /dev/null
> +++ b/drivers/net/dsa/netc/netc_ptp.c

[ ... ]

> @@ -0,0 +1,454 @@

[ ... ]

> +static void netc_port_txtstamp_twostep(struct netc_port *np,
> +				       struct sk_buff *nskb)
> +{
> +	DECLARE_BITMAP(ts_req_id_bitmap, NETC_NUM_TS_REQ_ID);
> +	struct sk_buff *clone = skb_clone_sk(nskb);
> +	struct netc_switch *priv = np->switch_priv;
> +	struct sk_buff_head free_list;
> +	struct sk_buff *skb, *skb_tmp;
> +	unsigned long ts_req_id;
> +	int err = 0;
> +
> +	if (unlikely(!clone))
> +		return;
> +
> +	bitmap_zero(ts_req_id_bitmap, NETC_NUM_TS_REQ_ID);
> +	__skb_queue_head_init(&free_list);
> +	spin_lock_bh(&np->tstamp_lock);
> +
> +	skb_queue_walk_safe(&np->skb_txtstamp_queue, skb, skb_tmp) {
> +		if (time_before(NETC_SKB_CB(skb)->ptp_tx_time +
> +				NETC_TXTSTAMP_TIMEOUT, jiffies)) {

[Severity: High]
Does this lazy garbage collection of timed-out TX SKBs permanently leak sockets
and network namespaces?

When a frame requests a two-step hardware TX timestamp, skb_clone_sk() is used,
which takes a reference on the socket. If the frame is dropped in software
(e.g., due to qdisc drops or congestion) before reaching the hardware, the
hardware never generates a timestamp response, and the clone remains in
np->skb_txtstamp_queue.

Because the driver only lazily checks for timed-out SKBs here when a new PTP
packet is transmitted, the queue will not be purged if the application stops
transmitting or closes the socket. The stranded clone holds a reference to the
socket indefinitely, which in turn prevents the associated network namespace
from being destroyed.

> +			dev_dbg_ratelimited(priv->dev,
> +					    "Port %d ts_req_id %u which seems lost\n",
> +					    np->dp->index, NETC_SKB_CB(skb)->ts_req_id);
> +
> +			__skb_unlink(skb, &np->skb_txtstamp_queue);
> +			__skb_queue_tail(&free_list, skb);
> +		} else {
> +			__set_bit(NETC_SKB_CB(skb)->ts_req_id, ts_req_id_bitmap);
> +		}
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808032146.2335723-1-wei.fang@oss.nxp.com?part=6