Re: [PATCH net] net: ethtool: keep rtnl_lock for ops using ethtool_op_get_link()
Harshitha Ramamurthy <[email protected]>
| Newsgroups | org.kernel.vger.linux-hyperv,org.kernel.vger.linux-rdma,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CAEAWyHfDMjZ7b9LphcUn_Xrz2yWXdSD9cDj3bwW57+=2y+UsmA@mail.gmail.com> |
On Wed, Jun 24, 2026 at 12:04 PM Jakub Kicinski <[email protected]> wrote: > > Breno reports following splats on mlx5: > > RTNL: assertion failed at net/core/dev.c (2241) > WARNING: net/core/dev.c:2241 at netif_state_change+0xed/0x130, CPU#5: ethtool/1335 > RIP: 0010:netif_state_change+0xf9/0x130 > Call Trace: > <TASK> > __linkwatch_sync_dev+0xea/0x120 > ethtool_op_get_link+0xe/0x20 > __ethtool_get_link+0x26/0x40 > linkstate_prepare_data+0x51/0x200 > ethnl_default_doit+0x213/0x470 > genl_family_rcv_msg_doit+0xdd/0x110 > > Looks like I missed ethtool_op_get_link() trying to sync linkwatch, > which needs rtnl_lock. Not all drivers do this - bnxt doesn't, > it just returns the link state, so add an opt-in bit. > > Reported-by: Breno Leitao <[email protected]> > Fixes: 45079e00133e ("net: ethtool: optionally skip rtnl_lock on Netlink path for GET ops") > Signed-off-by: Jakub Kicinski <[email protected]> > --- > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > CC: [email protected] > --- > include/linux/ethtool.h | 2 ++ > net/ethtool/common.h | 4 ++++ > drivers/net/ethernet/google/gve/gve_ethtool.c | 3 ++- > drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 1 + > drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 3 ++- > drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 3 ++- > drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c | 4 +++- > drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c | 3 ++- > drivers/net/ethernet/microsoft/mana/mana_ethtool.c | 3 ++- > 9 files changed, 20 insertions(+), 6 deletions(-) > > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index 1b834e2a522e..5d491a98265e 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -942,6 +942,7 @@ struct kernel_ethtool_ts_info { > #define ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM BIT(5) > #define ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM BIT(6) > #define ETHTOOL_OP_NEEDS_RTNL_RSS BIT(7) > +#define ETHTOOL_OP_NEEDS_RTNL_GLINK BIT(8) > > /** > * struct ethtool_ops - optional netdev operations > @@ -978,6 +979,7 @@ struct kernel_ethtool_ts_info { > * - phylink helpers (note that phydev is currently unsupported!) > * - netdev_update_features() > * - netif_set_real_num_tx_queues() > + * - ethtool_op_get_link() (syncs link watch under rtnl_lock) > * > * @get_drvinfo: Report driver/device information. Modern drivers no > * longer have to implement this callback. Most fields are > diff --git a/net/ethtool/common.h b/net/ethtool/common.h > index 2b3847f00801..4e5356e26f40 100644 > --- a/net/ethtool/common.h > +++ b/net/ethtool/common.h > @@ -113,6 +113,8 @@ 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_LINKSTATE_GET: > + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_GLINK; > case ETHTOOL_MSG_TSCONFIG_GET: > case ETHTOOL_MSG_TSCONFIG_SET: > /* tsconfig calls ndos (ndo_hwtstamp_set/get), not ethtool ops. > @@ -159,6 +161,8 @@ ethtool_ioctl_needs_rtnl(const struct net_device *dev, u32 ethcmd) > case ETHTOOL_SRXFH: > case ETHTOOL_SRXFHINDIR: > return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_RSS; > + case ETHTOOL_GLINK: > + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_GLINK; > } > return false; > } > diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c > index 7cc22916852f..8199738ba979 100644 > --- a/drivers/net/ethernet/google/gve/gve_ethtool.c > +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c > @@ -984,7 +984,8 @@ const struct ethtool_ops gve_ethtool_ops = { > .supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT | > ETHTOOL_RING_USE_RX_BUF_LEN, > .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | > - ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, > + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | > + ETHTOOL_OP_NEEDS_RTNL_GLINK, Acked-by: Harshitha Ramamurthy <[email protected]> Thanks for the fix! > .get_drvinfo = gve_get_drvinfo, > .get_strings = gve_get_strings, > .get_sset_count = gve_get_sset_count, > diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > index a615d599b88e..e7cf12eaa268 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > @@ -1855,6 +1855,7 @@ static const struct ethtool_ops iavf_ethtool_ops = { > .supported_coalesce_params = ETHTOOL_COALESCE_USECS | > ETHTOOL_COALESCE_USE_ADAPTIVE, > .supported_input_xfrm = RXH_XFRM_SYM_XOR, > + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_GLINK, > .get_drvinfo = iavf_get_drvinfo, > .get_link = ethtool_op_get_link, > .get_ringparam = iavf_get_ringparam, > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c > index 2f5b626ba33f..112926d07634 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c > @@ -2721,7 +2721,8 @@ const struct ethtool_ops mlx5e_ethtool_ops = { > .rxfh_max_num_contexts = MLX5E_MAX_NUM_RSS, > .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | > ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | > - ETHTOOL_OP_NEEDS_RTNL_SPFLAGS, > + ETHTOOL_OP_NEEDS_RTNL_SPFLAGS | > + ETHTOOL_OP_NEEDS_RTNL_GLINK, > .supported_coalesce_params = ETHTOOL_COALESCE_USECS | > ETHTOOL_COALESCE_MAX_FRAMES | > ETHTOOL_COALESCE_USE_ADAPTIVE | > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c > index 1a8a19f980d3..c8b76d301c92 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c > @@ -419,7 +419,8 @@ static const struct ethtool_ops mlx5e_rep_ethtool_ops = { > ETHTOOL_COALESCE_MAX_FRAMES | > ETHTOOL_COALESCE_USE_ADAPTIVE, > .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | > - ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, > + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | > + ETHTOOL_OP_NEEDS_RTNL_GLINK, > .get_drvinfo = mlx5e_rep_get_drvinfo, > .get_link = ethtool_op_get_link, > .get_strings = mlx5e_rep_get_strings, > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c > index 9b3b32408c64..01ddc3def9ac 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c > @@ -286,7 +286,8 @@ const struct ethtool_ops mlx5i_ethtool_ops = { > ETHTOOL_COALESCE_MAX_FRAMES | > ETHTOOL_COALESCE_USE_ADAPTIVE, > .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | > - ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, > + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | > + ETHTOOL_OP_NEEDS_RTNL_GLINK, > .get_drvinfo = mlx5i_get_drvinfo, > .get_strings = mlx5i_get_strings, > .get_sset_count = mlx5i_get_sset_count, > @@ -309,6 +310,7 @@ const struct ethtool_ops mlx5i_ethtool_ops = { > }; > > const struct ethtool_ops mlx5i_pkey_ethtool_ops = { > + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_GLINK, > .get_drvinfo = mlx5i_get_drvinfo, > .get_link = ethtool_op_get_link, > .get_ts_info = mlx5i_get_ts_info, > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c > index cb34fc166ef9..0e47088ec44b 100644 > --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c > @@ -2024,7 +2024,8 @@ static const struct ethtool_ops fbnic_ethtool_ops = { > ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM | > ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM | > ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | > - ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, > + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | > + ETHTOOL_OP_NEEDS_RTNL_GLINK, > .get_drvinfo = fbnic_get_drvinfo, > .get_regs_len = fbnic_get_regs_len, > .get_regs = fbnic_get_regs, > diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c > index 94e658d07a27..881df597d7f9 100644 > --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c > +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c > @@ -597,7 +597,8 @@ static int mana_get_link_ksettings(struct net_device *ndev, > const struct ethtool_ops mana_ethtool_ops = { > .supported_coalesce_params = ETHTOOL_COALESCE_RX_CQE_FRAMES, > .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | > - ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, > + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | > + ETHTOOL_OP_NEEDS_RTNL_GLINK, > .get_ethtool_stats = mana_get_ethtool_stats, > .get_sset_count = mana_get_sset_count, > .get_strings = mana_get_strings, > -- > 2.54.0 >