On 6/3/2025 8:43 am, Vladimir Oltean wrote:
I originally changed 252 to 256 because our internal validation failed when setting 256 via ethtool. The test case was based on our old kernel OOT patches code, but this run was done on the upstreamed FPE framework plus this series. After thinking about it, it doesn’t seem right to change this just to accommodate the i226 quirk in a common layer when the IEEE standard and other devices use 252.diff --git a/net/ethtool/mm.c b/net/ethtool/mm.c index ad9b40034003..4c395cd949ab 100644 --- a/net/ethtool/mm.c +++ b/net/ethtool/mm.c @@ -153,7 +153,7 @@ const struct nla_policy ethnl_mm_set_policy[ETHTOOL_A_MM_MAX + 1] = { [ETHTOOL_A_MM_VERIFY_TIME] = NLA_POLICY_RANGE(NLA_U32, 1, 128), [ETHTOOL_A_MM_TX_ENABLED] = NLA_POLICY_MAX(NLA_U8, 1), [ETHTOOL_A_MM_PMAC_ENABLED] = NLA_POLICY_MAX(NLA_U8, 1), - [ETHTOOL_A_MM_TX_MIN_FRAG_SIZE] = NLA_POLICY_RANGE(NLA_U32, 60, 252), + [ETHTOOL_A_MM_TX_MIN_FRAG_SIZE] = NLA_POLICY_RANGE(NLA_U32, 60, 256),Please make this a separate patch with a reasonably convincing justification for any reader, and also state why it is a change that will not introduce regressions to the other drivers. It shows that you've done the due dilligence of checking that they all use ethtool_mm_frag_size_min_to_add(), which errors out on non-standard values. To be clear, extending the policy from 252 to 256 is just to suppress the netlink warning which states that the driver rounds up the minimum fragment size, correct? Because even if you pass 252 (the current netlink maximum), the driver will still use 256.
So, we’ll update our validation to use 252 instead. The driver already rounds up to 256 anyway. I’ll drop this change in the next revision.
Also, noted your point about being cautious with changes that impact other drivers.
Thanks.
