[PATCH net,1/2] hyperv: Fix a kernel warning from netvsc_linkstatus_callback()

[PATCH net,1/2] hyperv: Fix a kernel warning from netvsc_linkstatus_callback()

[Haiyang Zhang]

The warning about local_bh_enable inside IRQ happens when disconnecting a
virtual NIC.

The reason for the warning is -- netif_tx_disable() is called when the NIC
is disconnected. And it's called within irq context. netif_tx_disable() calls
local_bh_enable() which displays warning if in irq.

The fix is to remove the unnecessary netif_tx_disable & wake_queue() in the
netvsc_linkstatus_callback().

Reported-by: Richard Genoud <richard.genoud@gmail.com>
Tested-by: Long Li <longli@microsoft.com>
Tested-by: Richard Genoud <richard.genoud@gmail.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/net/hyperv/netvsc_drv.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 5f85205..8341b62 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -241,13 +241,11 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
 
 	if (status == 1) {
 		netif_carrier_on(net);
-		netif_wake_queue(net);
 		ndev_ctx = netdev_priv(net);
 		schedule_delayed_work(&ndev_ctx->dwork, 0);
 		schedule_delayed_work(&ndev_ctx->dwork, msecs_to_jiffies(20));
 	} else {
 		netif_carrier_off(net);
-		netif_tx_disable(net);
 	}
 }
 
-- 
1.7.4.1

[PATCH net,2/2] hyperv: Fix RNDIS send_completion code path

[Haiyang Zhang]

In some cases, the VM_PKT_COMP message can arrive later than RNDIS completion
message, which will free the packet memory. This may cause panic due to access
to freed memory in netvsc_send_completion().

This patch fixes this problem by removing rndis_filter_send_request_completion()
from the code path. The function was a no-op.

