Re: [PATCH net-next v3 1/4] gve: Add support for raw addressing device option
From: Jakub Kicinski <kuba@kernel.org>
Date: 2020-09-24 23:03:38
On Wed, 23 Sep 2020 18:01:01 -0700 David Awogbemila wrote:
quoted hunk ↗ jump to hunk
@@ -518,6 +521,49 @@ int gve_adminq_describe_device(struct gve_priv *priv) priv->rx_desc_cnt = priv->rx_pages_per_qpl; } priv->default_num_queues = be16_to_cpu(descriptor->default_num_queues); + dev_opt = (void *)(descriptor + 1); + + num_options = be16_to_cpu(descriptor->num_device_options); + for (i = 0; i < num_options; i++) { + u16 option_length = be16_to_cpu(dev_opt->option_length); + u16 option_id = be16_to_cpu(dev_opt->option_id); + + if ((void *)dev_opt + sizeof(*dev_opt) + option_length > (void *)descriptor + + be16_to_cpu(descriptor->total_length)) {
Can you calculate an void *end pointer before the loop and avoid this very long condition?
+ dev_err(&priv->dev->dev,
+ "options exceed device_descriptor's total length.\n");
+ err = -EINVAL;
+ goto free_device_descriptor;
+ }
+
+ switch (option_id) {
+ case GVE_DEV_OPT_ID_RAW_ADDRESSING:
+ /* If the length or feature mask doesn't match,
+ * continue without enabling the feature.
+ */
+ if (option_length != GVE_DEV_OPT_LEN_RAW_ADDRESSING ||
+ be32_to_cpu(dev_opt->feat_mask) !=
+ GVE_DEV_OPT_FEAT_MASK_RAW_ADDRESSING) {Apply the byteswap to the constant so it's done at compilation time.
+ dev_info(&priv->pdev->dev, + "Raw addressing device option not enabled, length or features mask did not match expected.\n");
dev_warn(), also do yourself a favor and print what the values were.
+ priv->raw_addressing = false;
+ } else {
+ dev_info(&priv->pdev->dev,
+ "Raw addressing device option enabled.\n");
+ priv->raw_addressing = true;Does this really need to be printed on every boot?
+ } + break; + default: + /* If we don't recognize the option just continue + * without doing anything. + */ + dev_info(&priv->pdev->dev, + "Unrecognized device option 0x%hx not enabled.\n",
dev_dbg()
+ option_id);
alignment is off
+ break; + } + dev_opt = (void *)dev_opt + sizeof(*dev_opt) + option_length; + }