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

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/[email protected]?part=1
---
--- Patch [1]: [PATCH net v2 1/4] net: hsr: fix packet drops caused by GRO superpackets ---
commit b386202ef95164a9e68085058531ac441d9df79d
Author: Xin Xie <[email protected]>

net: hsr: fix packet drops caused by GRO superpackets

This commit fixes HSR/PRP packet drops by preventing lower devices from
aggregating frames into GRO super-packets. It adds dev_disable_gro() and
netif_disable_gro() functions and calls them during HSR port setup.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5933c5dab09ee..a6cf2adc86256 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c

[ ... ]

> @@ -1840,6 +1840,24 @@ void netif_disable_lro(struct net_device *dev)
>  	}
>  }
>  
> +void netif_disable_gro(struct net_device *dev)
> +{
> +	struct net_device *lower_dev;
> +	struct list_head *iter;
> +
> +	dev->wanted_features &= ~(NETIF_F_GRO | NETIF_F_GRO_HW);

[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?

> +	netdev_update_features(dev);
> +
> +	if (unlikely(dev->features & (NETIF_F_GRO | NETIF_F_GRO_HW)))
> +		netdev_WARN(dev, "failed to disable GRO!\n");
> +
> +	netdev_for_each_lower_dev(dev, lower_dev, iter) {
> +		netdev_lock_ops(lower_dev);
> +		netif_disable_gro(lower_dev);
> +		netdev_unlock_ops(lower_dev);
> +	}
> +}