On 30/07/2026 16:46, Simon Horman wrote: > [Severity: High] > Does this one-time modification of wanted_features prevent userspace from > subsequently re-enabling GRO? > > Since NETIF_F_GRO and NETIF_F_GRO_HW are not included in the > NETIF_F_UPPER_DISABLES macro definition in include/linux/netdev_features.h, > would netdev_sync_upper_features() and netdev_sync_lower_features() fail > to enforce the disabled state on lower devices during features recalculation? > > Could an administrator run ethtool -K eth0 gro on (or add a new physical > device to an intermediate upper device already enslaved to HSR) to bypass > this restriction and cause the HSR packet drops to resurface? The review is correct that dev_disable_gro() is setup-time enforcement, not an immutable policy. Clearing wanted_features prevents an ordinary feature recalculation from restoring GRO, but a privileged administrator can explicitly request it again with ethtool. Adding a new lower device below an already enslaved stacked device can similarly escape the recursive setup-time walk.
I did not add GRO to NETIF_F_UPPER_DISABLES because that is a generic upper/lower feature rule. It would make an upper device which does not advertise GRO disable GRO on all of its lowers, well beyond HSR/PRP. That seems too broad for this fix. Patch 3 is the second line of defense. Even if GRO is explicitly re-enabled, no GSO super-packet is forwarded as one HSR/PRP frame. Plain Ethernet GSO packets from the master or interlink are unfolded and each segment receives its own tag and sequence number. GSO packets from an HSR/PRP LAN slave are rejected because software cannot reconstruct their individual HSR tags or PRP RCTs. Patch 2 makes the segmentation path practical by moving the expensive work outside seqnr_lock; it is not itself an additional GRO guard. The two layers therefore have different roles: patch 1 establishes the safe default when a port is enslaved, while patch 3 guarantees fail-safe handling if a super-packet nevertheless reaches the forward path. An administrator override can still cause packet loss on LAN ingress, but it cannot make an aggregate bypass the per-frame forwarding invariant. -- Xin
