Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH V1 net 0/2] net: ena: race condition bug fix and version update 
@ 2019-02-11 17:17 akiyano
  2019-02-11 17:17 ` [PATCH V1 net 1/2] net: ena: fix race between link up and device initalization akiyano
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: akiyano @ 2019-02-11 17:17 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi

From: Arthur Kiyanovski <akiyano@amazon.com>

This patchset includes a fix to a race condition that can cause
kernel panic, as well as a driver version update because of this
fix.

Arthur Kiyanovski (2):
  net: ena: fix race between link up and device initalization
  net: ena: update driver version from 2.0.2 to 2.0.3

 drivers/net/ethernet/amazon/ena/ena_netdev.c | 10 +++++-----
 drivers/net/ethernet/amazon/ena/ena_netdev.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V1 net 1/2] net: ena: fix race between link up and device initalization
  2019-02-11 17:17 [PATCH V1 net 0/2] net: ena: race condition bug fix and version update akiyano
@ 2019-02-11 17:17 ` akiyano
  2019-02-11 17:17 ` [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3 akiyano
  2019-02-12 19:06 ` [PATCH V1 net 0/2] net: ena: race condition bug fix and version update David Miller
  2 siblings, 0 replies; 9+ messages in thread
From: akiyano @ 2019-02-11 17:17 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi

From: Arthur Kiyanovski <akiyano@amazon.com>

Fix race condition between ena_update_on_link_change() and
ena_restore_device().

This race can occur if link notification arrives while the driver
is performing a reset sequence. In this case link can be set up,
enabling the device, before it is fully restored. If packets are
sent at this time, the driver might access uninitialized data
structures, causing kernel crash.

Move the clearing of ENA_FLAG_ONGOING_RESET and netif_carrier_on()
after ena_up() to ensure the device is ready when link is set up.

Fixes: d18e4f683445 ("net: ena: fix race condition between device reset and link up setup")
Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index a70bb1bb90e7..a6eacf2099c3 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -2663,11 +2663,6 @@ static int ena_restore_device(struct ena_adapter *adapter)
 		goto err_device_destroy;
 	}
 
-	clear_bit(ENA_FLAG_ONGOING_RESET, &adapter->flags);
-	/* Make sure we don't have a race with AENQ Links state handler */
-	if (test_bit(ENA_FLAG_LINK_UP, &adapter->flags))
-		netif_carrier_on(adapter->netdev);
-
 	rc = ena_enable_msix_and_set_admin_interrupts(adapter,
 						      adapter->num_queues);
 	if (rc) {
@@ -2684,6 +2679,11 @@ static int ena_restore_device(struct ena_adapter *adapter)
 	}
 
 	set_bit(ENA_FLAG_DEVICE_RUNNING, &adapter->flags);
