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);
> + }
> +}
