Re: [PATCH net-next v4 1/4] net: hsr: add PRP interlink (RedBox) datapath and duplicate discard

On Fri, Jul 24, 2026 at 11:27:57PM +0200, Xin Xie wrote:

On Fri, 24 Jul 2026 14:12:30 +0100 Simon Horman wrote:

quoted

[Severity: High]
... does modifying the MAC header directly in hsr_xmit() corrupt
shared packet data? [...]
Should the skb be unshared before making these direct modifications?

I checked this against the v4 base and the series diff.

The shared-data write is real. hsr_xmit() writes
eth_hdr(skb)->h_source in place - the slave-address substitution
for master-originated frames, and the macaddress_redbox
substitution for the interlink port - on skbs that can be clones.
Multiple consumers can share the same packet data while needing
different source addresses: a PRP LAN broadcast or multicast can be
delivered to the master (source rewritten via
hsr_addr_subst_source()) and forwarded to the interlink (rewritten
to macaddress_redbox) from the same underlying buffer.

Two scope facts. This is pre-existing HSR RedBox code, present
since v6.11: hsr_xmit() itself is untouched by the series (patch
1/4's hsr_forward.c changes sit below it and do not modify its
body), and the series' only h_source-adjacent change moves an
existing ether_addr_copy() a few lines earlier in
hsr_dev_finalize(), where no skb is involved. But the series does
make the existing interlink path reachable for PRP.

I will handle this as an independent fix for the existing HSR/PRP
code, posted separately for net with its own Fixes: tag - not
folded into this feature series.

Would you like the feature series to wait for that fix, or may v4
proceed while it is reviewed separately?

TL;DR: Yes, I think this patchset can proceed and fixes for
       pre-existing issues can be addressed separately.

Thanks for the detailed feedback above, and likewise on patch 3/4.
It is much appreciated.

My view is that in the age of AI-generated review there is a risk that
making corrections pre-existing problems a pre-requisite risks perpetual
scope increase. So I think that that unless a pre-existing bug is made
materially worse, or fundamentally prevents a patch from reaching it's
goals, then fixing the pre-existing bug can be considered as follow-up.

Of course, drawing that line is a subjective matter.  But it seems to me
that the pre-existing issues flagged both here and in the review I
forwarded for 3/4 can be treated as follow-up.

(The hsr_create_tagged_frame() side question overlaps the third
finding in your 3/4 mail; it is answered there.)