+
+	clear_bit(ENA_FLAG_ONGOING_RESET, &adapter->flags);
+	if (test_bit(ENA_FLAG_LINK_UP, &adapter->flags))
+		netif_carrier_on(adapter->netdev);
+
 	mod_timer(&adapter->timer_service, round_jiffies(jiffies + HZ));
 	dev_err(&pdev->dev,
 		"Device reset completed successfully, Driver info: %s\n",
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3
  2019-02-11 17:17 [PATCH V1 net 0/2] net: ena: race condition bug fix and version update akiyano
  2019-02-11 17:17 ` [PATCH V1 net 1/2] net: ena: fix race between link up and device initalization akiyano
@ 2019-02-11 17:17 ` akiyano
  2019-02-12 11:12   ` Moritz Fischer
  2019-02-12 19:06 ` [PATCH V1 net 0/2] net: ena: race condition bug fix and version update David Miller
  2 siblings, 1 reply; 9+ messages in thread
From: akiyano @ 2019-02-11 17:17 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi

From: Arthur Kiyanovski <akiyano@amazon.com>

Update driver version due to bug fix.

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index dc8b6173d8d8..63870072cbbd 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -45,7 +45,7 @@
 
 #define DRV_MODULE_VER_MAJOR	2
 #define DRV_MODULE_VER_MINOR	0
-#define DRV_MODULE_VER_SUBMINOR 2
+#define DRV_MODULE_VER_SUBMINOR 3
 
 #define DRV_MODULE_NAME		"ena"
 #ifndef DRV_MODULE_VERSION
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3
  2019-02-11 17:17 ` [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3 akiyano
@ 2019-02-12 11:12   ` Moritz Fischer
  2019-02-12 11:42     ` Kiyanovski, Arthur
  0 siblings, 1 reply; 9+ messages in thread
From: Moritz Fischer @ 2019-02-12 11:12 UTC (permalink / raw)
  To: akiyano
  Cc: David S. Miller, netdev, dwmw, zorik, matua, saeedb, msw,
	aliguori, nafea, gtzalik, netanel, alisaidi

Hi Arthur,

On Mon, Feb 11, 2019 at 9:19 AM <akiyano@amazon.com> wrote:
>
> From: Arthur Kiyanovski <akiyano@amazon.com>
>
> Update driver version due to bug fix.

Wouldn't you want to do this atomically with the actual fix in one commit?

Thanks,
Moritz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3
  2019-02-12 11:12   ` Moritz Fischer
@ 2019-02-12 11:42     ` Kiyanovski, Arthur
  2019-02-12 13:11       ` Andrew Lunn
  0 siblings, 1 reply; 9+ messages in thread
From: Kiyanovski, Arthur @ 2019-02-12 11:42 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: David S. Miller, netdev, Woodhouse, David, Machulsky, Zorik,
	Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt, Liguori,
	Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel, Saidi,
	Ali

Hi Moritz,

> -----Original Message-----
> From: Moritz Fischer <moritz.fischer@ettus.com>
> Subject: Re: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to
> 2.0.3
> 
> Hi Arthur,
> 
> On Mon, Feb 11, 2019 at 9:19 AM <akiyano@amazon.com> wrote:
> >
> > From: Arthur Kiyanovski <akiyano@amazon.com>
> >
> > Update driver version due to bug fix.
> 
> Wouldn't you want to do this atomically with the actual fix in one commit?
> 
> Thanks,
> Moritz

Thanks for the feedback.
I don't want to add the version to the fix.
Version 2.0.3 includes all of its content that came before this fix, including features.
Fixes are cherry-picked to older driver versions automatically using the "Fixes:" directive.
If I add the version change to the fix, older driver versions that get this fix will become version 2.0.3, which is incorrect feature-wise.

Thanks,
Arthur

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3
  2019-02-12 11:42     ` Kiyanovski, Arthur
@ 2019-02-12 13:11       ` Andrew Lunn
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-02-12 13:11 UTC (permalink / raw)
  To: Kiyanovski, Arthur
  Cc: Moritz Fischer, David S. Miller, netdev, Woodhouse, David,
	Machulsky, Zorik, Matushevsky, Alexander, Bshara, Saeed, Wilson,
	Matt, Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal,
	Netanel, Saidi, Ali

On Tue, Feb 12, 2019 at 11:42:31AM +0000, Kiyanovski, Arthur wrote:
> Hi Moritz,
> 
> > -----Original Message-----
> > From: Moritz Fischer <moritz.fischer@ettus.com>
> > Subject: Re: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to
> > 2.0.3
> > 
> > Hi Arthur,
> > 
> > On Mon, Feb 11, 2019 at 9:19 AM <akiyano@amazon.com> wrote:
> > >
> > > From: Arthur Kiyanovski <akiyano@amazon.com>
> > >
> > > Update driver version due to bug fix.
> > 
> > Wouldn't you want to do this atomically with the actual fix in one commit?
> > 
> > Thanks,
> > Moritz
> 
> Thanks for the feedback.
> I don't want to add the version to the fix.
> Version 2.0.3 includes all of its content that came before this fix, including features.
> Fixes are cherry-picked to older driver versions automatically using the "Fixes:" directive.
> If I add the version change to the fix, older driver versions that get this fix will become version 2.0.3, which is incorrect feature-wise.

Hi Arthur

I think this points out how useless the version string it, and why
many drivers don't use it.

     Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 net 0/2] net: ena: race condition bug fix and version update 
  2019-02-11 17:17 [PATCH V1 net 0/2] net: ena: race condition bug fix and version update akiyano
  2019-02-11 17:17 ` [PATCH V1 net 1/2] net: ena: fix race between link up and device initalization akiyano
  2019-02-11 17:17 ` [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3 akiyano
@ 2019-02-12 19:06 ` David Miller
  2019-03-13 12:59   ` Bshara, Saeed
  2 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2019-02-12 19:06 UTC (permalink / raw)
  To: akiyano
  Cc: netdev, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
	gtzalik, netanel, alisaidi

From: <akiyano@amazon.com>
Date: Mon, 11 Feb 2019 19:17:42 +0200

> From: Arthur Kiyanovski <akiyano@amazon.com>
> 
> This patchset includes a fix to a race condition that can cause
> kernel panic, as well as a driver version update because of this
> fix.

Series applied and patch #1 queued up for -stable.

But I want to reiterate what Andrew said, the version is so increibly
useless and stupid.

I'm going to submit the fix to -stable, and then people will then
doubly and triply have no relationship between driver version number
and what fixes exist.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 net 0/2] net: ena: race condition bug fix and version update 
  2019-02-12 19:06 ` [PATCH V1 net 0/2] net: ena: race condition bug fix and version update David Miller
@ 2019-03-13 12:59   ` Bshara, Saeed
  2019-03-13 17:52     ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Bshara, Saeed @ 2019-03-13 12:59 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Woodhouse, David, Machulsky, Zorik, Matushevsky,
	Alexander, Wilson, Matt, Liguori, Anthony, Bshara, Nafea, Tzalik,
	Guy, Belgazal, Netanel, Saidi, Ali, Kiyanovski, Arthur



