Netdev List
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Alexander Duyck <alexander.duyck@gmail.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	kernel-team@meta.com
Subject: Re: [net PATCH v2 1/6] net: ethtool: keep rtnl_lock for the ioctl self test
Date: Fri, 18 Sep 2026 17:12:32 +0100	[thread overview]
Message-ID: <20260918161232.GQ51261@horms.kernel.org> (raw)
In-Reply-To: <178942019771.7700.338431553546884773.stgit@ahduyck-xeon-server.home.arpa>

On Mon, Sep 14, 2026 at 02:09:57PM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@fb.com>
> 
> An offline self test that brings the interface down and back up with
> netif_close() / netif_open() requires rtnl_lock for both. Since the
> ethtool IOCTL path became rtnl-optional for ops-locked drivers, the
> ETHTOOL_TEST ioctl runs holding only the netdev instance lock, so on an
> ops-locked driver the self test now tears the device down without
> rtnl_lock.
> 
> With lockdep this reproduces deterministically on every offline self
> test on such a driver; note the sole lock held is the instance lock, not
> rtnl:
> 
>   WARNING: suspicious RCU usage
>   net/core/netpoll.c:207 suspicious rcu_dereference_protected() usage!
>   1 lock held by ethtool/107:
>    #0: (&dev->lock){+.+.}, at: dev_ethtool
>   Call Trace:
>    netpoll_poll_disable
>    __dev_close_many
>    netif_close_many
>    netif_close
>    fbnic_self_test
>    dev_ethtool_locked
>    dev_ethtool
>    dev_ioctl
>    sock_ioctl
>    __x64_sys_ioctl
> 
> Without lockdep the same condition trips ASSERT_RTNL() in
> __dev_close_many() / __dev_open(); that check only samples the global
> rtnl state, so it can be masked by a concurrent rtnl holder, but the
> device is still being reconfigured without the lock it requires.
> 
> The ethtool self_test is a legacy ioctl-only command, so an ETHTOOL_TEST
> case is only needed on the ioctl path. Add an opt-in bit for drivers whose
> self test needs rtnl_lock and set it on the ops-locked drivers whose
> offline self test tears the interface down and up:
> 
>   - fbnic (ops-locked via queue_mgmt_ops): fbnic_self_test() offline path
>     uses netif_close() / netif_open().
>   - bnxt (ops-locked via queue_mgmt_ops): bnxt_self_test() offline path
>     goes through bnxt_close_nic() / bnxt_half_open_nic() /
>     bnxt_half_close_nic() / bnxt_open_nic(), which close and reopen the
>     device.
> 
> Fixes: f994752b1127 ("net: ethtool: optionally skip rtnl_lock on IOCTL path")
> Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>

Reviewed-by: Simon Horman <horms@kernel.org>


  reply	other threads:[~2026-09-18 16:12 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-14 21:09 [net PATCH v2 0/6] eth: fbnic: a collection of fixes Alexander Duyck
2026-09-14 21:09 ` [net PATCH v2 1/6] net: ethtool: keep rtnl_lock for the ioctl self test Alexander Duyck
2026-09-18 16:12   ` Simon Horman [this message]
2026-09-14 21:10 ` [net PATCH v2 2/6] eth: fbnic: Handle maximum standalone channels Alexander Duyck
2026-09-18 16:12   ` Simon Horman
2026-09-14 21:10 ` [net PATCH v2 3/6] eth: fbnic: use the Rx queue napi pointer to find the napi vector Alexander Duyck
2026-09-17 21:12   ` netdev-bot+sashiko
2026-09-18 15:11     ` Alexander Duyck
2026-09-18 16:14       ` Simon Horman
2026-09-18 16:24         ` Alexander Duyck
2026-09-14 21:10 ` [net PATCH v2 4/6] eth: fbnic: reset num_napi when the napi vectors are freed Alexander Duyck
2026-09-18 16:15   ` Simon Horman
2026-09-14 21:10 ` [net PATCH v2 5/6] eth: fbnic: Set AW_FLUSH_MODE alongside AW_FLUSH when flushing the mailbox Alexander Duyck
2026-09-18 16:15   ` Simon Horman
2026-09-14 21:10 ` [net PATCH v2 6/6] eth: fbnic: Handle FW mailbox completions flagged with an error Alexander Duyck
2026-09-17 21:12   ` netdev-bot+sashiko
2026-09-18 15:17     ` Alexander Duyck
2026-09-18 16:16       ` Simon Horman
2026-09-19  0:50 ` [net PATCH v2 0/6] eth: fbnic: a collection of fixes patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260918161232.GQ51261@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=alexander.duyck@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kernel-team@meta.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox