[PATCH net-next 0/2] gve: Add RSS config support

[PATCH net-next 0/2] gve: Add RSS config support

[Praveen Kaligineedi]

From: Ziwei Xiao <ziweixiao@google.com>

These two patches are used to add RSS config support in GVE driver
between the device and ethtool.

Jeroen de Borst (1):
  gve: Add RSS adminq commands and ethtool support

Ziwei Xiao (1):
  gve: Add RSS device option

 drivers/net/ethernet/google/gve/gve.h         |  11 ++
 drivers/net/ethernet/google/gve/gve_adminq.c  | 169 +++++++++++++++++-
 drivers/net/ethernet/google/gve/gve_adminq.h  |  59 +++++-
 drivers/net/ethernet/google/gve/gve_ethtool.c | 118 +++++++++++-
 4 files changed, 352 insertions(+), 5 deletions(-)

-- 
2.46.0.rc1.232.g9752f9e123-goog

[PATCH net-next 1/2] gve: Add RSS device option

[Praveen Kaligineedi]

From: Ziwei Xiao <ziweixiao@google.com>

Add a device option to inform the driver about the hash key size and
hash table size used by the device. This information will be stored and
made available for RSS ethtool operations.

Signed-off-by: Ziwei Xiao <ziweixiao@google.com>
Signed-off-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/ethernet/google/gve/gve.h        |  3 ++
 drivers/net/ethernet/google/gve/gve_adminq.c | 38 ++++++++++++++++++--
 drivers/net/ethernet/google/gve/gve_adminq.h | 15 +++++++-
 3 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
