Re: [dpdk-dev] [PATCH v4 1/5] net/virtio: add initial RSS support

["Xia, Chenbo" <chenbo.xia@intel.com>]

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, October 4, 2021 10:30 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
> david.marchand@redhat.com; andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh
> <ferruh.yigit@intel.com>; michaelba@nvidia.com; viacheslavo@nvidia.com; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Cc: nelio.laranjeiro@6wind.com; yvugenfi@redhat.com; ybendito@redhat.com;
> Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v4 1/5] net/virtio: add initial RSS support
> 
> Provide the capability to update the hash key, hash types
> and RETA table on the fly (without needing to stop/start
> the device). However, the key length and the number of RETA
> entries are fixed to 40B and 128 entries respectively. This
> is done in order to simplify the design, but may be
> revisited later as the Virtio spec provides this
> flexibility.
> 
> Note that only VIRTIO_NET_F_RSS support is implemented,
> VIRTIO_NET_F_HASH_REPORT, which would enable reporting the
> packet RSS hash calculated by the device into mbuf.rss, is
> not yet supported.
> 
> Regarding the default RSS configuration, it has been
> chosen to use the default Intel ixgbe key as default key,
> and default RETA is a simple modulo between the hash and
> the number of Rx queues.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---

[...]

> +static int
> +virtio_dev_get_rss_config(struct virtio_hw *hw, uint32_t *rss_hash_types)
> +{
> +	struct virtio_net_config local_config;
> +	struct virtio_net_config *config = &local_config;
> +
> +	virtio_read_dev_config(hw,
> +			offsetof(struct virtio_net_config, rss_max_key_size),
> +			&config->rss_max_key_size,
> +			sizeof(config->rss_max_key_size));
> +	if (config->rss_max_key_size < VIRTIO_NET_RSS_KEY_SIZE) {
> +		PMD_INIT_LOG(ERR, "Invalid device RSS max key size (%u)",
> +				config->rss_max_key_size);
> +		return -EINVAL;
> +	}
> +
> +	virtio_read_dev_config(hw,
> +			offsetof(struct virtio_net_config,
> +				rss_max_indirection_table_length),
> +			&config->rss_max_indirection_table_length,
> +			sizeof(config->rss_max_indirection_table_length));
> +	if (config->rss_max_indirection_table_length < VIRTIO_NET_RSS_RETA_SIZE)
> {
> +		PMD_INIT_LOG(ERR, "Invalid device RSS max reta size (%u)",
> +				config->rss_max_indirection_table_length);
> +		return -EINVAL;
> +	}
> +
> +	virtio_read_dev_config(hw,
> +			offsetof(struct virtio_net_config, supported_hash_types),
> +			&config->supported_hash_types,
> +			sizeof(config->supported_hash_types));
> +	if ((config->supported_hash_types & VIRTIO_NET_HASH_TYPE_MASK) == 0) {
> +		PMD_INIT_LOG(ERR, "Invalid device RSS hash types (%u)",

Better use 0x%x to be more user friendly

> +				config->supported_hash_types);
> +		return -EINVAL;
> +	}
> +
> +	*rss_hash_types = config->supported_hash_types &
> VIRTIO_NET_HASH_TYPE_MASK;
> +
> +	PMD_INIT_LOG(DEBUG, "Device RSS config:");
> +	PMD_INIT_LOG(DEBUG, "\t-Max key size: %u", config->rss_max_key_size);
> +	PMD_INIT_LOG(DEBUG, "\t-Max reta size: %u", config-
> >rss_max_indirection_table_length);
> +	PMD_INIT_LOG(DEBUG, "\t-Supported hash types: 0x%x", *rss_hash_types);
> +
> +	return 0;
> +}
> +
> +static int
> +virtio_dev_rss_hash_update(struct rte_eth_dev *dev,
> +		struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +	uint16_t nb_queues;
> +
> +	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return -ENOTSUP;
> +
> +	if (rss_conf->rss_hf &
> ~virtio_to_ethdev_rss_offloads(VIRTIO_NET_HASH_TYPE_MASK))
> +		return -EINVAL;
> +
> +	hw->rss_hash_types = ethdev_to_virtio_rss_offloads(rss_conf->rss_hf);
> +
> +	if (rss_conf->rss_key && rss_conf->rss_key_len) {
> +		if (rss_conf->rss_key_len != VIRTIO_NET_RSS_KEY_SIZE) {
> +			PMD_INIT_LOG(ERR, "Driver only supports %u RSS key length",
> +					VIRTIO_NET_RSS_KEY_SIZE);
> +			return -EINVAL;
> +		}
> +		memcpy(hw->rss_key, rss_conf->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
> +	}
> +
> +	nb_queues = RTE_MAX(dev->data->nb_rx_queues, dev->data->nb_tx_queues);
> +	return virtio_set_multiple_queues_rss(dev, nb_queues);
> +}
> +
> +static int
> +virtio_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> +		struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +
> +	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return -ENOTSUP;
> +
> +	if (rss_conf->rss_key && rss_conf->rss_key_len >=
> VIRTIO_NET_RSS_KEY_SIZE)
> +		memcpy(rss_conf->rss_key, hw->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
> +	rss_conf->rss_key_len = VIRTIO_NET_RSS_KEY_SIZE;
> +	rss_conf->rss_hf = virtio_to_ethdev_rss_offloads(hw->rss_hash_types);
> +
> +	return 0;
> +}
> +
> +static int virtio_dev_rss_reta_update(struct rte_eth_dev *dev,
> +			 struct rte_eth_rss_reta_entry64 *reta_conf,
> +			 uint16_t reta_size)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +	uint16_t nb_queues;
> +	int idx, pos, i;
> +
> +	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return -ENOTSUP;
> +
> +	if (reta_size != VIRTIO_NET_RSS_RETA_SIZE)
> +		return -EINVAL;
> +
> +	for (idx = 0, i = 0; i < reta_size; i++) {

idx is calculated every time. No need to do 'idx = 0'.

> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		pos = i % RTE_RETA_GROUP_SIZE;
> +
> +		if (((reta_conf[idx].mask >> pos) & 0x1) == 0)
> +			continue;
> +
> +		hw->rss_reta[i] = reta_conf[idx].reta[pos];
> +	}
> +
> +	nb_queues = RTE_MAX(dev->data->nb_rx_queues, dev->data->nb_tx_queues);
> +	return virtio_set_multiple_queues_rss(dev, nb_queues);
> +}
> +
> +static int virtio_dev_rss_reta_query(struct rte_eth_dev *dev,
> +			 struct rte_eth_rss_reta_entry64 *reta_conf,
> +			 uint16_t reta_size)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +	int idx, i;
> +
> +	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return -ENOTSUP;
> +
> +	if (reta_size != VIRTIO_NET_RSS_RETA_SIZE)
> +		return -EINVAL;
> +
> +	for (idx = 0, i = 0; i < reta_size; i++) {

Ditto

> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		reta_conf[idx].reta[i % RTE_RETA_GROUP_SIZE] = hw->rss_reta[i];
> +	}
> +
> +	return 0;
> +}
> +
> +/*

[...]

> +/**
> + * RSS control
> + *
> + * The RSS feature <todo>
> + */
> +#define VIRTIO_NET_RSS_RETA_SIZE 128
> +#define VIRTIO_NET_RSS_KEY_SIZE       40

Forgot to align the numbers? :)

Thanks,
Chenbo

> +
> +struct virtio_net_ctrl_rss {
> +	uint32_t hash_types;
> +	uint16_t indirection_table_mask;
> +	uint16_t unclassified_queue;
> +	uint16_t indirection_table[VIRTIO_NET_RSS_RETA_SIZE];
> +	uint16_t max_tx_vq;
> +	uint8_t hash_key_length;
> +	uint8_t hash_key_data[VIRTIO_NET_RSS_KEY_SIZE];
> +};
> +
>  /*
>   * Control link announce acknowledgement
>   *
> @@ -292,7 +310,10 @@ struct virtqueue {
> 
>  /* If multiqueue is provided by host, then we suppport it. */
>  #define VIRTIO_NET_CTRL_MQ   4
> +
>  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
> +#define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
> +
>  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
>  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
> 
> --
> 2.31.1