Re: [PATCH net v2 1/4] net: hsr: fix packet drops caused by GRO superpackets

[Xin Xie <xiexinet@gmail.com>]

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