From: Leon Romanovsky <leon@kernel.org>
To: Jeroen de Borst <jeroendb@google.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org
Subject: Re: [PATCH net-next v2] gve: Introduce a way to disable queue formats.
Date: Wed, 1 Feb 2023 12:18:15 +0200 [thread overview]
Message-ID: <Y9o8Z5RJUerdEQVz@unreal> (raw)
In-Reply-To: <20230131163329.213361-1-jeroendb@google.com>
On Tue, Jan 31, 2023 at 08:33:29AM -0800, Jeroen de Borst wrote:
> The device is capable of simultaneously supporting multiple
> queue formats. These queue formats are:
>
> - GQI-QPL: A queue format with in-order completions and a
> bounce-buffer (Queue Page List)
> - GQI-RDA: A queue format with in-order completions and no
> bounce-buffer (Raw DMA Access)
> - DQO-RDA: A queue format with out-of-order completions and
> no bounce buffer
>
> With this change the driver can deliberately pick a queue format.
>
> Changed in v2:
> - Documented queue formats and addressed nits
Please put changelog under --- trailer.
Thanks
>
> Signed-off-by: Jeroen de Borst <jeroendb@google.com>
> ---
> drivers/net/ethernet/google/gve/gve.h | 28 +++++++++
> drivers/net/ethernet/google/gve/gve_adminq.c | 35 +++++++-----
> drivers/net/ethernet/google/gve/gve_ethtool.c | 57 ++++++++++++-------
> drivers/net/ethernet/google/gve/gve_main.c | 26 ++++++++-
> 4 files changed, 109 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
> index 64eb0442c82f..68f9bf9f216d 100644
> --- a/drivers/net/ethernet/google/gve/gve.h
> +++ b/drivers/net/ethernet/google/gve/gve.h
> @@ -505,6 +505,8 @@ enum gve_queue_format {
> GVE_DQO_RDA_FORMAT = 0x3,
> };
>
> +const char *gve_queue_format_name(enum gve_queue_format format);
> +
> struct gve_priv {
> struct net_device *dev;
> struct gve_tx_ring *tx; /* array of tx_cfg.num_queues */
> @@ -584,6 +586,7 @@ struct gve_priv {
> u64 stats_report_len;
> dma_addr_t stats_report_bus; /* dma address for the stats report */
> unsigned long ethtool_flags;
> + unsigned long ethtool_formats; /* queue formats allowed */
>
> unsigned long stats_report_timer_period;
> struct timer_list stats_report_timer;
> @@ -599,6 +602,7 @@ struct gve_priv {
> int data_buffer_size_dqo;
>
> enum gve_queue_format queue_format;
> + enum gve_queue_format next_queue_format;
>
> /* Interrupt coalescing settings */
> u32 tx_coalesce_usecs;
> @@ -621,8 +625,17 @@ enum gve_state_flags_bit {
>
> enum gve_ethtool_flags_bit {
> GVE_PRIV_FLAGS_REPORT_STATS = 0,
> + GVE_PRIV_FLAGS_ENABLE_DQO_RDA = 1,
> + GVE_PRIV_FLAGS_ENABLE_GQI_RDA = 2,
> + GVE_PRIV_FLAGS_ENABLE_GQI_QPL = 3,
> };
>
> +#define GVE_PRIV_FLAGS_MASK \
> + (BIT(GVE_PRIV_FLAGS_REPORT_STATS) | \
> + BIT(GVE_PRIV_FLAGS_ENABLE_DQO_RDA) | \
> + BIT(GVE_PRIV_FLAGS_ENABLE_GQI_RDA) | \
> + BIT(GVE_PRIV_FLAGS_ENABLE_GQI_QPL))
> +
> static inline bool gve_get_do_reset(struct gve_priv *priv)
> {
> return test_bit(GVE_PRIV_FLAGS_DO_RESET, &priv->service_task_flags);
> @@ -756,6 +769,21 @@ static inline void gve_clear_report_stats(struct gve_priv *priv)
> clear_bit(GVE_PRIV_FLAGS_REPORT_STATS, &priv->ethtool_flags);
> }
>
> +static inline bool gve_get_enable_dqo_rda(struct gve_priv *priv)
> +{
> + return test_bit(GVE_PRIV_FLAGS_ENABLE_DQO_RDA, &priv->ethtool_flags);
> +}
> +
> +static inline bool gve_get_enable_gqi_rda(struct gve_priv *priv)
> +{
> + return test_bit(GVE_PRIV_FLAGS_ENABLE_GQI_RDA, &priv->ethtool_flags);
> +}
> +
> +static inline bool gve_get_enable_gqi_qpl(struct gve_priv *priv)
> +{
> + return test_bit(GVE_PRIV_FLAGS_ENABLE_GQI_QPL, &priv->ethtool_flags);
> +}
> +
> /* Returns the address of the ntfy_blocks irq doorbell
> */
> static inline __be32 __iomem *gve_irq_doorbell(struct gve_priv *priv,
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 60061288ad9d..d8bf761ef133 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -59,10 +59,9 @@ void gve_parse_device_option(struct gve_priv *priv,
> option_length, req_feat_mask);
> break;
> }
> -
> dev_info(&priv->pdev->dev,
> "Gqi raw addressing device option enabled.\n");
> - priv->queue_format = GVE_GQI_RDA_FORMAT;
> + priv->ethtool_formats |= BIT(GVE_PRIV_FLAGS_ENABLE_GQI_RDA);
> break;
> case GVE_DEV_OPT_ID_GQI_RDA:
> if (option_length < sizeof(**dev_op_gqi_rda) ||
> @@ -79,6 +78,7 @@ void gve_parse_device_option(struct gve_priv *priv,
> GVE_DEVICE_OPTION_TOO_BIG_FMT, "GQI RDA");
> }
> *dev_op_gqi_rda = (void *)(option + 1);
> + priv->ethtool_formats |= BIT(GVE_PRIV_FLAGS_ENABLE_GQI_RDA);
> break;
> case GVE_DEV_OPT_ID_GQI_QPL:
> if (option_length < sizeof(**dev_op_gqi_qpl) ||
> @@ -95,6 +95,7 @@ void gve_parse_device_option(struct gve_priv *priv,
> GVE_DEVICE_OPTION_TOO_BIG_FMT, "GQI QPL");
> }
> *dev_op_gqi_qpl = (void *)(option + 1);
> + priv->ethtool_formats |= BIT(GVE_PRIV_FLAGS_ENABLE_GQI_QPL);
> break;
> case GVE_DEV_OPT_ID_DQO_RDA:
> if (option_length < sizeof(**dev_op_dqo_rda) ||
> @@ -111,6 +112,7 @@ void gve_parse_device_option(struct gve_priv *priv,
> GVE_DEVICE_OPTION_TOO_BIG_FMT, "DQO RDA");
> }
> *dev_op_dqo_rda = (void *)(option + 1);
> + priv->ethtool_formats |= BIT(GVE_PRIV_FLAGS_ENABLE_DQO_RDA);
> break;
> case GVE_DEV_OPT_ID_JUMBO_FRAMES:
> if (option_length < sizeof(**dev_op_jumbo_frames) ||
> @@ -737,33 +739,36 @@ int gve_adminq_describe_device(struct gve_priv *priv)
> if (err)
> goto free_device_descriptor;
>
> + priv->ethtool_flags = priv->ethtool_formats;
> +
> /* If the GQI_RAW_ADDRESSING option is not enabled and the queue format
> * is not set to GqiRda, choose the queue format in a priority order:
> * DqoRda, GqiRda, GqiQpl. Use GqiQpl as default.
> */
> if (dev_op_dqo_rda) {
> priv->queue_format = GVE_DQO_RDA_FORMAT;
> - dev_info(&priv->pdev->dev,
> - "Driver is running with DQO RDA queue format.\n");
> supported_features_mask =
> be32_to_cpu(dev_op_dqo_rda->supported_features_mask);
> } else if (dev_op_gqi_rda) {
> priv->queue_format = GVE_GQI_RDA_FORMAT;
> - dev_info(&priv->pdev->dev,
> - "Driver is running with GQI RDA queue format.\n");
> supported_features_mask =
> be32_to_cpu(dev_op_gqi_rda->supported_features_mask);
> - } else if (priv->queue_format == GVE_GQI_RDA_FORMAT) {
> - dev_info(&priv->pdev->dev,
> - "Driver is running with GQI RDA queue format.\n");
> - } else {
> + } else if (dev_op_gqi_qpl) {
> priv->queue_format = GVE_GQI_QPL_FORMAT;
> - if (dev_op_gqi_qpl)
> - supported_features_mask =
> - be32_to_cpu(dev_op_gqi_qpl->supported_features_mask);
> - dev_info(&priv->pdev->dev,
> - "Driver is running with GQI QPL queue format.\n");
> + supported_features_mask =
> + be32_to_cpu(dev_op_gqi_qpl->supported_features_mask);
> + } else if (gve_get_enable_gqi_rda(priv)) {
> + priv->queue_format = GVE_GQI_RDA_FORMAT;
> + } else {
> + dev_err(&priv->pdev->dev, "No compatible queue formats\n");
> + err = -EINVAL;
> + goto free_device_descriptor;
> }
> + dev_info(&priv->pdev->dev, "Driver is running with %s queue format.\n",
> + gve_queue_format_name(priv->queue_format));
> +
> + priv->next_queue_format = priv->queue_format;
> +
> if (gve_is_gqi(priv)) {
> err = gve_set_desc_cnt(priv, descriptor);
> } else {
> diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
> index ce574d097e28..9e36d2c450f7 100644
> --- a/drivers/net/ethernet/google/gve/gve_ethtool.c
> +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
> @@ -68,7 +68,7 @@ static const char gve_gstrings_adminq_stats[][ETH_GSTRING_LEN] = {
> };
>
> static const char gve_gstrings_priv_flags[][ETH_GSTRING_LEN] = {
> - "report-stats",
> + "report-stats", "enable-dqo-rda", "enable-gqi-rda", "enable-gqi-qpl",
> };
>
> #define GVE_MAIN_STATS_LEN ARRAY_SIZE(gve_gstrings_main_stats)
> @@ -490,37 +490,36 @@ static int gve_set_tunable(struct net_device *netdev,
> static u32 gve_get_priv_flags(struct net_device *netdev)
> {
> struct gve_priv *priv = netdev_priv(netdev);
> - u32 ret_flags = 0;
> -
> - /* Only 1 flag exists currently: report-stats (BIT(O)), so set that flag. */
> - if (priv->ethtool_flags & BIT(0))
> - ret_flags |= BIT(0);
> - return ret_flags;
> + return priv->ethtool_flags;
> }
>
> static int gve_set_priv_flags(struct net_device *netdev, u32 flags)
> {
> struct gve_priv *priv = netdev_priv(netdev);
> - u64 ori_flags, new_flags;
> + enum gve_queue_format new_format;
> + u32 ori_flags;
> +
> + /* Make sure there is a queue format to use */
> + if ((priv->ethtool_formats & flags) == 0) {
> + dev_err(&priv->pdev->dev,
> + "All available queue formats disabled\n");
> + return -EINVAL;
> + }
>
> ori_flags = READ_ONCE(priv->ethtool_flags);
> - new_flags = ori_flags;
>
> - /* Only one priv flag exists: report-stats (BIT(0))*/
> - if (flags & BIT(0))
> - new_flags |= BIT(0);
> - else
> - new_flags &= ~(BIT(0));
> - priv->ethtool_flags = new_flags;
> + priv->ethtool_flags = flags & GVE_PRIV_FLAGS_MASK;
> +
> /* start report-stats timer when user turns report stats on. */
> - if (flags & BIT(0)) {
> + if (gve_get_report_stats(priv))
> mod_timer(&priv->stats_report_timer,
> round_jiffies(jiffies +
> msecs_to_jiffies(priv->stats_report_timer_period)));
> - }
> +
> /* Zero off gve stats when report-stats turned off and */
> /* delete report stats timer. */
> - if (!(flags & BIT(0)) && (ori_flags & BIT(0))) {
> + if (!gve_get_report_stats(priv) &&
> + (ori_flags & BIT(GVE_PRIV_FLAGS_REPORT_STATS))) {
> int tx_stats_num = GVE_TX_STATS_REPORT_NUM *
> priv->tx_cfg.num_queues;
> int rx_stats_num = GVE_RX_STATS_REPORT_NUM *
> @@ -530,7 +529,27 @@ static int gve_set_priv_flags(struct net_device *netdev, u32 flags)
> sizeof(struct stats));
> del_timer_sync(&priv->stats_report_timer);
> }
> - return 0;
> +
> + if (priv->ethtool_formats == 0)
> + /* Can't change queue format */
> + return 0;
> +
> + /* Check if a new queue format should be activated */
> + if (gve_get_enable_dqo_rda(priv))
> + new_format = GVE_DQO_RDA_FORMAT;
> + else if (gve_get_enable_gqi_rda(priv))
> + new_format = GVE_GQI_RDA_FORMAT;
> + else
> + new_format = GVE_GQI_QPL_FORMAT;
> +
> + if (priv->queue_format == new_format)
> + return 0;
> +
> + priv->next_queue_format = new_format;
> + dev_info(&priv->pdev->dev, "Driver is switching to %s queue format.\n",
> + gve_queue_format_name(priv->next_queue_format));
> +
> + return gve_reset(priv, false);
> }
>
> static int gve_get_link_ksettings(struct net_device *netdev,
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 5b40f9c53196..23f30d9d97a0 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -32,6 +32,23 @@
> const char gve_version_str[] = GVE_VERSION;
> static const char gve_version_prefix[] = GVE_VERSION_PREFIX;
>
> +const char *gve_queue_format_name(enum gve_queue_format format)
> +{
> + switch (format) {
> + case GVE_QUEUE_FORMAT_UNSPECIFIED:
> + return "unspecified";
> + case GVE_GQI_RDA_FORMAT:
> + return "GQI RDA";
> + case GVE_GQI_QPL_FORMAT:
> + return "GQI QPL";
> + case GVE_DQO_RDA_FORMAT:
> + return "DQO RDA";
> + default:
> + return "unknown";
> + }
> +}
> +
> +struct bpf_prog;
> static int gve_verify_driver_compatibility(struct gve_priv *priv)
> {
> int err;
> @@ -1413,6 +1430,11 @@ static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
> return err;
> }
>
> + if (skip_describe_device) {
> + priv->queue_format = priv->next_queue_format;
> + goto setup_device;
> + }
> +
> err = gve_verify_driver_compatibility(priv);
> if (err) {
> dev_err(&priv->pdev->dev,
> @@ -1420,9 +1442,6 @@ static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
> goto err;
> }
>
> - if (skip_describe_device)
> - goto setup_device;
> -
> priv->queue_format = GVE_QUEUE_FORMAT_UNSPECIFIED;
> /* Get the initial information we need from the device */
> err = gve_adminq_describe_device(priv);
> @@ -1661,6 +1680,7 @@ static int gve_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> priv->service_task_flags = 0x0;
> priv->state_flags = 0x0;
> priv->ethtool_flags = 0x0;
> + priv->ethtool_formats = 0x0;
>
> gve_set_probe_in_progress(priv);
> priv->gve_wq = alloc_ordered_workqueue("gve", 0);
> --
> 2.39.1.456.gfc5497dd1b-goog
>
prev parent reply other threads:[~2023-02-01 10:20 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 16:33 [PATCH net-next v2] gve: Introduce a way to disable queue formats Jeroen de Borst
2023-02-01 5:52 ` Jakub Kicinski
2023-02-01 17:44 ` Jeroen de Borst
2023-02-01 10:18 ` Leon Romanovsky [this message]
Reply instructions:
You may reply publicly 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=Y9o8Z5RJUerdEQVz@unreal \
--to=leon@kernel.org \
--cc=davem@davemloft.net \
--cc=jeroendb@google.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.