[RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[akiyano]

From: Arthur Kiyanovski <akiyano@amazon.com>

In this commit we revert the part of
commit 1a63443afd70 ("net/amazon: Ensure that driver version is aligned to the linux kernel"),
which breaks the interface between the ENA driver and FW.

We also replace the use of DRIVER_VERSION with DRIVER_GENERATION
when we bring back the deleted constants that are used in interface with
ENA device FW.

This commit does not change the driver version reported to the user via
ethtool, which remains the kernel version.

Fixes: 1a63443afd70 ("net/amazon: Ensure that driver version is aligned to the linux kernel")
Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c |  6 +++++-
 drivers/net/ethernet/amazon/ena/ena_netdev.h | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 4faf81c456d8..555c7273d712 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -3090,7 +3090,11 @@ static void ena_config_host_info(struct ena_com_dev *ena_dev,
 	host_info->os_dist = 0;
 	strncpy(host_info->os_dist_str, utsname()->release,
 		sizeof(host_info->os_dist_str) - 1);
-	host_info->driver_version = LINUX_VERSION_CODE;
+	host_info->driver_version =
+		(DRV_MODULE_GEN_MAJOR) |
+		(DRV_MODULE_GEN_MINOR << ENA_ADMIN_HOST_INFO_MINOR_SHIFT) |
+		(DRV_MODULE_GEN_SUBMINOR << ENA_ADMIN_HOST_INFO_SUB_MINOR_SHIFT) |
+		("K"[0] << ENA_ADMIN_HOST_INFO_MODULE_TYPE_SHIFT);
 	host_info->num_cpus = num_online_cpus();
 
 	host_info->driver_supported_features =
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index 74c7f10b60dd..97dfd0c67e84 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -45,7 +45,18 @@
 #include "ena_com.h"
 #include "ena_eth_com.h"
 
+#define DRV_MODULE_GEN_MAJOR	2
+#define DRV_MODULE_GEN_MINOR	1
+#define DRV_MODULE_GEN_SUBMINOR 0
+
 #define DRV_MODULE_NAME		"ena"
+#ifndef DRV_MODULE_GENERATION
+#define DRV_MODULE_GENERATION \
+	__stringify(DRV_MODULE_GEN_MAJOR) "."	\
+	__stringify(DRV_MODULE_GEN_MINOR) "."	\
+	__stringify(DRV_MODULE_GEN_SUBMINOR) "K"
+#endif
+
 #define DEVICE_NAME	"Elastic Network Adapter (ENA)"
 
 /* 1 for AENQ + ADMIN */
-- 
2.23.0

Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[David Miller]

From: <akiyano@amazon.com>
Date: Wed, 26 Feb 2020 12:03:35 +0200

> From: Arthur Kiyanovski <akiyano@amazon.com>
> 
> In this commit we revert the part of
> commit 1a63443afd70 ("net/amazon: Ensure that driver version is aligned to the linux kernel"),
> which breaks the interface between the ENA driver and FW.
> 
> We also replace the use of DRIVER_VERSION with DRIVER_GENERATION
> when we bring back the deleted constants that are used in interface with
> ENA device FW.
> 
> This commit does not change the driver version reported to the user via
> ethtool, which remains the kernel version.
> 
> Fixes: 1a63443afd70 ("net/amazon: Ensure that driver version is aligned to the linux kernel")
> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>

Applied.

Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[Leon Romanovsky]

On Wed, Feb 26, 2020 at 08:48:09PM -0800, David Miller wrote:
> From: <akiyano@amazon.com>
> Date: Wed, 26 Feb 2020 12:03:35 +0200
>
> > From: Arthur Kiyanovski <akiyano@amazon.com>
> >
> > In this commit we revert the part of
> > commit 1a63443afd70 ("net/amazon: Ensure that driver version is aligned to the linux kernel"),
> > which breaks the interface between the ENA driver and FW.
> >
> > We also replace the use of DRIVER_VERSION with DRIVER_GENERATION
> > when we bring back the deleted constants that are used in interface with
> > ENA device FW.
> >
> > This commit does not change the driver version reported to the user via
> > ethtool, which remains the kernel version.
> >
> > Fixes: 1a63443afd70 ("net/amazon: Ensure that driver version is aligned to the linux kernel")
> > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
>
> Applied.

Dave,

I see that I'm late here and my email sounds like old man grumbling,
but I asked from those guys to update their commit with request
to put the following line:
"/* DO NOT CHANGE DRV_MODULE_GEN_* values in in-tree code */"
https://lore.kernel.org/netdev/20200224162649.GA4526@unreal/

I also asked how those versions are transferred to FW and used there,
but was ignored.
https://lore.kernel.org/netdev/20200224094116.GD422704@unreal/

BTW, It is also unclear why I wasn't CCed in this patch.

Thanks

RE: [RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[Kiyanovski, Arthur]

> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Sunday, March 1, 2020 3:50 PM
> To: David Miller <davem@davemloft.net>
> Cc: Kiyanovski, Arthur <akiyano@amazon.com>; netdev@vger.kernel.org;
> Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik
> <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>;
> Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>;
> Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea
> <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, Netanel
> <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; Herrenschmidt,
> Benjamin <benh@amazon.com>; Dagan, Noam <ndagan@amazon.com>;
> Agroskin, Shay <shayagr@amazon.com>; Jubran, Samih
> <sameehj@amazon.com>
> Subject: Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between
> ENA driver and FW
> 
> On Wed, Feb 26, 2020 at 08:48:09PM -0800, David Miller wrote:
> > From: <akiyano@amazon.com>
> > Date: Wed, 26 Feb 2020 12:03:35 +0200
> >
> > > From: Arthur Kiyanovski <akiyano@amazon.com>
> > >
> > > In this commit we revert the part of commit 1a63443afd70
> > > ("net/amazon: Ensure that driver version is aligned to the linux
> > > kernel"), which breaks the interface between the ENA driver and FW.
> > >
> > > We also replace the use of DRIVER_VERSION with DRIVER_GENERATION
> > > when we bring back the deleted constants that are used in interface
> > > with ENA device FW.
> > >
> > > This commit does not change the driver version reported to the user
> > > via ethtool, which remains the kernel version.
> > >
> > > Fixes: 1a63443afd70 ("net/amazon: Ensure that driver version is
> > > aligned to the linux kernel")
> > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
> >
> > Applied.
> 
> Dave,
> 
> I see that I'm late here and my email sounds like old man grumbling, but I asked
> from those guys to update their commit with request to put the following line:
> "/* DO NOT CHANGE DRV_MODULE_GEN_* values in in-tree code */"
> https://lore.kernel.org/netdev/20200224162649.GA4526@unreal/
> 
> I also asked how those versions are transferred to FW and used there, but was
> ignored.
> https://lore.kernel.org/netdev/20200224094116.GD422704@unreal/
> 
> BTW, It is also unclear why I wasn't CCed in this patch.
> 
> Thanks

Leon,
 Sorry for not responding earlier to your inquiries, they are exactly touching the
 points that we would like to discuss.
 Up until now, we in AWS, have been monitoring the drivers in the datacenter using the
 driver version, and actively suggesting driver updates to our customers
 whenever a critical bug was fixed, or a new important feature was added.
 Removing the driver version and not allowing to maintain our own internal
 version negatively impacts our effort to give our customers the best possible cloud
 experience. We therefore prefer to maintain using our internal driver version.

 Are there any other recommended ways to achieve our goal without a driver
 version?
 
 Thanks!

Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[Leon Romanovsky]

On Thu, Mar 05, 2020 at 02:28:33PM +0000, Kiyanovski, Arthur wrote:
> > -----Original Message-----
> > From: Leon Romanovsky <leon@kernel.org>
> > Sent: Sunday, March 1, 2020 3:50 PM
> > To: David Miller <davem@davemloft.net>
> > Cc: Kiyanovski, Arthur <akiyano@amazon.com>; netdev@vger.kernel.org;
> > Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik
> > <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>;
> > Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>;
> > Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea
> > <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, Netanel
> > <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; Herrenschmidt,
> > Benjamin <benh@amazon.com>; Dagan, Noam <ndagan@amazon.com>;
> > Agroskin, Shay <shayagr@amazon.com>; Jubran, Samih
> > <sameehj@amazon.com>
> > Subject: Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between
> > ENA driver and FW
> >
> > On Wed, Feb 26, 2020 at 08:48:09PM -0800, David Miller wrote:
> > > From: <akiyano@amazon.com>
> > > Date: Wed, 26 Feb 2020 12:03:35 +0200
> > >
> > > > From: Arthur Kiyanovski <akiyano@amazon.com>
> > > >
> > > > In this commit we revert the part of commit 1a63443afd70
> > > > ("net/amazon: Ensure that driver version is aligned to the linux
> > > > kernel"), which breaks the interface between the ENA driver and FW.
> > > >
> > > > We also replace the use of DRIVER_VERSION with DRIVER_GENERATION
> > > > when we bring back the deleted constants that are used in interface
> > > > with ENA device FW.
> > > >
> > > > This commit does not change the driver version reported to the user
> > > > via ethtool, which remains the kernel version.
> > > >
> > > > Fixes: 1a63443afd70 ("net/amazon: Ensure that driver version is
> > > > aligned to the linux kernel")
> > > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
> > >
> > > Applied.
> >
> > Dave,
> >
> > I see that I'm late here and my email sounds like old man grumbling, but I asked
> > from those guys to update their commit with request to put the following line:
> > "/* DO NOT CHANGE DRV_MODULE_GEN_* values in in-tree code */"
> > https://lore.kernel.org/netdev/20200224162649.GA4526@unreal/
> >
> > I also asked how those versions are transferred to FW and used there, but was
> > ignored.
> > https://lore.kernel.org/netdev/20200224094116.GD422704@unreal/
> >
> > BTW, It is also unclear why I wasn't CCed in this patch.
> >
> > Thanks
>
> Leon,
>  Sorry for not responding earlier to your inquiries, they are exactly touching the
>  points that we would like to discuss.
>  Up until now, we in AWS, have been monitoring the drivers in the datacenter using the
>  driver version, and actively suggesting driver updates to our customers
>  whenever a critical bug was fixed, or a new important feature was added.
>  Removing the driver version and not allowing to maintain our own internal
>  version negatively impacts our effort to give our customers the best possible cloud
>  experience. We therefore prefer to maintain using our internal driver version.
>
>  Are there any other recommended ways to achieve our goal without a driver
>  version?

Of course, drivers are supposed to behave like any other user visible API.
They need to ensure backward compatibility, so new code will work with
old HW/FW. This is normally done by capability masks, see how it is done
in Mellanox drivers and I think in Intel too.

So your update policy based on driver version string is nonsense and
broken by design.

Original thread with Linus is here [1].

[1] https://lore.kernel.org/ksummit-discuss/CA+55aFx9A=5cc0QZ7CySC4F2K7eYaEfzkdYEc9JaNgCcV25=rg@mail.gmail.com/

Thanks

>
>  Thanks!
>

Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[Machulsky, Zorik]

On 3/5/20, 11:17 AM, "Leon Romanovsky" <leon@kernel.org> wrote:

    
    On Thu, Mar 05, 2020 at 02:28:33PM +0000, Kiyanovski, Arthur wrote:
    > > -----Original Message-----
    > > From: Leon Romanovsky <leon@kernel.org>
    > > Sent: Sunday, March 1, 2020 3:50 PM
    > > To: David Miller <davem@davemloft.net>
    > > Cc: Kiyanovski, Arthur <akiyano@amazon.com>; netdev@vger.kernel.org;
    > > Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik
    > > <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>;
    > > Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>;
    > > Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea
    > > <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, Netanel
    > > <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; Herrenschmidt,
    > > Benjamin <benh@amazon.com>; Dagan, Noam <ndagan@amazon.com>;
    > > Agroskin, Shay <shayagr@amazon.com>; Jubran, Samih
    > > <sameehj@amazon.com>
    > > Subject: Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between
    > > ENA driver and FW
    > >
    > > On Wed, Feb 26, 2020 at 08:48:09PM -0800, David Miller wrote:
    > > > From: <akiyano@amazon.com>
    > > > Date: Wed, 26 Feb 2020 12:03:35 +0200
    > > >
    > > > > From: Arthur Kiyanovski <akiyano@amazon.com>
    > > > >
    > > > > In this commit we revert the part of commit 1a63443afd70
    > > > > ("net/amazon: Ensure that driver version is aligned to the linux
    > > > > kernel"), which breaks the interface between the ENA driver and FW.
    > > > >
    > > > > We also replace the use of DRIVER_VERSION with DRIVER_GENERATION
    > > > > when we bring back the deleted constants that are used in interface
    > > > > with ENA device FW.
    > > > >
    > > > > This commit does not change the driver version reported to the user
    > > > > via ethtool, which remains the kernel version.
    > > > >
    > > > > Fixes: 1a63443afd70 ("net/amazon: Ensure that driver version is
    > > > > aligned to the linux kernel")
    > > > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
    > > >
    > > > Applied.
    > >
    > > Dave,
    > >
    > > I see that I'm late here and my email sounds like old man grumbling, but I asked
    > > from those guys to update their commit with request to put the following line:
    > > "/* DO NOT CHANGE DRV_MODULE_GEN_* values in in-tree code */"
    > > https://lore.kernel.org/netdev/20200224162649.GA4526@unreal/
    > >
    > > I also asked how those versions are transferred to FW and used there, but was
    > > ignored.
    > > https://lore.kernel.org/netdev/20200224094116.GD422704@unreal/
    > >
    > > BTW, It is also unclear why I wasn't CCed in this patch.
    > >
    > > Thanks
    >
    > Leon,
    >  Sorry for not responding earlier to your inquiries, they are exactly touching the
    >  points that we would like to discuss.
    >  Up until now, we in AWS, have been monitoring the drivers in the datacenter using the
    >  driver version, and actively suggesting driver updates to our customers
    >  whenever a critical bug was fixed, or a new important feature was added.
    >  Removing the driver version and not allowing to maintain our own internal
    >  version negatively impacts our effort to give our customers the best possible cloud
    >  experience. We therefore prefer to maintain using our internal driver version.
    >
    >  Are there any other recommended ways to achieve our goal without a driver
    >  version?
    
    Of course, drivers are supposed to behave like any other user visible API.
    They need to ensure backward compatibility, so new code will work with
    old HW/FW. This is normally done by capability masks, see how it is done
    in Mellanox drivers and I think in Intel too.
    
    So your update policy based on driver version string is nonsense and
    broken by design.
    
    Original thread with Linus is here [1].
    
    [1] https://lore.kernel.org/ksummit-discuss/CA+55aFx9A=5cc0QZ7CySC4F2K7eYaEfzkdYEc9JaNgCcV25=rg@mail.gmail.com/
    
    Thanks 

We do support features capability mask as well as versioning per feature. 
However, whenever there are known issues in a certain version of the  driver
that can be worked around by the device, we need the device to be aware of the 
driver version. Another purpose is operational - knowing driver version helps us 
reproduce customer issues and debug them, as well as suggest our customers
to upgrade their drivers, as Arthur mentioned above.
Thanks

    
    >
    >  Thanks!
    >
    

Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between ENA driver and FW

[Leon Romanovsky]

On Thu, Mar 05, 2020 at 08:37:43PM +0000, Machulsky, Zorik wrote:
>
>
> On 3/5/20, 11:17 AM, "Leon Romanovsky" <leon@kernel.org> wrote:
>
>
>     On Thu, Mar 05, 2020 at 02:28:33PM +0000, Kiyanovski, Arthur wrote:
>     > > -----Original Message-----
>     > > From: Leon Romanovsky <leon@kernel.org>
>     > > Sent: Sunday, March 1, 2020 3:50 PM
>     > > To: David Miller <davem@davemloft.net>
>     > > Cc: Kiyanovski, Arthur <akiyano@amazon.com>; netdev@vger.kernel.org;
>     > > Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik
>     > > <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>;
>     > > Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>;
>     > > Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea
>     > > <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, Netanel
>     > > <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; Herrenschmidt,
>     > > Benjamin <benh@amazon.com>; Dagan, Noam <ndagan@amazon.com>;
>     > > Agroskin, Shay <shayagr@amazon.com>; Jubran, Samih
>     > > <sameehj@amazon.com>
>     > > Subject: Re: [RESEND PATCH V1 net-next] net: ena: fix broken interface between
>     > > ENA driver and FW
>     > >
>     > > On Wed, Feb 26, 2020 at 08:48:09PM -0800, David Miller wrote:
>     > > > From: <akiyano@amazon.com>
>     > > > Date: Wed, 26 Feb 2020 12:03:35 +0200
>     > > >
>     > > > > From: Arthur Kiyanovski <akiyano@amazon.com>
>     > > > >
>     > > > > In this commit we revert the part of commit 1a63443afd70
>     > > > > ("net/amazon: Ensure that driver version is aligned to the linux
>     > > > > kernel"), which breaks the interface between the ENA driver and FW.
>     > > > >
>     > > > > We also replace the use of DRIVER_VERSION with DRIVER_GENERATION
>     > > > > when we bring back the deleted constants that are used in interface
>     > > > > with ENA device FW.
>     > > > >
>     > > > > This commit does not change the driver version reported to the user
>     > > > > via ethtool, which remains the kernel version.
>     > > > >
>     > > > > Fixes: 1a63443afd70 ("net/amazon: Ensure that driver version is
>     > > > > aligned to the linux kernel")
>     > > > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
>     > > >
>     > > > Applied.
>     > >
>     > > Dave,
>     > >
>     > > I see that I'm late here and my email sounds like old man grumbling, but I asked
>     > > from those guys to update their commit with request to put the following line:
>     > > "/* DO NOT CHANGE DRV_MODULE_GEN_* values in in-tree code */"
>     > > https://lore.kernel.org/netdev/20200224162649.GA4526@unreal/
>     > >
>     > > I also asked how those versions are transferred to FW and used there, but was
>     > > ignored.
>     > > https://lore.kernel.org/netdev/20200224094116.GD422704@unreal/
>     > >
>     > > BTW, It is also unclear why I wasn't CCed in this patch.
>     > >
>     > > Thanks
>     >
>     > Leon,
>     >  Sorry for not responding earlier to your inquiries, they are exactly touching the
>     >  points that we would like to discuss.
>     >  Up until now, we in AWS, have been monitoring the drivers in the datacenter using the
>     >  driver version, and actively suggesting driver updates to our customers
>     >  whenever a critical bug was fixed, or a new important feature was added.
>     >  Removing the driver version and not allowing to maintain our own internal
>     >  version negatively impacts our effort to give our customers the best possible cloud
>     >  experience. We therefore prefer to maintain using our internal driver version.
>     >
>     >  Are there any other recommended ways to achieve our goal without a driver
>     >  version?
>
>     Of course, drivers are supposed to behave like any other user visible API.
>     They need to ensure backward compatibility, so new code will work with
>     old HW/FW. This is normally done by capability masks, see how it is done
>     in Mellanox drivers and I think in Intel too.
>
>     So your update policy based on driver version string is nonsense and
>     broken by design.
>
>     Original thread with Linus is here [1].
>
>     [1] https://lore.kernel.org/ksummit-discuss/CA+55aFx9A=5cc0QZ7CySC4F2K7eYaEfzkdYEc9JaNgCcV25=rg@mail.gmail.com/
>
>     Thanks
>
> We do support features capability mask as well as versioning per feature.
> However, whenever there are known issues in a certain version of the  driver
> that can be worked around by the device, we need the device to be aware of the
> driver version. Another purpose is operational - knowing driver version helps us
> reproduce customer issues and debug them, as well as suggest our customers
> to upgrade their drivers, as Arthur mentioned above.
> Thanks

Unfortunately, the versioning doesn't work outside closed source world.
This driver version doesn't say anything when customer uses stable@
kernel, distro or its custom variant. I asked more than once to explain
how this versioning works, but got only marketing answers that not
backed-up by any technical details.

The driver version works in closed source products, but here it has very
distant correlation between real driver in use and reported.

So please, stop those driver version bumps in the upstream kernel.

If you disagree with me, feel free to revive the thread in the link
posted above and we will be glad to hear your technical claims there.

Till then, we have unified kernel policy, no driver versions.

Thanks

>
>
>     >
>     >  Thanks!
>     >
>
>