Reported-by: Long Li <longli@microsoft.com>
Tested-by: Long Li <longli@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/net/hyperv/netvsc.c       |   17 ++++++++++++-----
 drivers/net/hyperv/rndis_filter.c |   14 +-------------
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 1cd7748..f5f0f09 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -470,8 +470,10 @@ static void netvsc_send_completion(struct hv_device *device,
 			packet->trans_id;
 
 		/* Notify the layer above us */
-		nvsc_packet->completion.send.send_completion(
-			nvsc_packet->completion.send.send_completion_ctx);
+		if (nvsc_packet)
+			nvsc_packet->completion.send.send_completion(
+				nvsc_packet->completion.send.
+				send_completion_ctx);
 
 		num_outstanding_sends =
 			atomic_dec_return(&net_device->num_outstanding_sends);
@@ -498,6 +500,7 @@ int netvsc_send(struct hv_device *device,
 	int ret = 0;
 	struct nvsp_message sendMessage;
 	struct net_device *ndev;
+	u64 req_id;
 
 	net_device = get_outbound_net_device(device);
 	if (!net_device)
@@ -518,20 +521,24 @@ int netvsc_send(struct hv_device *device,
 		0xFFFFFFFF;
 	sendMessage.msg.v1_msg.send_rndis_pkt.send_buf_section_size = 0;
 
+	if (packet->completion.send.send_completion)
+		req_id = (u64)packet;
+	else
+		req_id = 0;
+
 	if (packet->page_buf_cnt) {
 		ret = vmbus_sendpacket_pagebuffer(device->channel,
 						  packet->page_buf,
 						  packet->page_buf_cnt,
 						  &sendMessage,
 						  sizeof(struct nvsp_message),
-						  (unsigned long)packet);
+						  req_id);
 	} else {
 		ret = vmbus_sendpacket(device->channel, &sendMessage,
 				sizeof(struct nvsp_message),
-				(unsigned long)packet,
+				req_id,
 				VM_PKT_DATA_INBAND,
 				VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-
 	}
 
 	if (ret == 0) {
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 2b657d4..0775f0a 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -61,9 +61,6 @@ struct rndis_request {
 
 static void rndis_filter_send_completion(void *ctx);
 
-static void rndis_filter_send_request_completion(void *ctx);
-
-
 
 static struct rndis_device *get_rndis_device(void)
 {
@@ -241,10 +238,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
 			packet->page_buf[0].len;
 	}
 
-	packet->completion.send.send_completion_ctx = req;/* packet; */
-	packet->completion.send.send_completion =
-		rndis_filter_send_request_completion;
-	packet->completion.send.send_completion_tid = (unsigned long)dev;
+	packet->completion.send.send_completion = NULL;
 
 	ret = netvsc_send(dev->net_dev->dev, packet);
 	return ret;
@@ -999,9 +993,3 @@ static void rndis_filter_send_completion(void *ctx)
 	/* Pass it back to the original handler */
 	filter_pkt->completion(filter_pkt->completion_ctx);
 }
-
-
-static void rndis_filter_send_request_completion(void *ctx)
-{
-	/* Noop */
-}
-- 
1.7.4.1

Re: [PATCH net,2/2] hyperv: Fix RNDIS send_completion code path

[David Miller]

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Fri,  5 Apr 2013 14:44:40 -0700

> In some cases, the VM_PKT_COMP message can arrive later than RNDIS completion
> message, which will free the packet memory. This may cause panic due to access
> to freed memory in netvsc_send_completion().
> 
> This patch fixes this problem by removing rndis_filter_send_request_completion()
> from the code path. The function was a no-op.
> 
> Reported-by: Long Li <longli@microsoft.com>
> Tested-by: Long Li <longli@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

Applied.

Re: [PATCH net,1/2] hyperv: Fix a kernel warning from netvsc_linkstatus_callback()

[David Miller]

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Fri,  5 Apr 2013 14:44:39 -0700

> The warning about local_bh_enable inside IRQ happens when disconnecting a
> virtual NIC.
> 
> The reason for the warning is -- netif_tx_disable() is called when the NIC
> is disconnected. And it's called within irq context. netif_tx_disable() calls
> local_bh_enable() which displays warning if in irq.
> 
> The fix is to remove the unnecessary netif_tx_disable & wake_queue() in the
> netvsc_linkstatus_callback().
> 
> Reported-by: Richard Genoud <richard.genoud@gmail.com>
> Tested-by: Long Li <longli@microsoft.com>
> Tested-by: Richard Genoud <richard.genoud@gmail.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

Applied.

Re: [PATCH net, 1/2] hyperv: Fix a kernel warning from netvsc_linkstatus_callback()

[Richard Genoud]

2013/4/5 Haiyang Zhang <haiyangz@microsoft.com>:
> The warning about local_bh_enable inside IRQ happens when disconnecting a
> virtual NIC.
>
> The reason for the warning is -- netif_tx_disable() is called when the NIC
> is disconnected. And it's called within irq context. netif_tx_disable() calls
> local_bh_enable() which displays warning if in irq.
>
> The fix is to remove the unnecessary netif_tx_disable & wake_queue() in the
> netvsc_linkstatus_callback().
>
> Reported-by: Richard Genoud <richard.genoud@gmail.com>
> Tested-by: Long Li <longli@microsoft.com>
> Tested-by: Richard Genoud <richard.genoud@gmail.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
>
> ---
>  drivers/net/hyperv/netvsc_drv.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 5f85205..8341b62 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -241,13 +241,11 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
>
>         if (status == 1) {
>                 netif_carrier_on(net);
> -               netif_wake_queue(net);
>                 ndev_ctx = netdev_priv(net);
>                 schedule_delayed_work(&ndev_ctx->dwork, 0);
>                 schedule_delayed_work(&ndev_ctx->dwork, msecs_to_jiffies(20));
>         } else {
>                 netif_carrier_off(net);
> -               netif_tx_disable(net);
>         }
>  }
>
> --
> 1.7.4.1

This should also go to the stable tree, shouldn't it ?
At least 3.8, 3.7 and 3.4 (the 3.2 version is different, and it's in
staging. (at not used by distros anyway))


Richard.

Kernel v3.2 used in SlackWare 14.00 please, backport all needed patches Re: [PATCH net, 1/2] hyperv: Fix a kernel warning from netvsc_linkstatus_callback()

[Victor Miasnikov]

Hi!

> (the 3.2 version is different,
> and it's instaging. (at not used by distros anyway))

 No: used

Linux Kernel v3.2 used in:
-- SlackWare 14.00

This is actual stable version SlackWare

Please, backport all(!) needed patches

 I'm download https://www.kernel.org/pub/linux/kernel/v3.x/linux-3.2.45.tar.xz

File ata_piix.c ( SHA1 ceaf441142ad3f6768ff71f023e73d0695458e2c ) from
./linux-3.2.45/drivers/ata/
not contain:
( even not contain sub-string "Hyper-V")
== ( VVM: symbol TAB replaced by Space+Space -- special for users of MS IE, MS OutLook [Express] )
static int prefer_ms_hyperv = 1;
module_param(prefer_ms_hyperv, int, 0);
MODULE_PARM_DESC(prefer_ms_hyperv,
  "Prefer Hyper-V paravirtualization drivers instead of ATA, "
  "0 - Use ATA drivers, "
  "1 (Default) - Use the paravirtualization drivers.");

static void piix_ignore_devices_quirk(struct ata_host *host)
{
#if IS_ENABLED(CONFIG_HYPERV_STORAGE)
  static const struct dmi_system_id ignore_hyperv[] = {
    {
      /* On Hyper-V hypervisors the disks are exposed on
       * both the emulated SATA controller and on the
       * paravirtualised drivers.  The CD/DVD devices
       * are only exposed on the emulated controller.
       * Request we ignore ATA devices on this host.
       */
      .ident = "Hyper-V Virtual Machine",
      .matches = {
        DMI_MATCH(DMI_SYS_VENDOR,
            "Microsoft Corporation"),
        DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
      },
    },
    { }  /* terminate list */
  };
  static const struct dmi_system_id allow_virtual_pc[] = {
    {
      /* In MS Virtual PC guests the DMI ident is nearly
       * identical to a Hyper-V guest. One difference is the
       * product version which is used here to identify
       * a Virtual PC guest. This entry allows ata_piix to
       * drive the emulated hardware.
       */
      .ident = "MS Virtual PC 2007",
      .matches = {
        DMI_MATCH(DMI_SYS_VENDOR,
            "Microsoft Corporation"),
        DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
        DMI_MATCH(DMI_PRODUCT_VERSION, "VS2005R2"),
      },
    },
    { }  /* terminate list */
  };
  const struct dmi_system_id *ignore = dmi_first_match(ignore_hyperv);
  const struct dmi_system_id *allow = dmi_first_match(allow_virtual_pc);

  if (ignore && !allow && prefer_ms_hyperv) {
    host->flags |= ATA_HOST_IGNORE_ATA;
    dev_info(host->dev, "%s detected, ATA device ignore set\n",
      ignore->ident);
  }
#endif
==


_Absolutly_ ( as _minimum_ for SlackWare 14.00 ) need backport requested at 2012-06-08(!) :
( look on sub-string "3.2")
==
----- Original Message ----- 
From: "Victor Miasnikov"
To: "Greg KH" ; "Jonathan Nieder" ; "Andy Whitcroft"
Cc: <stable@vger.kernel.org>; <linux-kernel@vger.kernel.org>; "KY Srinivasan"; "Mike Sterling"
Sent: Friday, June 08, 2012 1:36 PM
Subject: Re: Re: ToDo: backport to v3.4 , v3.3 , v3.2 patches 1b) db63a4c8115a libata 1) cd006086fa5d ata_piix: defer
disks to the Hyper-V drivers by default Fw: use hv_storvsc instead of ata_piix for IDE disks ( but not for the CD-ROM)

 . . .

{
> > > Hyper-V admins need _worked_ Linux v3.4.X / v3.3.X / v3.2.X
> > > Please, _fix_ errors related "use hv_storvsc instead of ata_piix to
> > > handle the IDE disks devices ( but not for the CD-ROM)"

  i.e. need backport to all actual version after 3.1

  cd006086fa5d ata_piix: defer disks to the Hyper-V drivers by default

and its prerequisite

  db63a4c8115a libata: add a host flag to ignore detected ATA devices
}
==

and not forget patch related Bug 52821 :

{{
== ==
----- Original Message ----- 
From: vvm
To: Andreas
Sent: Wednesday, January 30, 2013 4:20 PM
Subject: FIXed: ef773e1..bec35f4 Re: ata_piix.prefer_ms_hyperv=0 works Fw: Bug 52821 - ata_piix ATA_HOST_IGNORE_ATA for
Hyper-V also affects Virtual PC 7 https://bugzilla.novell.com/show_bug.cgi?id=737532 Or
https://bugzilla.kernel.org/show_bug.cgi?id=52821


Hi!

 Short:

> Still, if there is a way to cleanly identify Hyper-V but not Virtual PC 7, this would be great!

FIXed: ef773e1..bec35f4

 Write in
https://bugzilla.kernel.org/show_bug.cgi?id=52821

"please, backport to 3.4 . . . 3.6 . . . 3.7"

   . . .

 Full:



(
 . . .

> Still, if there is a way to cleanly identify Hyper-V but not Virtual PC 7, this would be great!

FIXed: ef773e1..bec35f4


----- Original Message ----- 
From: "Jeff Garzik"
To: "Olaf Hering"
Cc:  . . . ; "KY Srinivasan"
Sent: Wednesday, November 28, 2012 8:44 PM
Subject: Re: [PATCH] ata_piix: reenable MS Virtual PC guests

On 09/18/2012 11:48 AM, Olaf Hering wrote:
An earlier commit cd006086fa5d91414d8ff9ff2b78fbb593878e3c ("ata_piix:
defer disks to the Hyper-V drivers by default") broke MS Virtual PC
guests. Hyper-V guests and Virtual PC guests have nearly identical DMI
info. As a result the driver does currently ignore the emulated hardware
in Virtual PC guests and defers the handling to hv_blkvsc [ VVM: in current ver kernel -- hv_storvsc ]  . Since Virtual
PC does not offer paravirtualized drivers no disks will be found in the
guest.

One difference in the DMI info is the product version. This patch adds a
match for MS Virtual PC 2007 and "unignores" the emulated hardware.

This was reported for openSuSE 12.1 in bugzilla:
https://bugzilla.novell.com/show_bug.cgi?id=737532
.  .  .

Signed-off-by: Olaf Hering

applied.  Apologies for missing this one.  It was accidentally shifting
into the low-priority pile.
==


== ==


==
Andreas 2013-01-30 22:19:21
  First appearance:
https://bugzilla.novell.com/show_bug.cgi?id=737532
Patch:
https://lkml.org/lkml/2012/9/18/306

==

}}




> This should also go to the stable tree, shouldn't it ?
> At least 3.8, 3.7 and 3.4

 In kernel 3.2 used in

-- Ubuntu 12.4.0 LTS (  after 12.4.2 -- 3.5.X)
and
-- Debian 7.0

source code related Hyper-V backported from "vanila" Kernel v3.5 ( and someone from hi versions)

Please, ask Support Teams of Debian and Ubuntu for backport



Best regards, Victor Miasnikov
Blog:  http://vvm.blog.tut.by/


P.S.

to To Microsoft Hyper-V for Linux Team:

what are mint phrase:
"LIS (Linux Integration Services) 4.0"
?

If need details -- ask me by e-mail . . .



P.P.S.

Again to To Microsoft Hyper-V for Linux Team:

 See on MS Forum topic
"Linux VM literally crashes Hyper-V host server with STOP 0x00020001"
 and (or)
please, remember, CVE number



P.P.P.S.

To SUSE Hyper-V for Linux Team:

Olaf, I'm received Yours e-mail msg ( answer on my earler msg)

 Thanks for answer

Unfortunately Yours ( SUSE/Novel) Bug-Tracker not opened by IE from Win Srv Edtions
I'm try do it from OpenSUSE v12.3 LiveCD but can not boot from it in Hyper-V VM when "Dynamic memory" turn on

 See details on
http://vvm.blog.tut.by/2013/05/18/opensuse-on-hyper-v/

 Or ask SUSE Support Team write me by e-mail for detail


P.P.P.P.S.

 In
./linux-3.10-rc2/drivers/scsi/storvsc_drv.c

I am not found sub-string "Fiber Channel"

  As I  undestand Linux Kernel roadmap/timeline traditions
need ask by backport needed patches to v3.10

To Microsoft Hyper-V for Linux Team:

 Who must do it?

Again Hypev-V sysadmin[s] ?


P.P.P.P.P.S.

Sorry, if my msg not full "political correct"

I preffered solve problem as possibily earler, what wait big problems

Sorry, again . . .

Re: Kernel v3.2 used in SlackWare 14.00 please, backport all needed patches Re: [PATCH net, 1/2] hyperv: Fix a kernel warning from netvsc_linkstatus_callback()

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]

On Thu, 2013-05-23 at 16:14 +0300, Victor Miasnikov wrote:
> Hi!
> 
> > (the 3.2 version is different,
> > and it's instaging. (at not used by distros anyway))
> 
>  No: used
> 
> Linux Kernel v3.2 used in:
> -- SlackWare 14.00
> 
> This is actual stable version SlackWare
> 
> Please, backport all(!) needed patches
[...]
> P.P.P.P.P.S.
> 
> Sorry, if my msg not full "political correct"
> 
> I preffered solve problem as possibily earler, what wait big problems
> 
> Sorry, again . . .

Please read Documentation/stable_kernel_rules.txt.  In short, you need
to identify the commit hash for each of the changes you want, and
provide the backported patch for any commits that can't simply be
cherry-picked.  I'm not going to read through all of what you wrote to
work out exactly what's needed.

I think Slackware might do better to backport the Hyper-V drivers from
Linux 3.4, as Debian and Ubuntu have done with 3.2-based kernels.  But
that sort of large backport isn't allowed on kernel.org stable branches.

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

RE: [PATCH net, 1/2] hyperv: Fix a kernel warning from netvsc_linkstatus_callback()

[Haiyang Zhang]

> -----Original Message-----
> From: Richard Genoud [mailto:richard.genoud@gmail.com]
> Sent: Thursday, May 23, 2013 4:44 AM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; olaf@aepfle.de;
> jasowang@redhat.com; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org
> Subject: Re: [PATCH net, 1/2] hyperv: Fix a kernel warning from
> netvsc_linkstatus_callback()
> 
> 2013/4/5 Haiyang Zhang <haiyangz@microsoft.com>:
> > The warning about local_bh_enable inside IRQ happens when
> > disconnecting a virtual NIC.
> >
> > The reason for the warning is -- netif_tx_disable() is called when the
> > NIC is disconnected. And it's called within irq context.
> > netif_tx_disable() calls
> > local_bh_enable() which displays warning if in irq.
> >
> > The fix is to remove the unnecessary netif_tx_disable & wake_queue()
> > in the netvsc_linkstatus_callback().
> >
> > Reported-by: Richard Genoud <richard.genoud@gmail.com>
> > Tested-by: Long Li <longli@microsoft.com>
> > Tested-by: Richard Genoud <richard.genoud@gmail.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
> >
> > ---
> >  drivers/net/hyperv/netvsc_drv.c |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c index 5f85205..8341b62 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -241,13 +241,11 @@ void netvsc_linkstatus_callback(struct hv_device
> > *device_obj,
> >
> >         if (status == 1) {
> >                 netif_carrier_on(net);
> > -               netif_wake_queue(net);
> >                 ndev_ctx = netdev_priv(net);
> >                 schedule_delayed_work(&ndev_ctx->dwork, 0);
> >                 schedule_delayed_work(&ndev_ctx->dwork,
> msecs_to_jiffies(20));
> >         } else {
> >                 netif_carrier_off(net);
> > -               netif_tx_disable(net);
> >         }
> >  }
> >
> > --
> > 1.7.4.1
> 
> This should also go to the stable tree, shouldn't it ?
> At least 3.8, 3.7 and 3.4 (the 3.2 version is different, and it's in staging. (at not
> used by distros anyway))

Cc: stable <stable@kernel.org>

I agree. Adding stable@kernel.org to Cc list.

Thanks,
- Haiyang