Re: [PATCH net-next v8 5/8] hsr: Use the port and header information in hsr_forward_skb()

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-10-06 14:31:18 [+0000], netdev-bot+sashiko@kernel.org wrote:
> [Severity: Medium]
> Does "deliver it only on the requested port" still hold when the slaves
> have HSR hardware offload enabled?

No, this is software only. Driver with HW offload support enabled need
to be adjusted to support PTP without duplicating packets and so on.

> [Severity: Low]
> Does this clone lose the socket ownership of the original skb?
> 
> __skb_clone() sets n->sk and n->destructor to NULL on the clone. After
> that, hsr_forward_skb() frees the socket-owned original:
> 
> 	kfree_skb(frame.skb_std);
> 
> That runs sock_wfree() while the clone may still be queued on the slave.
> The socket's wmem accounting then stops covering the in-flight packet.
> Also, skb_tstamp_tx() sees a NULL sk, so the PTP TX timestamp never reaches
> the sender.
> 
> A later commit in the series, "hsr: Assign a socket for cloned skbs",
> seems to fix this. It adds the following to this branch, and does the same
> for the req_tx_port case in hsr_create_tagged_frame():
> 
> 	if (skb && frame->skb_std->sk)
> 		skb_set_owner_w(skb, frame->skb_std->sk);
> 
> The old path through hsr_create_tagged_frame() also cloned without
> restoring the socket, so this patch doesn't make things worse.
> 
> Could that fix be folded into this patch?

No, I want to keep it separate. The skbs are simply cloned, preserving
as much data as possible and later the socket is wired up. This is
primary to get the timestamp to work.

> [ ... ]

Sebastian