[PATCH v2 09/14] net/idpf: add support for RSS

[Andrew Rybchenko]

On 9/5/22 13:58, Junfeng Guo wrote:
> Add RSS support.
> 
> Signed-off-by: Beilei Xing <beilei.xing at intel.com>
> Signed-off-by: Junfeng Guo <junfeng.guo at intel.com>
> ---
>   drivers/net/idpf/idpf_ethdev.c | 118 ++++++++++++++++++++++++++++++++-
>   drivers/net/idpf/idpf_ethdev.h |  17 +++++
>   drivers/net/idpf/idpf_vchnl.c  |  96 +++++++++++++++++++++++++++
>   3 files changed, 229 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/idpf/idpf_ethdev.c b/drivers/net/idpf/idpf_ethdev.c
> index 6310745684..b1e2ca21ca 100644
> --- a/drivers/net/idpf/idpf_ethdev.c
> +++ b/drivers/net/idpf/idpf_ethdev.c
> @@ -87,6 +87,7 @@ idpf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>   	dev_info->max_mtu = dev_info->max_rx_pktlen - IDPF_ETH_OVERHEAD;
>   	dev_info->min_mtu = RTE_ETHER_MIN_MTU;
>   
> +	dev_info->flow_type_rss_offloads = IDPF_RSS_OFFLOAD_ALL;
>   	dev_info->max_mac_addrs = IDPF_NUM_MACADDR_MAX;
>   	dev_info->dev_capa = RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP |
>   		RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP;
> @@ -319,9 +320,116 @@ idpf_init_vport(struct rte_eth_dev *dev)
>   }
>   
>   static int
> -idpf_dev_configure(__rte_unused struct rte_eth_dev *dev)
> +idpf_config_rss(struct idpf_vport *vport)
>   {
> -	return 0;
> +	int ret;
> +
> +	ret = idpf_set_rss_key(vport);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to configure RSS key");
> +		return ret;
> +	}
> +
> +	ret = idpf_set_rss_lut(vport);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to configure RSS lut");
> +		return ret;
> +	}
> +
> +	ret = idpf_set_rss_hash(vport);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to configure RSS hash");
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int
> +idpf_init_rss(struct idpf_vport *vport)
> +{
> +	struct rte_eth_rss_conf *rss_conf;
> +	uint16_t i, nb_q, lut_size;
> +	int ret = 0;
> +
> +	rss_conf = &vport->dev_data->dev_conf.rx_adv_conf.rss_conf;
> +	nb_q = vport->dev_data->nb_rx_queues;
> +
> +	vport->rss_key = (uint8_t *)rte_zmalloc("rss_key",
> +					     vport->rss_key_size, 0);
> +	if (!vport->rss_key) {
> +		PMD_INIT_LOG(ERR, "Failed to allocate RSS key");
> +		ret = -ENOMEM;
> +		goto err_key;
> +	}
> +
> +	lut_size = vport->rss_lut_size;
> +	vport->rss_lut = (uint32_t *)rte_zmalloc("rss_lut",
> +					      sizeof(uint32_t) * lut_size, 0);
> +	if (!vport->rss_lut) {
> +		PMD_INIT_LOG(ERR, "Failed to allocate RSS lut");
> +		ret = -ENOMEM;
> +		goto err_lut;
> +	}
> +
> +	if (!rss_conf->rss_key) {
> +		for (i = 0; i < vport->rss_key_size; i++)
> +			vport->rss_key[i] = (uint8_t)rte_rand();

IMHO it is a bad idea. Random key could result in very
bad distribution of the traffic.

> +	} else {
> +		rte_memcpy(vport->rss_key, rss_conf->rss_key,
> +			   RTE_MIN(rss_conf->rss_key_len,
> +				   vport->rss_key_size));

It looks like rss_key_len from rss_conf is saved nowhere.
How do you know which bits of the RSS key are really valid/
initialized.

> +	}
> +
> +	for (i = 0; i < lut_size; i++)
> +		vport->rss_lut[i] = i % nb_q;
> +
> +	vport->rss_hf = IECM_DEFAULT_RSS_HASH_EXPANDED;

Hm, what about rss_conf->rss_hf?

> +
> +	ret = idpf_config_rss(vport);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to configure RSS");
> +		goto err_cfg;
> +	}
> +
> +	return ret;
> +
> +err_cfg:
> +	rte_free(vport->rss_lut);
> +	vport->rss_lut = NULL;
> +err_lut:
> +	rte_free(vport->rss_key);
> +	vport->rss_key = NULL;
> +err_key:
> +	return ret;
> +}
> +
> +static int
> +idpf_dev_configure(struct rte_eth_dev *dev)
> +{
> +	struct idpf_vport *vport = dev->data->dev_private;
> +	struct idpf_adapter *adapter = vport->adapter;
> +	int ret = 0;
> +
> +	if (dev->data->nb_tx_queues > IDPF_DEFAULT_TXQ_NUM ||
> +	    dev->data->nb_rx_queues > IDPF_DEFAULT_RXQ_NUM) {
> +		PMD_INIT_LOG(ERR, "Invalid queue number.");
> +		return -EINVAL;
> +	}

Above is checked on ethdev level if you report maximums
correctly.

> +
> +	if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG)
> +		dev->data->dev_conf.rxmode.offloads |=
> +			RTE_ETH_RX_OFFLOAD_RSS_HASH;

It looks wrong. Offload configuration is a user choice.

> +
> +	if (adapter->caps->rss_caps) {
> +		ret = idpf_init_rss(vport);
> +		if (ret) {
> +			PMD_INIT_LOG(ERR, "Failed to init rss");
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
>   }
>   
>   static int

[snip]