index 84ac004d3953..6c21f3c53619 100644
--- a/drivers/net/ethernet/google/gve/gve.h
+++ b/drivers/net/ethernet/google/gve/gve.h
@@ -831,6 +831,9 @@ struct gve_priv {
 	u32 num_flow_rules;
 
 	struct gve_flow_rules_cache flow_rules_cache;
+
+	u16 rss_key_size;
+	u16 rss_lut_size;
 };
 
 enum gve_service_task_flags_bit {
diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
index c5bbc1b7524e..b0b7ef8a47d5 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.c
+++ b/drivers/net/ethernet/google/gve/gve_adminq.c
@@ -45,7 +45,8 @@ void gve_parse_device_option(struct gve_priv *priv,
 			     struct gve_device_option_dqo_qpl **dev_op_dqo_qpl,
 			     struct gve_device_option_buffer_sizes **dev_op_buffer_sizes,
 			     struct gve_device_option_flow_steering **dev_op_flow_steering,
-			     struct gve_device_option_modify_ring **dev_op_modify_ring)
+			     struct gve_device_option_modify_ring **dev_op_modify_ring,
+			     struct gve_device_option_rss_config **dev_op_rss_config)
 {
 	u32 req_feat_mask = be32_to_cpu(option->required_features_mask);
 	u16 option_length = be16_to_cpu(option->option_length);
@@ -207,6 +208,23 @@ void gve_parse_device_option(struct gve_priv *priv,
 				 "Flow Steering");
 		*dev_op_flow_steering = (void *)(option + 1);
 		break;
+	case GVE_DEV_OPT_ID_RSS_CONFIG:
+		if (option_length < sizeof(**dev_op_rss_config) ||
+		    req_feat_mask != GVE_DEV_OPT_REQ_FEAT_MASK_RSS_CONFIG) {
+			dev_warn(&priv->pdev->dev, GVE_DEVICE_OPTION_ERROR_FMT,
+				 "RSS config",
+				 (int)sizeof(**dev_op_rss_config),
+				 GVE_DEV_OPT_REQ_FEAT_MASK_RSS_CONFIG,
+				 option_length, req_feat_mask);
+			break;
+		}
+
+		if (option_length > sizeof(**dev_op_rss_config))
+			dev_warn(&priv->pdev->dev,
+				 GVE_DEVICE_OPTION_TOO_BIG_FMT,
+				 "RSS config");
+		*dev_op_rss_config = (void *)(option + 1);
+		break;
 	default:
 		/* If we don't recognize the option just continue
 		 * without doing anything.
@@ -227,6 +245,7 @@ gve_process_device_options(struct gve_priv *priv,
 			   struct gve_device_option_dqo_qpl **dev_op_dqo_qpl,
 			   struct gve_device_option_buffer_sizes **dev_op_buffer_sizes,
 			   struct gve_device_option_flow_steering **dev_op_flow_steering,
+			   struct gve_device_option_rss_config **dev_op_rss_config,
 			   struct gve_device_option_modify_ring **dev_op_modify_ring)
 {
 	const int num_options = be16_to_cpu(descriptor->num_device_options);
@@ -249,7 +268,8 @@ gve_process_device_options(struct gve_priv *priv,
 					dev_op_gqi_rda, dev_op_gqi_qpl,
 					dev_op_dqo_rda, dev_op_jumbo_frames,
 					dev_op_dqo_qpl, dev_op_buffer_sizes,
-					dev_op_flow_steering, dev_op_modify_ring);
+					dev_op_flow_steering, dev_op_modify_ring,
+					dev_op_rss_config);
 		dev_opt = next_opt;
 	}
 
@@ -867,6 +887,8 @@ static void gve_enable_supported_features(struct gve_priv *priv,
 					  *dev_op_buffer_sizes,
 					  const struct gve_device_option_flow_steering
 					  *dev_op_flow_steering,
+					  const struct gve_device_option_rss_config
+					  *dev_op_rss_config,
 					  const struct gve_device_option_modify_ring
 					  *dev_op_modify_ring)
 {
@@ -931,6 +953,14 @@ static void gve_enable_supported_features(struct gve_priv *priv,
 				 priv->max_flow_rules);
 		}
 	}
+
+	if (dev_op_rss_config &&
+	    (supported_features_mask & GVE_SUP_RSS_CONFIG_MASK)) {
+		priv->rss_key_size =
+			be16_to_cpu(dev_op_rss_config->hash_key_size);
+		priv->rss_lut_size =
+			be16_to_cpu(dev_op_rss_config->hash_lut_size);
+	}
 }
 
 int gve_adminq_describe_device(struct gve_priv *priv)
@@ -939,6 +969,7 @@ int gve_adminq_describe_device(struct gve_priv *priv)
 	struct gve_device_option_buffer_sizes *dev_op_buffer_sizes = NULL;
 	struct gve_device_option_jumbo_frames *dev_op_jumbo_frames = NULL;
 	struct gve_device_option_modify_ring *dev_op_modify_ring = NULL;
+	struct gve_device_option_rss_config *dev_op_rss_config = NULL;
 	struct gve_device_option_gqi_rda *dev_op_gqi_rda = NULL;
 	struct gve_device_option_gqi_qpl *dev_op_gqi_qpl = NULL;
 	struct gve_device_option_dqo_rda *dev_op_dqo_rda = NULL;
@@ -973,6 +1004,7 @@ int gve_adminq_describe_device(struct gve_priv *priv)
 					 &dev_op_jumbo_frames, &dev_op_dqo_qpl,
 					 &dev_op_buffer_sizes,
 					 &dev_op_flow_steering,
+					 &dev_op_rss_config,
 					 &dev_op_modify_ring);
 	if (err)
 		goto free_device_descriptor;
@@ -1035,7 +1067,7 @@ int gve_adminq_describe_device(struct gve_priv *priv)
 	gve_enable_supported_features(priv, supported_features_mask,
 				      dev_op_jumbo_frames, dev_op_dqo_qpl,
 				      dev_op_buffer_sizes, dev_op_flow_steering,
-				      dev_op_modify_ring);
+				      dev_op_rss_config, dev_op_modify_ring);
 
 free_device_descriptor:
 	dma_pool_free(priv->adminq_pool, descriptor, descriptor_bus);
diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
index ed1370c9b197..7d9ef9a12fef 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.h
+++ b/drivers/net/ethernet/google/gve/gve_adminq.h
@@ -164,6 +164,14 @@ struct gve_device_option_flow_steering {
 
 static_assert(sizeof(struct gve_device_option_flow_steering) == 12);
 
+struct gve_device_option_rss_config {
+	__be32 supported_features_mask;
+	__be16 hash_key_size;
+	__be16 hash_lut_size;
+};
+
+static_assert(sizeof(struct gve_device_option_rss_config) == 8);
+
 /* Terminology:
  *
  * RDA - Raw DMA Addressing - Buffers associated with SKBs are directly DMA
@@ -182,6 +190,7 @@ enum gve_dev_opt_id {
 	GVE_DEV_OPT_ID_JUMBO_FRAMES		= 0x8,
 	GVE_DEV_OPT_ID_BUFFER_SIZES		= 0xa,
 	GVE_DEV_OPT_ID_FLOW_STEERING		= 0xb,
+	GVE_DEV_OPT_ID_RSS_CONFIG		= 0xe,
 };
 
 enum gve_dev_opt_req_feat_mask {
@@ -194,6 +203,7 @@ enum gve_dev_opt_req_feat_mask {
 	GVE_DEV_OPT_REQ_FEAT_MASK_BUFFER_SIZES		= 0x0,
 	GVE_DEV_OPT_REQ_FEAT_MASK_MODIFY_RING		= 0x0,
 	GVE_DEV_OPT_REQ_FEAT_MASK_FLOW_STEERING		= 0x0,
+	GVE_DEV_OPT_REQ_FEAT_MASK_RSS_CONFIG		= 0x0,
 };
 
 enum gve_sup_feature_mask {
@@ -201,6 +211,7 @@ enum gve_sup_feature_mask {
 	GVE_SUP_JUMBO_FRAMES_MASK	= 1 << 2,
 	GVE_SUP_BUFFER_SIZES_MASK	= 1 << 4,
 	GVE_SUP_FLOW_STEERING_MASK	= 1 << 5,
+	GVE_SUP_RSS_CONFIG_MASK		= 1 << 7,
 };
 
 #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
@@ -214,6 +225,7 @@ enum gve_driver_capbility {
 	gve_driver_capability_dqo_rda = 3,
 	gve_driver_capability_alt_miss_compl = 4,
 	gve_driver_capability_flexible_buffer_size = 5,
+	gve_driver_capability_flexible_rss_size = 6,
 };
 
 #define GVE_CAP1(a) BIT((int)a)
@@ -226,7 +238,8 @@ enum gve_driver_capbility {
 	 GVE_CAP1(gve_driver_capability_gqi_rda) | \
 	 GVE_CAP1(gve_driver_capability_dqo_rda) | \
 	 GVE_CAP1(gve_driver_capability_alt_miss_compl) | \
-	 GVE_CAP1(gve_driver_capability_flexible_buffer_size))
+	 GVE_CAP1(gve_driver_capability_flexible_buffer_size) | \
+	 GVE_CAP1(gve_driver_capability_flexible_rss_size))
 
 #define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
 #define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
-- 
2.46.0.rc1.232.g9752f9e123-goog

[PATCH net-next 2/2] gve: Add RSS adminq commands and ethtool support

[Praveen Kaligineedi]

From: Jeroen de Borst <jeroendb@google.com>

Introduce adminq commands to configure and retrieve RSS settings from
the device. Implement corresponding ethtool ops for user-level
management.

Signed-off-by: Jeroen de Borst <jeroendb@google.com>
Co-developed-by: Ziwei Xiao <ziweixiao@google.com>
Signed-off-by: Ziwei Xiao <ziweixiao@google.com>
Signed-off-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/ethernet/google/gve/gve.h         |   8 ++
 drivers/net/ethernet/google/gve/gve_adminq.c  | 131 ++++++++++++++++++
 drivers/net/ethernet/google/gve/gve_adminq.h  |  44 ++++++
 drivers/net/ethernet/google/gve/gve_ethtool.c | 118 +++++++++++++++-
 4 files changed, 300 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
index 6c21f3c53619..b838a135b320 100644
--- a/drivers/net/ethernet/google/gve/gve.h
+++ b/drivers/net/ethernet/google/gve/gve.h
@@ -643,6 +643,12 @@ struct gve_ptype_lut {
 	struct gve_ptype ptypes[GVE_NUM_PTYPES];
 };
 
+struct gve_rss_config {
+	u8 hash_alg;
+	u8 *hash_key;
+	u32 *hash_lut;
+};
+
 /* Parameters for allocating resources for tx queues */
 struct gve_tx_alloc_rings_cfg {
 	struct gve_queue_config *qcfg;
@@ -784,6 +790,8 @@ struct gve_priv {
 	u32 adminq_verify_driver_compatibility_cnt;
 	u32 adminq_query_flow_rules_cnt;
 	u32 adminq_cfg_flow_rule_cnt;
+	u32 adminq_cfg_rss_cnt;
+	u32 adminq_query_rss_cnt;
 
 	/* Global stats */
 	u32 interface_up_cnt; /* count of times interface turned up since last reset */
diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
index b0b7ef8a47d5..9ed4ab6c2809 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.c
+++ b/drivers/net/ethernet/google/gve/gve_adminq.c
@@ -309,6 +309,8 @@ int gve_adminq_alloc(struct device *dev, struct gve_priv *priv)
 	priv->adminq_get_ptype_map_cnt = 0;
 	priv->adminq_query_flow_rules_cnt = 0;
 	priv->adminq_cfg_flow_rule_cnt = 0;
+	priv->adminq_cfg_rss_cnt = 0;
+	priv->adminq_query_rss_cnt = 0;
 
 	/* Setup Admin queue with the device */
 	if (priv->pdev->revision < 0x1) {
@@ -554,6 +556,12 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
 	case GVE_ADMINQ_CONFIGURE_FLOW_RULE:
 		priv->adminq_cfg_flow_rule_cnt++;
 		break;
+	case GVE_ADMINQ_CONFIGURE_RSS:
+		priv->adminq_cfg_rss_cnt++;
+		break;
+	case GVE_ADMINQ_QUERY_RSS:
+		priv->adminq_query_rss_cnt++;
+		break;
 	default:
 		dev_err(&priv->pdev->dev, "unknown AQ command opcode %d\n", opcode);
 	}
@@ -1280,6 +1288,69 @@ int gve_adminq_reset_flow_rules(struct gve_priv *priv)
 	return gve_adminq_configure_flow_rule(priv, &flow_rule_cmd);
 }
 
+int gve_adminq_configure_rss(struct gve_priv *priv, struct gve_rss_config *rss_config)
+{
+	dma_addr_t lut_bus = 0, key_bus = 0;
+	u16 key_size = 0, lut_size = 0;
+	union gve_adminq_command cmd;
+	__be32 *lut = NULL;
+	u8 *key = NULL;
+	int err = 0;
+	u16 i;
+
+	if (rss_config->hash_lut) {
+		lut_size = priv->rss_lut_size;
+		lut = dma_alloc_coherent(&priv->pdev->dev,
+					 lut_size * sizeof(*lut),
+					 &lut_bus, GFP_KERNEL);
+		if (!lut)
+			return -ENOMEM;
+
+		for (i = 0; i < priv->rss_lut_size; i++)
+			lut[i] = cpu_to_be32(rss_config->hash_lut[i]);
+	}
+
+	if (rss_config->hash_key) {
+		key_size = priv->rss_key_size;
+		key = dma_alloc_coherent(&priv->pdev->dev,
+					 key_size * sizeof(*key),
+					 &key_bus, GFP_KERNEL);
+		if (!key) {
+			err = -ENOMEM;
+			goto out;
+		}
+
+		memcpy(key, rss_config->hash_key, priv->rss_key_size * sizeof(*key));
+	}
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = cpu_to_be32(GVE_ADMINQ_CONFIGURE_RSS);
+	cmd.configure_rss = (struct gve_adminq_configure_rss) {
+		.hash_types = cpu_to_be16(BIT(GVE_RSS_HASH_TCPV4) |
+					  BIT(GVE_RSS_HASH_UDPV4) |
+					  BIT(GVE_RSS_HASH_TCPV6) |
+					  BIT(GVE_RSS_HASH_UDPV6)),
+		.hash_alg = rss_config->hash_alg,
+		.hash_key_size = cpu_to_be16(key_size),
+		.hash_lut_size = cpu_to_be16(lut_size),
+		.hash_key_addr = cpu_to_be64(key_bus),
+		.hash_lut_addr = cpu_to_be64(lut_bus),
+	};
+
+	err = gve_adminq_execute_cmd(priv, &cmd);
+
+out:
+	if (lut)
+		dma_free_coherent(&priv->pdev->dev,
+				  priv->rss_lut_size * sizeof(*lut),
+				  lut, lut_bus);
+	if (key)
+		dma_free_coherent(&priv->pdev->dev,
+				  priv->rss_key_size * sizeof(*key),
+				  key, key_bus);
+	return err;
+}
+
 /* In the dma memory that the driver allocated for the device to query the flow rules, the device
  * will first write it with a struct of gve_query_flow_rules_descriptor. Next to it, the device
  * will write an array of rules or rule ids with the count that specified in the descriptor.
@@ -1357,3 +1428,63 @@ int gve_adminq_query_flow_rules(struct gve_priv *priv, u16 query_opcode, u32 sta
 	dma_pool_free(priv->adminq_pool, descriptor, descriptor_bus);
 	return err;
 }
+
+static int gve_adminq_process_rss_query(struct gve_priv *priv,
+					struct gve_query_rss_descriptor *descriptor,
+					struct gve_rss_config *rss_config)
+{
+	u16 hash_key_length, hash_lut_length;
+	u32 total_memory_length;
+	void *rss_info_addr;
+	__be32 *lut;
+	u16 i;
+
+	total_memory_length = be32_to_cpu(descriptor->total_length);
+	hash_key_length = priv->rss_key_size * sizeof(*rss_config->hash_key);
+	hash_lut_length = priv->rss_lut_size * sizeof(*rss_config->hash_lut);
+
+	if (sizeof(*descriptor) + hash_key_length + hash_lut_length != total_memory_length) {
+		dev_err(&priv->dev->dev,
+			"rss query desc from device has invalid length parameter.\n");
+		return -EINVAL;
+	}
+
+	rss_config->hash_alg = descriptor->hash_alg;
+	rss_info_addr = (void *)(descriptor + 1);
+	memcpy(rss_config->hash_key, rss_info_addr, hash_key_length);
+
+	rss_info_addr += hash_key_length;
+	lut = (__be32 *)rss_info_addr;
+	for (i = 0; i < priv->rss_lut_size; i++)
+		rss_config->hash_lut[i] = be32_to_cpu(lut[i]);
+
+	return 0;
+}
+
+int gve_adminq_query_rss_config(struct gve_priv *priv, struct gve_rss_config *rss_config)
+{
+	struct gve_query_rss_descriptor *descriptor;
+	union gve_adminq_command cmd;
+	dma_addr_t descriptor_bus;
+	int err = 0;
+
+	descriptor = dma_pool_alloc(priv->adminq_pool, GFP_KERNEL, &descriptor_bus);
+	if (!descriptor)
+		return -ENOMEM;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = cpu_to_be32(GVE_ADMINQ_QUERY_RSS);
+	cmd.query_rss = (struct gve_adminq_query_rss) {
+		.available_length = cpu_to_be64(GVE_ADMINQ_BUFFER_SIZE),
+		.rss_descriptor_addr = cpu_to_be64(descriptor_bus),
+	};
+	err = gve_adminq_execute_cmd(priv, &cmd);
+	if (err)
+		goto out;
+
+	err = gve_adminq_process_rss_query(priv, descriptor, rss_config);
+
+out:
+	dma_pool_free(priv->adminq_pool, descriptor, descriptor_bus);
+	return err;
+}
diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
index 7d9ef9a12fef..a4390b60f9fc 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.h
+++ b/drivers/net/ethernet/google/gve/gve_adminq.h
@@ -20,12 +20,14 @@ enum gve_adminq_opcodes {
 	GVE_ADMINQ_DESTROY_TX_QUEUE		= 0x7,
 	GVE_ADMINQ_DESTROY_RX_QUEUE		= 0x8,
 	GVE_ADMINQ_DECONFIGURE_DEVICE_RESOURCES	= 0x9,
+	GVE_ADMINQ_CONFIGURE_RSS		= 0xA,
 	GVE_ADMINQ_SET_DRIVER_PARAMETER		= 0xB,
 	GVE_ADMINQ_REPORT_STATS			= 0xC,
 	GVE_ADMINQ_REPORT_LINK_SPEED		= 0xD,
 	GVE_ADMINQ_GET_PTYPE_MAP		= 0xE,
 	GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY	= 0xF,
 	GVE_ADMINQ_QUERY_FLOW_RULES		= 0x10,
+	GVE_ADMINQ_QUERY_RSS			= 0x12,
 
 	/* For commands that are larger than 56 bytes */
 	GVE_ADMINQ_EXTENDED_COMMAND		= 0xFF,
@@ -522,6 +524,44 @@ struct gve_adminq_query_flow_rules {
 
 static_assert(sizeof(struct gve_adminq_query_flow_rules) == 24);
 
+enum gve_rss_hash_type {
+	GVE_RSS_HASH_IPV4,
+	GVE_RSS_HASH_TCPV4,
+	GVE_RSS_HASH_IPV6,
+	GVE_RSS_HASH_IPV6_EX,
+	GVE_RSS_HASH_TCPV6,
+	GVE_RSS_HASH_TCPV6_EX,
+	GVE_RSS_HASH_UDPV4,
+	GVE_RSS_HASH_UDPV6,
+	GVE_RSS_HASH_UDPV6_EX,
+};
+
+struct gve_adminq_configure_rss {
+	__be16 hash_types;
+	u8 hash_alg;
+	u8 reserved;
+	__be16 hash_key_size;
+	__be16 hash_lut_size;
+	__be64 hash_key_addr;
+	__be64 hash_lut_addr;
+};
+
+static_assert(sizeof(struct gve_adminq_configure_rss) == 24);
+
+struct gve_query_rss_descriptor {
+	__be32 total_length;
+	__be16 hash_types;
+	u8 hash_alg;
+	u8 reserved;
+};
+
+struct gve_adminq_query_rss {
+	__be64 available_length;
+	__be64 rss_descriptor_addr;
+};
+
+static_assert(sizeof(struct gve_adminq_query_rss) == 16);
+
 union gve_adminq_command {
 	struct {
 		__be32 opcode;
@@ -543,6 +583,8 @@ union gve_adminq_command {
 			struct gve_adminq_verify_driver_compatibility
 						verify_driver_compatibility;
 			struct gve_adminq_query_flow_rules query_flow_rules;
+			struct gve_adminq_configure_rss configure_rss;
+			struct gve_adminq_query_rss query_rss;
 			struct gve_adminq_extended_command extended_command;
 		};
 	};
@@ -581,6 +623,8 @@ int gve_adminq_add_flow_rule(struct gve_priv *priv, struct gve_adminq_flow_rule
 int gve_adminq_del_flow_rule(struct gve_priv *priv, u32 loc);
 int gve_adminq_reset_flow_rules(struct gve_priv *priv);
 int gve_adminq_query_flow_rules(struct gve_priv *priv, u16 query_opcode, u32 starting_loc);
+int gve_adminq_configure_rss(struct gve_priv *priv, struct gve_rss_config *config);
+int gve_adminq_query_rss_config(struct gve_priv *priv, struct gve_rss_config *config);
 
 struct gve_ptype_lut;
 int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv,
diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
index 3480ff5c7ed6..0148274c0bbb 100644
--- a/drivers/net/ethernet/google/gve/gve_ethtool.c
+++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
@@ -75,7 +75,8 @@ static const char gve_gstrings_adminq_stats[][ETH_GSTRING_LEN] = {
 	"adminq_destroy_tx_queue_cnt", "adminq_destroy_rx_queue_cnt",
 	"adminq_dcfg_device_resources_cnt", "adminq_set_driver_parameter_cnt",
 	"adminq_report_stats_cnt", "adminq_report_link_speed_cnt", "adminq_get_ptype_map_cnt",
-	"adminq_query_flow_rules", "adminq_cfg_flow_rule",
+	"adminq_query_flow_rules", "adminq_cfg_flow_rule", "adminq_cfg_rss_cnt",
+	"adminq_query_rss_cnt",
 };
 
 static const char gve_gstrings_priv_flags[][ETH_GSTRING_LEN] = {
@@ -453,6 +454,8 @@ gve_get_ethtool_stats(struct net_device *netdev,
 	data[i++] = priv->adminq_get_ptype_map_cnt;
 	data[i++] = priv->adminq_query_flow_rules_cnt;
 	data[i++] = priv->adminq_cfg_flow_rule_cnt;
+	data[i++] = priv->adminq_cfg_rss_cnt;
+	data[i++] = priv->adminq_query_rss_cnt;
 }
 
 static void gve_get_channels(struct net_device *netdev,
@@ -838,6 +841,115 @@ static int gve_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd, u
 	return err;
 }
 
+static u32 gve_get_rxfh_key_size(struct net_device *netdev)
+{
+	struct gve_priv *priv = netdev_priv(netdev);
+
+	return priv->rss_key_size;
+}
+
+static u32 gve_get_rxfh_indir_size(struct net_device *netdev)
+{
+	struct gve_priv *priv = netdev_priv(netdev);
+
+	return priv->rss_lut_size;
+}
+
+static int gve_get_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh)
+{
+	struct gve_priv *priv = netdev_priv(netdev);
+	struct gve_rss_config rss_config = {0};
+	u32 *indir = rxfh->indir;
+	u8 *hfunc = &rxfh->hfunc;
+	u8 *key = rxfh->key;
+	int err = 0;
+
+	if (!priv->rss_key_size || !priv->rss_lut_size)
+		return -EOPNOTSUPP;
+
+	rss_config.hash_key = kvcalloc(priv->rss_key_size,
+				       sizeof(*rss_config.hash_key), GFP_KERNEL);
+	if (!rss_config.hash_key)
+		return -ENOMEM;
+
+	rss_config.hash_lut = kvcalloc(priv->rss_lut_size,
+				       sizeof(*rss_config.hash_lut), GFP_KERNEL);
+	if (!rss_config.hash_lut) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	err = gve_adminq_query_rss_config(priv, &rss_config);
+	if (err)
+		goto out;
+
+	if (hfunc)
+		*hfunc = rss_config.hash_alg;
+
+	if (key)
+		memcpy(key, rss_config.hash_key, priv->rss_key_size);
+
+	if (indir)
+		memcpy(indir, rss_config.hash_lut,
+		       priv->rss_lut_size * sizeof(*rss_config.hash_lut));
+
+out:
+	kvfree(rss_config.hash_lut);
+	kvfree(rss_config.hash_key);
+	return err;
+}
+
+static int gve_set_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh,
+			struct netlink_ext_ack *extack)
+{
+	struct gve_priv *priv = netdev_priv(netdev);
+	struct gve_rss_config rss_config = {0};
+	u32 *indir = rxfh->indir;
+	u8 hfunc = rxfh->hfunc;
+	u8 *key = rxfh->key;
+	int err = 0;
+
+	if (!priv->rss_key_size || !priv->rss_lut_size)
+		return -EOPNOTSUPP;
+
+	switch (hfunc) {
+	case ETH_RSS_HASH_NO_CHANGE:
+		break;
+	case ETH_RSS_HASH_TOP:
+		rss_config.hash_alg = ETH_RSS_HASH_TOP;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	if (key) {
+		rss_config.hash_key = kvcalloc(priv->rss_key_size,
+					       sizeof(*rss_config.hash_key), GFP_KERNEL);
+		if (!rss_config.hash_key)
+			return -ENOMEM;
+
+		memcpy(rss_config.hash_key, key, priv->rss_key_size * sizeof(*key));
+	}
+
+	if (indir) {
+		rss_config.hash_lut = kvcalloc(priv->rss_lut_size,
+					       sizeof(*rss_config.hash_lut), GFP_KERNEL);
+		if (!rss_config.hash_lut) {
+			err = -ENOMEM;
+			goto out;
+		}
+
+		memcpy(rss_config.hash_lut, indir, priv->rss_lut_size * sizeof(*indir));
+	}
+
+	err = gve_adminq_configure_rss(priv, &rss_config);
+
+out:
+	kvfree(rss_config.hash_lut);
+	kvfree(rss_config.hash_key);
+	return err;
+}
+
 const struct ethtool_ops gve_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS,
 	.supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT,
@@ -851,6 +963,10 @@ const struct ethtool_ops gve_ethtool_ops = {
 	.get_channels = gve_get_channels,
 	.set_rxnfc = gve_set_rxnfc,
 	.get_rxnfc = gve_get_rxnfc,
+	.get_rxfh_indir_size = gve_get_rxfh_indir_size,
+	.get_rxfh_key_size = gve_get_rxfh_key_size,
+	.get_rxfh = gve_get_rxfh,
+	.set_rxfh = gve_set_rxfh,
 	.get_link = ethtool_op_get_link,
 	.get_coalesce = gve_get_coalesce,
 	.set_coalesce = gve_set_coalesce,
-- 
2.46.0.rc1.232.g9752f9e123-goog

Re: [PATCH net-next 1/2] gve: Add RSS device option

[Jakub Kicinski]

On Thu,  1 Aug 2024 18:28:33 -0700 Praveen Kaligineedi wrote:
> @@ -45,7 +45,8 @@ void gve_parse_device_option(struct gve_priv *priv,
>  			     struct gve_device_option_dqo_qpl **dev_op_dqo_qpl,
>  			     struct gve_device_option_buffer_sizes **dev_op_buffer_sizes,
>  			     struct gve_device_option_flow_steering **dev_op_flow_steering,
> -			     struct gve_device_option_modify_ring **dev_op_modify_ring)
> +			     struct gve_device_option_modify_ring **dev_op_modify_ring,
> +			     struct gve_device_option_rss_config **dev_op_rss_config)
>  {
>  	u32 req_feat_mask = be32_to_cpu(option->required_features_mask);
>  	u16 option_length = be16_to_cpu(option->option_length);


> @@ -867,6 +887,8 @@ static void gve_enable_supported_features(struct gve_priv *priv,
>  					  *dev_op_buffer_sizes,
>  					  const struct gve_device_option_flow_steering
>  					  *dev_op_flow_steering,
> +					  const struct gve_device_option_rss_config
> +					  *dev_op_rss_config,
>  					  const struct gve_device_option_modify_ring
>  					  *dev_op_modify_ring)

Any reason these two functions order the arguments differently?

Re: [PATCH net-next 2/2] gve: Add RSS adminq commands and ethtool support

[Jakub Kicinski]

On Thu,  1 Aug 2024 18:28:34 -0700 Praveen Kaligineedi wrote:
> +static int gve_set_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh,
> +			struct netlink_ext_ack *extack)
> +{
> +	struct gve_priv *priv = netdev_priv(netdev);
> +	struct gve_rss_config rss_config = {0};

I never remember the exact rules, are you sure this is the one that 
is guaranteed per standard to zero all the fields?

> +	u32 *indir = rxfh->indir;
> +	u8 hfunc = rxfh->hfunc;
> +	u8 *key = rxfh->key;
> +	int err = 0;
> +
> +	if (!priv->rss_key_size || !priv->rss_lut_size)
> +		return -EOPNOTSUPP;
> +
> +	switch (hfunc) {
> +	case ETH_RSS_HASH_NO_CHANGE:
> +		break;
> +	case ETH_RSS_HASH_TOP:
> +		rss_config.hash_alg = ETH_RSS_HASH_TOP;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (key) {
> +		rss_config.hash_key = kvcalloc(priv->rss_key_size,
> +					       sizeof(*rss_config.hash_key), GFP_KERNEL);

key is a bitstream...
IOW this code is like allocating a string with 
	calloc(length, sizeof(char)) 
:S

But what is the point of the allocations in this function in the first
place? You kvcalloc here, copy, call gve_adminq_configure_rss()
which dma_alloc_coherent() and copies into that, again.

> +		if (!rss_config.hash_key)
> +			return -ENOMEM;
> +
> +		memcpy(rss_config.hash_key, key, priv->rss_key_size * sizeof(*key));
> +	}
> +
> +	if (indir) {
> +		rss_config.hash_lut = kvcalloc(priv->rss_lut_size,
> +					       sizeof(*rss_config.hash_lut), GFP_KERNEL);
> +		if (!rss_config.hash_lut) {
> +			err = -ENOMEM;
> +			goto out;
> +		}
> +
> +		memcpy(rss_config.hash_lut, indir, priv->rss_lut_size * sizeof(*indir));
> +	}
> +
> +	err = gve_adminq_configure_rss(priv, &rss_config);
> +
> +out:
> +	kvfree(rss_config.hash_lut);
> +	kvfree(rss_config.hash_key);
> +	return err;
> +}

Re: [PATCH net-next 2/2] gve: Add RSS adminq commands and ethtool support

[Ziwei Xiao]

On Fri, Aug 2, 2024 at 4:21 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu,  1 Aug 2024 18:28:34 -0700 Praveen Kaligineedi wrote:
> > +static int gve_set_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh,
> > +                     struct netlink_ext_ack *extack)
> > +{
> > +     struct gve_priv *priv = netdev_priv(netdev);
> > +     struct gve_rss_config rss_config = {0};
>
> I never remember the exact rules, are you sure this is the one that
> is guaranteed per standard to zero all the fields?
>
I didn't find specific rules for standards older than C99, but looks
like this should be supported for all standards.
https://stackoverflow.com/a/63356270. Also there are multiple drivers
using {0} to zero the struct now, so I think it should be good?

> > +     u32 *indir = rxfh->indir;
> > +     u8 hfunc = rxfh->hfunc;
> > +     u8 *key = rxfh->key;
> > +     int err = 0;
> > +
> > +     if (!priv->rss_key_size || !priv->rss_lut_size)
> > +             return -EOPNOTSUPP;
> > +
> > +     switch (hfunc) {
> > +     case ETH_RSS_HASH_NO_CHANGE:
> > +             break;
> > +     case ETH_RSS_HASH_TOP:
> > +             rss_config.hash_alg = ETH_RSS_HASH_TOP;
> > +             break;
> > +     default:
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     if (key) {
> > +             rss_config.hash_key = kvcalloc(priv->rss_key_size,
> > +                                            sizeof(*rss_config.hash_key), GFP_KERNEL);
>
> key is a bitstream...
> IOW this code is like allocating a string with
>         calloc(length, sizeof(char))
> :S
>
> But what is the point of the allocations in this function in the first
> place? You kvcalloc here, copy, call gve_adminq_configure_rss()
> which dma_alloc_coherent() and copies into that, again.
>
Thanks for pointing it out. I will send a V2 to avoid the extra copy.

Re: [PATCH net-next 1/2] gve: Add RSS device option

[Ziwei Xiao]

On Fri, Aug 2, 2024 at 4:13 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu,  1 Aug 2024 18:28:33 -0700 Praveen Kaligineedi wrote:
> > @@ -45,7 +45,8 @@ void gve_parse_device_option(struct gve_priv *priv,
> >                            struct gve_device_option_dqo_qpl **dev_op_dqo_qpl,
> >                            struct gve_device_option_buffer_sizes **dev_op_buffer_sizes,
> >                            struct gve_device_option_flow_steering **dev_op_flow_steering,
> > -                          struct gve_device_option_modify_ring **dev_op_modify_ring)
> > +                          struct gve_device_option_modify_ring **dev_op_modify_ring,
> > +                          struct gve_device_option_rss_config **dev_op_rss_config)
> >  {
> >       u32 req_feat_mask = be32_to_cpu(option->required_features_mask);
> >       u16 option_length = be16_to_cpu(option->option_length);
>
>
> > @@ -867,6 +887,8 @@ static void gve_enable_supported_features(struct gve_priv *priv,
> >                                         *dev_op_buffer_sizes,
> >                                         const struct gve_device_option_flow_steering
> >                                         *dev_op_flow_steering,
> > +                                       const struct gve_device_option_rss_config
> > +                                       *dev_op_rss_config,
> >                                         const struct gve_device_option_modify_ring
> >                                         *dev_op_modify_ring)
>
> Any reason these two functions order the arguments differently?
Will update it in v2 so that the arguments order will be the same for
those functions. Thanks!