RE: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

2021-04-08 Thread Dexuan Cui
> From: Stephen Hemminger 
> Sent: Thursday, April 8, 2021 9:52 AM

Thanks all for your input! We'll make the below changes as suggested:

Microsoft Azure Network Device ==> Microsoft Network Devices
Drop the default m
validated ==> supported

We'll also fix some warnings reported by "kernel test robot".

Will post v3 later today.

Thanks,
Dexuan

diff --git a/drivers/net/ethernet/microsoft/Kconfig 
b/drivers/net/ethernet/microsoft/Kconfig
index 12ef6b581566..e1ac0a5d808d 100644
--- a/drivers/net/ethernet/microsoft/Kconfig
+++ b/drivers/net/ethernet/microsoft/Kconfig
@@ -3,26 +3,25 @@
 #
 
 config NET_VENDOR_MICROSOFT
-   bool "Microsoft Azure Network Device"
+   bool "Microsoft Network Devices"
default y
help
  If you have a network (Ethernet) device belonging to this class, say 
Y.
 
  Note that the answer to this question doesn't directly affect the
  kernel: saying N will just cause the configurator to skip the
- question about Microsoft Azure network device. If you say Y, you
- will be asked for your specific device in the following question.
+ question about Microsoft network devices. If you say Y, you will be
+ asked for your specific device in the following question.
 
 if NET_VENDOR_MICROSOFT
 
 config MICROSOFT_MANA
tristate "Microsoft Azure Network Adapter (MANA) support"
-   default m
depends on PCI_MSI && X86_64
select PCI_HYPERV
help
  This driver supports Microsoft Azure Network Adapter (MANA).
- So far, the driver is only validated on X86_64.
+ So far, the driver is only supported on X86_64.
 
  To compile this driver as a module, choose M here.
  The module will be called mana.


RE: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

2021-04-08 Thread Haiyang Zhang



> -Original Message-
> From: Stephen Hemminger 
> Sent: Thursday, April 8, 2021 12:52 PM
> To: Randy Dunlap 
> Cc: Dexuan Cui ; da...@davemloft.net;
> k...@kernel.org; KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger
> ; wei@kernel.org; Wei Liu
> ; net...@vger.kernel.org; l...@kernel.org;
> and...@lunn.ch; be...@petrovitsch.priv.at; linux-kernel@vger.kernel.org;
> linux-hyp...@vger.kernel.org
> Subject: Re: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure
> Network Adapter (MANA)
> 
> On Thu, 8 Apr 2021 09:22:57 -0700
> Randy Dunlap  wrote:
> 
> > On 4/8/21 2:15 AM, Dexuan Cui wrote:
> > > diff --git a/drivers/net/ethernet/microsoft/Kconfig
> > > b/drivers/net/ethernet/microsoft/Kconfig
> > > new file mode 100644
> > > index ..12ef6b581566
> > > --- /dev/null
> > > +++ b/drivers/net/ethernet/microsoft/Kconfig
> > > @@ -0,0 +1,30 @@
> > > +#
> > > +# Microsoft Azure network device configuration #
> > > +
> > > +config NET_VENDOR_MICROSOFT
> > > + bool "Microsoft Azure Network Device"
> >
> > Seems to me that should be generalized, more like:
> >
> > bool "Microsoft Network Devices"
> 
> Yes, that is what it should be at this level.
> 
> >
> >
> > > + default y
> 
> This follows the existing policy for network vendor level
> 
> > > + help
> > > +   If you have a network (Ethernet) device belonging to this class, say 
> > > Y.
> > > +
> > > +   Note that the answer to this question doesn't directly affect the
> > > +   kernel: saying N will just cause the configurator to skip the
> > > +   question about Microsoft Azure network device. If you say Y, you
> >
> >about Microsoft networking devices.
> >
> > > +   will be asked for your specific device in the following question.
> > > +
> > > +if NET_VENDOR_MICROSOFT
> > > +
> > > +config MICROSOFT_MANA
> > > + tristate "Microsoft Azure Network Adapter (MANA) support"
> > > + default m
> >
> > Please drop the default m. We don't randomly add drivers to be built.
> 
> Yes, it should be no (or no default which is the default for default)
> 
> > Or leave this as is and change NET_VENDOR_MICROSOFT to be default n.
> >
> >
> > > + depends on PCI_MSI && X86_64
> > > + select PCI_HYPERV
> > > + help
> > > +   This driver supports Microsoft Azure Network Adapter (MANA).
> > > +   So far, the driver is only validated on X86_64.
> >
> > validated how?
> 
> Maybe change validated to supported?

Sounds better. We will change it to "supported".
Also other suggested changes.

Thanks,
- Haiyang


Re: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

2021-04-08 Thread Stephen Hemminger
On Thu, 8 Apr 2021 09:22:57 -0700
Randy Dunlap  wrote:

> On 4/8/21 2:15 AM, Dexuan Cui wrote:
> > diff --git a/drivers/net/ethernet/microsoft/Kconfig 
> > b/drivers/net/ethernet/microsoft/Kconfig
> > new file mode 100644
> > index ..12ef6b581566
> > --- /dev/null
> > +++ b/drivers/net/ethernet/microsoft/Kconfig
> > @@ -0,0 +1,30 @@
> > +#
> > +# Microsoft Azure network device configuration
> > +#
> > +
> > +config NET_VENDOR_MICROSOFT
> > +   bool "Microsoft Azure Network Device"  
> 
> Seems to me that should be generalized, more like:
> 
>   bool "Microsoft Network Devices"

Yes, that is what it should be at this level.

> 
> 
> > +   default y

This follows the existing policy for network vendor level

> > +   help
> > + If you have a network (Ethernet) device belonging to this class, say 
> > Y.
> > +
> > + Note that the answer to this question doesn't directly affect the
> > + kernel: saying N will just cause the configurator to skip the
> > + question about Microsoft Azure network device. If you say Y, you  
> 
>  about Microsoft networking devices.
> 
> > + will be asked for your specific device in the following question.
> > +
> > +if NET_VENDOR_MICROSOFT
> > +
> > +config MICROSOFT_MANA
> > +   tristate "Microsoft Azure Network Adapter (MANA) support"
> > +   default m  
> 
> Please drop the default m. We don't randomly add drivers to be built.

Yes, it should be no (or no default which is the default for default)

> Or leave this as is and change NET_VENDOR_MICROSOFT to be default n.
> 
> 
> > +   depends on PCI_MSI && X86_64
> > +   select PCI_HYPERV
> > +   help
> > + This driver supports Microsoft Azure Network Adapter (MANA).
> > + So far, the driver is only validated on X86_64.  
> 
> validated how?

Maybe change validated to supported?




RE: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

2021-04-08 Thread Haiyang Zhang



> -Original Message-
> From: Andrew Lunn 
> Sent: Thursday, April 8, 2021 12:45 PM
> To: Haiyang Zhang 
> Cc: Randy Dunlap ; Dexuan Cui
> ; da...@davemloft.net; k...@kernel.org; KY
> Srinivasan ; Stephen Hemminger
> ; wei@kernel.org; Wei Liu
> ; net...@vger.kernel.org; l...@kernel.org;
> be...@petrovitsch.priv.at; linux-kernel@vger.kernel.org; linux-
> hyp...@vger.kernel.org
> Subject: Re: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure
> Network Adapter (MANA)
> 
> > > > diff --git a/drivers/net/ethernet/microsoft/Kconfig
> > > b/drivers/net/ethernet/microsoft/Kconfig
> > > > new file mode 100644
> > > > index ..12ef6b581566
> > > > --- /dev/null
> > > > +++ b/drivers/net/ethernet/microsoft/Kconfig
> > > > @@ -0,0 +1,30 @@
> > > > +#
> > > > +# Microsoft Azure network device configuration
> > > > +#
> > > > +
> > > > +config NET_VENDOR_MICROSOFT
> > > > +   bool "Microsoft Azure Network Device"
> > >
> > > Seems to me that should be generalized, more like:
> > >
> > >   bool "Microsoft Network Devices"
> > This device is planned for Azure cloud at this time.
> > We will update the wording if things change.
> 
> This section is about the Vendor. Broadcom, Marvell, natsemi, toshiba,
> etc. Microsoft is the Vendor here and all Microsoft Ethernet drivers
> belong here. It does not matter what platform they are for.

Thanks. We will update the wording.

- Haiyang


Re: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

2021-04-08 Thread Andrew Lunn
> > > diff --git a/drivers/net/ethernet/microsoft/Kconfig
> > b/drivers/net/ethernet/microsoft/Kconfig
> > > new file mode 100644
> > > index ..12ef6b581566
> > > --- /dev/null
> > > +++ b/drivers/net/ethernet/microsoft/Kconfig
> > > @@ -0,0 +1,30 @@
> > > +#
> > > +# Microsoft Azure network device configuration
> > > +#
> > > +
> > > +config NET_VENDOR_MICROSOFT
> > > + bool "Microsoft Azure Network Device"
> > 
> > Seems to me that should be generalized, more like:
> > 
> > bool "Microsoft Network Devices"
> This device is planned for Azure cloud at this time.
> We will update the wording if things change.

This section is about the Vendor. Broadcom, Marvell, natsemi, toshiba,
etc. Microsoft is the Vendor here and all Microsoft Ethernet drivers
belong here. It does not matter what platform they are for.

   Andrew


RE: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

2021-04-08 Thread Haiyang Zhang


> -Original Message-
> From: Randy Dunlap 
> Sent: Thursday, April 8, 2021 12:23 PM
> To: Dexuan Cui ; da...@davemloft.net;
> k...@kernel.org; KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger
> ; wei@kernel.org; Wei Liu
> ; net...@vger.kernel.org; l...@kernel.org;
> and...@lunn.ch; be...@petrovitsch.priv.at
> Cc: linux-kernel@vger.kernel.org; linux-hyp...@vger.kernel.org
> Subject: Re: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure
> Network Adapter (MANA)
> 
> On 4/8/21 2:15 AM, Dexuan Cui wrote:
> > diff --git a/drivers/net/ethernet/microsoft/Kconfig
> b/drivers/net/ethernet/microsoft/Kconfig
> > new file mode 100644
> > index ..12ef6b581566
> > --- /dev/null
> > +++ b/drivers/net/ethernet/microsoft/Kconfig
> > @@ -0,0 +1,30 @@
> > +#
> > +# Microsoft Azure network device configuration
> > +#
> > +
> > +config NET_VENDOR_MICROSOFT
> > +   bool "Microsoft Azure Network Device"
> 
> Seems to me that should be generalized, more like:
> 
>   bool "Microsoft Network Devices"
This device is planned for Azure cloud at this time.
We will update the wording if things change.

> 
> 
> > +   default y
> > +   help
> > + If you have a network (Ethernet) device belonging to this class, say 
> > Y.
> > +
> > + Note that the answer to this question doesn't directly affect the
> > + kernel: saying N will just cause the configurator to skip the
> > + question about Microsoft Azure network device. If you say Y, you
> 
>  about Microsoft networking devices.
(ditto)

> 
> > + will be asked for your specific device in the following question.
> > +
> > +if NET_VENDOR_MICROSOFT
> > +
> > +config MICROSOFT_MANA
> > +   tristate "Microsoft Azure Network Adapter (MANA) support"
> > +   default m
> 
> Please drop the default m. We don't randomly add drivers to be built.
We will.

> 
> Or leave this as is and change NET_VENDOR_MICROSOFT to be default n.
> 
> 
> > +   depends on PCI_MSI && X86_64
> > +   select PCI_HYPERV
> > +   help
> > + This driver supports Microsoft Azure Network Adapter (MANA).
> > + So far, the driver is only validated on X86_64.
> 
> validated how?
On our pre-released HW.

Thanks,
- Haiyang


Re: [PATCH v2 net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

2021-04-08 Thread Randy Dunlap
On 4/8/21 2:15 AM, Dexuan Cui wrote:
> diff --git a/drivers/net/ethernet/microsoft/Kconfig 
> b/drivers/net/ethernet/microsoft/Kconfig
> new file mode 100644
> index ..12ef6b581566
> --- /dev/null
> +++ b/drivers/net/ethernet/microsoft/Kconfig
> @@ -0,0 +1,30 @@
> +#
> +# Microsoft Azure network device configuration
> +#
> +
> +config NET_VENDOR_MICROSOFT
> + bool "Microsoft Azure Network Device"

Seems to me that should be generalized, more like:

bool "Microsoft Network Devices"


> + default y
> + help
> +   If you have a network (Ethernet) device belonging to this class, say 
> Y.
> +
> +   Note that the answer to this question doesn't directly affect the
> +   kernel: saying N will just cause the configurator to skip the
> +   question about Microsoft Azure network device. If you say Y, you

   about Microsoft networking devices.

> +   will be asked for your specific device in the following question.
> +
> +if NET_VENDOR_MICROSOFT
> +
> +config MICROSOFT_MANA
> + tristate "Microsoft Azure Network Adapter (MANA) support"
> + default m

Please drop the default m. We don't randomly add drivers to be built.

Or leave this as is and change NET_VENDOR_MICROSOFT to be default n.


> + depends on PCI_MSI && X86_64
> + select PCI_HYPERV
> + help
> +   This driver supports Microsoft Azure Network Adapter (MANA).
> +   So far, the driver is only validated on X86_64.

validated how?


> +
> +   To compile this driver as a module, choose M here.
> +   The module will be called mana.
> +
> +endif #NET_VENDOR_MICROSOFT


thanks.
-- 
~Randy