From: Haiyang Zhang <haiyangz@microsoft.com>
To: Wojciech Drewek <wojciech.drewek@intel.com>,
"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: KY Srinivasan <kys@microsoft.com>,
"wei.liu@kernel.org" <wei.liu@kernel.org>,
Dexuan Cui <decui@microsoft.com>,
"edumazet@google.com" <edumazet@google.com>,
"kuba@kernel.org" <kuba@kernel.org>,
"pabeni@redhat.com" <pabeni@redhat.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH net] hv_netvsc: fix race of netvsc and VF register_netdevice
Date: Thu, 26 Oct 2023 14:52:07 +0000 [thread overview]
Message-ID: <BL1PR21MB3113F0E65A58AEB1E617C2C4CADDA@BL1PR21MB3113.namprd21.prod.outlook.com> (raw)
In-Reply-To: <99f1f2d1-8fb0-4e56-9820-86254ce8bd02@intel.com>
> -----Original Message-----
> From: Wojciech Drewek <wojciech.drewek@intel.com>
> Sent: Thursday, October 26, 2023 6:48 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org
> Cc: KY Srinivasan <kys@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <decui@microsoft.com>; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; davem@davemloft.net; linux-kernel@vger.kernel.org;
> stable@vger.kernel.org
> Subject: Re: [PATCH net] hv_netvsc: fix race of netvsc and VF
> register_netdevice
>
> [You don't often get email from wojciech.drewek@intel.com. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On 25.10.2023 23:16, Haiyang Zhang wrote:
> > The rtnl lock also needs to be held before rndis_filter_device_add()
> > which advertises nvsp_2_vsc_capability / sriov bit, and triggers
> > VF NIC offering and registering. If VF NIC finished register_netdev()
> > earlier it may cause name based config failure.
> >
> > To fix this issue, move the call to rtnl_lock() before
> > rndis_filter_device_add(), so VF will be registered later than netvsc
> > / synthetic NIC, and gets a name numbered (ethX) after netvsc.
> >
> > And, move register_netdevice_notifier() earlier, so the call back
> > function is set before probing.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: e04e7a7bbd4b ("hv_netvsc: Fix a deadlock by getting rtnl lock earlier
> in netvsc_probe()")
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > ---
> > drivers/net/hyperv/netvsc_drv.c | 30 +++++++++++++++++++-----------
> > 1 file changed, 19 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> > index 3ba3c8fb28a5..feca1391f756 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -2531,15 +2531,6 @@ static int netvsc_probe(struct hv_device *dev,
> > goto devinfo_failed;
> > }
> >
> > - nvdev = rndis_filter_device_add(dev, device_info);
> > - if (IS_ERR(nvdev)) {
> > - ret = PTR_ERR(nvdev);
> > - netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
> > - goto rndis_failed;
> > - }
> > -
> > - eth_hw_addr_set(net, device_info->mac_adr);
> > -
> > /* We must get rtnl lock before scheduling nvdev->subchan_work,
> > * otherwise netvsc_subchan_work() can get rtnl lock first and wait
> > * all subchannels to show up, but that may not happen because
> > @@ -2547,9 +2538,23 @@ static int netvsc_probe(struct hv_device *dev,
> > * -> ... -> device_add() -> ... -> __device_attach() can't get
> > * the device lock, so all the subchannels can't be processed --
> > * finally netvsc_subchan_work() hangs forever.
> > + *
> > + * The rtnl lock also needs to be held before rndis_filter_device_add()
> > + * which advertises nvsp_2_vsc_capability / sriov bit, and triggers
> > + * VF NIC offering and registering. If VF NIC finished register_netdev()
> > + * earlier it may cause name based config failure.
> > */
> > rtnl_lock();
> >
> > + nvdev = rndis_filter_device_add(dev, device_info);
> > + if (IS_ERR(nvdev)) {
> > + ret = PTR_ERR(nvdev);
> > + netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
> > + goto rndis_failed;
>
> In case of error rtnl won't be unlocked.
Good catch! Will correct this.
Thanks,
- Haiyang
prev parent reply other threads:[~2023-10-26 14:52 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-25 21:16 [PATCH net] hv_netvsc: fix race of netvsc and VF register_netdevice Haiyang Zhang
2023-10-26 10:47 ` Wojciech Drewek
2023-10-26 14:52 ` Haiyang Zhang [this message]
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=BL1PR21MB3113F0E65A58AEB1E617C2C4CADDA@BL1PR21MB3113.namprd21.prod.outlook.com \
--to=haiyangz@microsoft.com \
--cc=davem@davemloft.net \
--cc=decui@microsoft.com \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=kys@microsoft.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=stable@vger.kernel.org \
--cc=wei.liu@kernel.org \
--cc=wojciech.drewek@intel.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).