* Re: [PATCH net-next] ethtool: tsconfig: always take rtnl_lock
2026-06-11 20:03 [PATCH net-next] ethtool: tsconfig: always take rtnl_lock Jakub Kicinski
@ 2026-06-11 20:05 ` Jakub Kicinski
2026-06-11 20:05 ` Jakub Kicinski
2026-06-11 20:24 ` Stanislav Fomichev
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-06-11 20:05 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, andrew, gal,
jacob.e.keller, sdf, kory.maincent
On Thu, 11 Jun 2026 13:03:55 -0700 Jakub Kicinski wrote:
> Fixes: 6e9e2eed4f39 ("net: ethtool: Add support for tsconfig command to get/set hwtstamp config")
Ugh, wrong fixes tag of course, should be:
Fixes: f9a3e05114b8 ("net: ethtool: optionally skip rtnl_lock on Netlink path for SET ops")
I guess the GET side was also wrong but its harmless.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] ethtool: tsconfig: always take rtnl_lock
2026-06-11 20:05 ` Jakub Kicinski
@ 2026-06-11 20:05 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-06-11 20:05 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, andrew, gal,
jacob.e.keller, sdf, kory.maincent
On Thu, 11 Jun 2026 13:05:20 -0700 Jakub Kicinski wrote:
> On Thu, 11 Jun 2026 13:03:55 -0700 Jakub Kicinski wrote:
> > Fixes: 6e9e2eed4f39 ("net: ethtool: Add support for tsconfig command to get/set hwtstamp config")
>
> Ugh, wrong fixes tag of course, should be:
>
> Fixes: f9a3e05114b8 ("net: ethtool: optionally skip rtnl_lock on Netlink path for SET ops")
>
> I guess the GET side was also wrong but its harmless.
I mean -- so it doesn't need a Fixes tag, it's covered by the patch,
of course.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ethtool: tsconfig: always take rtnl_lock
2026-06-11 20:03 [PATCH net-next] ethtool: tsconfig: always take rtnl_lock Jakub Kicinski
2026-06-11 20:05 ` Jakub Kicinski
@ 2026-06-11 20:24 ` Stanislav Fomichev
2026-06-11 20:55 ` Jacob Keller
2026-06-11 22:25 ` Vadim Fedorenko
3 siblings, 0 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2026-06-11 20:24 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, andrew,
gal, jacob.e.keller, sdf, kory.maincent
On 06/11, Jakub Kicinski wrote:
> mlx5 throws ASSERT_RTNL() warnings on timestamp config, because
> it tries to update features. mlx5e_hwtstamp_set() calls
> netdev_update_features().
>
> I missed this while grepping the drivers because tsconfig goes
> through ndo_hwtstamp_set/get, not ethtool ops, even tho the new
> uAPI is in ethtool Netlink. We could add a dedicated opt out bit
> for mlx5, but NDOs were not supposed to be part of the ethtool locking
> conversion in the first place.
>
> The mlx5 features update is related to the "compressed CQE" format
> which lacks timestamp, apparently. See commit c0194e2d0ef0 ("net/mlx5e:
> Disable rxhash when CQE compress is enabled").
>
> Fixes: 6e9e2eed4f39 ("net: ethtool: Add support for tsconfig command to get/set hwtstamp config")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] ethtool: tsconfig: always take rtnl_lock
2026-06-11 20:03 [PATCH net-next] ethtool: tsconfig: always take rtnl_lock Jakub Kicinski
2026-06-11 20:05 ` Jakub Kicinski
2026-06-11 20:24 ` Stanislav Fomichev
@ 2026-06-11 20:55 ` Jacob Keller
2026-06-11 22:25 ` Vadim Fedorenko
3 siblings, 0 replies; 6+ messages in thread
From: Jacob Keller @ 2026-06-11 20:55 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, andrew, gal, sdf,
kory.maincent
On 6/11/2026 1:03 PM, Jakub Kicinski wrote:
> mlx5 throws ASSERT_RTNL() warnings on timestamp config, because
> it tries to update features. mlx5e_hwtstamp_set() calls
> netdev_update_features().
>
> I missed this while grepping the drivers because tsconfig goes
> through ndo_hwtstamp_set/get, not ethtool ops, even tho the new
> uAPI is in ethtool Netlink. We could add a dedicated opt out bit
> for mlx5, but NDOs were not supposed to be part of the ethtool locking
> conversion in the first place.
Makes sense, even if we have plans to improve that in the future we
didn't plan to change it right now.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>
> The mlx5 features update is related to the "compressed CQE" format
> which lacks timestamp, apparently. See commit c0194e2d0ef0 ("net/mlx5e:
> Disable rxhash when CQE compress is enabled").
>
> Fixes: 6e9e2eed4f39 ("net: ethtool: Add support for tsconfig command to get/set hwtstamp config")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: andrew@lunn.ch
> CC: gal@nvidia.com
> CC: jacob.e.keller@intel.com
> CC: sdf@fomichev.me
> CC: kory.maincent@bootlin.com
> ---
> net/ethtool/common.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/ethtool/common.h b/net/ethtool/common.h
> index e3052972f953..2b3847f00801 100644
> --- a/net/ethtool/common.h
> +++ b/net/ethtool/common.h
> @@ -113,6 +113,13 @@ ethtool_nl_msg_needs_rtnl(const struct net_device *dev, u8 cmd)
> return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM;
> case ETHTOOL_MSG_RSS_SET:
> return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_RSS;
> + case ETHTOOL_MSG_TSCONFIG_GET:
> + case ETHTOOL_MSG_TSCONFIG_SET:
> + /* tsconfig calls ndos (ndo_hwtstamp_set/get), not ethtool ops.
> + * Also, there is no corresponding ethtool ioctl, therefore
> + * these cases are Netlink-only.
> + */
> + return true;
> }
> return false;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] ethtool: tsconfig: always take rtnl_lock
2026-06-11 20:03 [PATCH net-next] ethtool: tsconfig: always take rtnl_lock Jakub Kicinski
` (2 preceding siblings ...)
2026-06-11 20:55 ` Jacob Keller
@ 2026-06-11 22:25 ` Vadim Fedorenko
3 siblings, 0 replies; 6+ messages in thread
From: Vadim Fedorenko @ 2026-06-11 22:25 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, andrew, gal,
jacob.e.keller, sdf, kory.maincent
On 11/06/2026 21:03, Jakub Kicinski wrote:
> mlx5 throws ASSERT_RTNL() warnings on timestamp config, because
> it tries to update features. mlx5e_hwtstamp_set() calls
> netdev_update_features().
>
> I missed this while grepping the drivers because tsconfig goes
> through ndo_hwtstamp_set/get, not ethtool ops, even tho the new
> uAPI is in ethtool Netlink. We could add a dedicated opt out bit
> for mlx5, but NDOs were not supposed to be part of the ethtool locking
> conversion in the first place.
>
> The mlx5 features update is related to the "compressed CQE" format
> which lacks timestamp, apparently. See commit c0194e2d0ef0 ("net/mlx5e:
> Disable rxhash when CQE compress is enabled").
>
> Fixes: 6e9e2eed4f39 ("net: ethtool: Add support for tsconfig command to get/set hwtstamp config")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: andrew@lunn.ch
> CC: gal@nvidia.com
> CC: jacob.e.keller@intel.com
> CC: sdf@fomichev.me
> CC: kory.maincent@bootlin.com
> ---
> net/ethtool/common.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/ethtool/common.h b/net/ethtool/common.h
> index e3052972f953..2b3847f00801 100644
> --- a/net/ethtool/common.h
> +++ b/net/ethtool/common.h
> @@ -113,6 +113,13 @@ ethtool_nl_msg_needs_rtnl(const struct net_device *dev, u8 cmd)
> return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM;
> case ETHTOOL_MSG_RSS_SET:
> return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_RSS;
> + case ETHTOOL_MSG_TSCONFIG_GET:
> + case ETHTOOL_MSG_TSCONFIG_SET:
> + /* tsconfig calls ndos (ndo_hwtstamp_set/get), not ethtool ops.
> + * Also, there is no corresponding ethtool ioctl, therefore
> + * these cases are Netlink-only.
> + */
> + return true;
> }
> return false;
> }
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
^ permalink raw reply [flat|nested] 6+ messages in thread