David, 
sorry for the late response. I agree that with stable versions the driver version is not good indication for which exact code is running.
however, we have many users that port the latest ena driver as-is from kernel sources into their older kernels, mainly to pull new ENA features (e.g. low latency queues). for those cases, the driver version helps us to identify the driver's code, so in practice this just works fine.

saeed


From: David Miller <davem@davemloft.net>
Sent: Tuesday, February 12, 2019 9:06 PM
To: Kiyanovski, Arthur
Cc: netdev@vger.kernel.org; Woodhouse, David; Machulsky, Zorik; Matushevsky, Alexander; Bshara, Saeed; Wilson, Matt; Liguori, Anthony; Bshara, Nafea; Tzalik, Guy; Belgazal, Netanel; Saidi, Ali
Subject: Re: [PATCH V1 net 0/2] net: ena: race condition bug fix and version update 
    
From: <akiyano@amazon.com>
Date: Mon, 11 Feb 2019 19:17:42 +0200

> From: Arthur Kiyanovski <akiyano@amazon.com>
> 
> This patchset includes a fix to a race condition that can cause
> kernel panic, as well as a driver version update because of this
> fix.

Series applied and patch #1 queued up for -stable.

But I want to reiterate what Andrew said, the version is so increibly
useless and stupid.

I'm going to submit the fix to -stable, and then people will then
doubly and triply have no relationship between driver version number
and what fixes exist.
    

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 net 0/2] net: ena: race condition bug fix and version update 
  2019-03-13 12:59   ` Bshara, Saeed
@ 2019-03-13 17:52     ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2019-03-13 17:52 UTC (permalink / raw)
  To: saeedb
  Cc: netdev, dwmw, zorik, matua, msw, aliguori, nafea, gtzalik,
	netanel, alisaidi, akiyano

From: "Bshara, Saeed" <saeedb@amazon.com>
Date: Wed, 13 Mar 2019 12:59:34 +0000

> sorry for the late response. I agree that with stable versions the
> driver version is not good indication for which exact code is
> running.  however, we have many users that port the latest ena
> driver as-is from kernel sources into their older kernels, mainly to
> pull new ENA features (e.g. low latency queues). for those cases,
> the driver version helps us to identify the driver's code, so in
> practice this just works fine.

I will try one more time.

If I push fix X for the ENA driver to -stable and that bumps the version
number, all of the features associated with that version number are not
present in the -stable backport.

It does NOT work.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 17:17 [PATCH V1 net 0/2] net: ena: race condition bug fix and version update akiyano
2019-02-11 17:17 ` [PATCH V1 net 1/2] net: ena: fix race between link up and device initalization akiyano
2019-02-11 17:17 ` [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3 akiyano
2019-02-12 11:12   ` Moritz Fischer
2019-02-12 11:42     ` Kiyanovski, Arthur
2019-02-12 13:11       ` Andrew Lunn
2019-02-12 19:06 ` [PATCH V1 net 0/2] net: ena: race condition bug fix and version update David Miller
2019-03-13 12:59   ` Bshara, Saeed
2019-03-13 17:52     ` David Miller

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox