Netdev Archive on lore.kernel.org
 help / color / Atom feed
From: Jakub Kicinski <jakub.kicinski@netronome.com>
To: Catherine Sullivan <csully@google.com>
Cc: netdev@vger.kernel.org, Sagi Shahar <sagis@google.com>,
	Jon Olson <jonolson@google.com>,
	Willem de Bruijn <willemb@google.com>,
	Luigi Rizzo <lrizzo@google.com>
Subject: Re: [net-next 1/4] gve: Add basic driver framework for Compute Engine Virtual NIC
Date: Wed, 26 Jun 2019 16:08:32 -0700
Message-ID: <20190626160832.3f191a53@cakuba.netronome.com> (raw)
In-Reply-To: <20190626185251.205687-2-csully@google.com>

On Wed, 26 Jun 2019 11:52:48 -0700, Catherine Sullivan wrote:
> Add a driver framework for the Compute Engine Virtual NIC that will be
> available in the future.
> 
> At this point the only functionality is loading the driver.
> 
> Signed-off-by: Catherine Sullivan <csully@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Jon Olson <jonolson@google.com>
> Acked-by: Willem de Bruijn <willemb@google.com>
> Reviewed-by: Luigi Rizzo <lrizzo@google.com>
> ---

> +if NET_VENDOR_GOOGLE
> +
> +config GVE
> +	tristate "Google Virtual NIC (gVNIC) support"
> +	depends on (PCI_MSI && X86)

We usually prefer for drivers not to depend on the platform unless
really necessary, but IDK how that applies to the curious new world 
of NICs nobody can buy :)

> +	help
> +	  This driver supports Google Virtual NIC (gVNIC)"
> +
> +	  To compile this driver as a module, choose M here.
> +	  The module will be called gve.
> +
> +endif #NET_VENDOR_GOOGLE

> +void gve_adminq_release(struct gve_priv *priv)
> +{
> +	int i;
> +
> +	/* Tell the device the adminq is leaving */
> +	writel(0x0, &priv->reg_bar0->adminq_pfn);
> +	for (i = 0; i < GVE_MAX_ADMINQ_RELEASE_CHECK; i++) {
> +		if (!readl(&priv->reg_bar0->adminq_pfn)) {
> +			gve_clear_device_rings_ok(priv);
> +			gve_clear_device_resources_ok(priv);
> +			gve_clear_admin_queue_ok(priv);
> +			return;
> +		}
> +		msleep(GVE_ADMINQ_SLEEP_LEN);
> +	}
> +	/* If this is reached the device is unrecoverable and still holding
> +	 * memory. Anything other than a BUG risks memory corruption.
> +	 */
> +	WARN(1, "Unrecoverable platform error!");
> +	BUG();

Please don't add BUG()s to the kernel.  You're probably better off
spinning for ever in the loop above.  Also if there is an IOMMU in 
the way the device won't be able to mess with the memory.

> +}
> +

> diff --git a/drivers/net/ethernet/google/gve/gve_size_assert.h b/drivers/net/ethernet/google/gve/gve_size_assert.h
> new file mode 100644
> index 000000000000..a58422d4f16e
> --- /dev/null
> +++ b/drivers/net/ethernet/google/gve/gve_size_assert.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT)
> + * Google virtual Ethernet (gve) driver
> + *
> + * Copyright (C) 2015-2019 Google, Inc.
> + */
> +
> +#ifndef _GVE_ASSERT_H_
> +#define _GVE_ASSERT_H_
> +#define GVE_ASSERT_SIZE(tag, type, size) \
> +	static void gve_assert_size_ ## type(void) __attribute__((used)); \
> +	static inline void gve_assert_size_ ## type(void) \
> +	{ \
> +		BUILD_BUG_ON(sizeof(tag type) != (size)); \
> +	}
> +#endif /* _GVE_ASSERT_H_ */

Please use static_assert() directly in your struct size checks.

  parent reply index

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-26 18:52 [net-next 0/4] Add gve driver Catherine Sullivan
2019-06-26 18:52 ` [net-next 1/4] gve: Add basic driver framework for Compute Engine Virtual NIC Catherine Sullivan
2019-06-26 19:35   ` Andrew Lunn
2019-06-28 17:54     ` Catherine Sullivan
2019-06-26 23:08   ` Jakub Kicinski [this message]
2019-06-28 17:52     ` Catherine Sullivan
2019-06-28 18:46       ` Jakub Kicinski
2019-06-28 20:06         ` Andrew Lunn
2019-06-28 23:05           ` Catherine Sullivan
2019-06-29  2:21   ` kbuild test robot
2019-06-29  2:21   ` [RFC PATCH] gve: gve_version_prefix[] can be static kbuild test robot
2019-06-29 10:25   ` [net-next 1/4] gve: Add basic driver framework for Compute Engine Virtual NIC kbuild test robot
2019-06-26 18:52 ` [net-next 2/4] gve: Add transmit and receive support Catherine Sullivan
2019-06-26 19:42   ` Andrew Lunn
2019-06-28 17:50     ` Catherine Sullivan
2019-06-26 19:49   ` David Miller
2019-06-28 17:48     ` Catherine Sullivan
2019-06-29  4:13   ` kbuild test robot
2019-06-29  4:13   ` [RFC PATCH] gve: gve_napi_poll() can be static kbuild test robot
2019-06-29 11:30   ` [net-next 2/4] gve: Add transmit and receive support kbuild test robot
2019-06-26 18:52 ` [net-next 3/4] gve: Add workqueue and reset support Catherine Sullivan
2019-06-26 18:52 ` [net-next 4/4] gve: Add ethtool support Catherine Sullivan
2019-06-26 19:48   ` Andrew Lunn
2019-06-28 17:42     ` Catherine Sullivan
2019-06-29  2:44   ` Florian Fainelli
2019-07-01 23:37     ` Catherine Sullivan
2019-06-29  6:40   ` kbuild test robot
2019-06-29  6:40   ` [RFC PATCH] gve: gve_get_channels() can be static kbuild test robot

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190626160832.3f191a53@cakuba.netronome.com \
    --to=jakub.kicinski@netronome.com \
    --cc=csully@google.com \
    --cc=jonolson@google.com \
    --cc=lrizzo@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=sagis@google.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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

Example config snippet for mirrors

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