[PATCH V1 net 0/2] net: ena: race condition bug fix and version update

[PATCH V1 net 0/2] net: ena: race condition bug fix and version update

[akiyano]

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

[PATCH V1 net 1/2] net: ena: fix race between link up and device initalization

[akiyano]

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

[PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3

[akiyano]

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

Re: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3

[Moritz Fischer]

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

RE: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3

[Kiyanovski, Arthur]

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

Re: [PATCH V1 net 2/2] net: ena: update driver version from 2.0.2 to 2.0.3

[Andrew Lunn]

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

Re: [PATCH V1 net 0/2] net: ena: race condition bug fix and version update

[David Miller]

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.

Re: [PATCH V1 net 0/2] net: ena: race condition bug fix and version update

[Bshara, Saeed]

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.
    

Re: [PATCH V1 net 0/2] net: ena: race condition bug fix and version update

[David Miller]

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.