Re: [PATCH v4 13/15] net/zxdh: rss hash config/update, reta update/get

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 18 Dec 2024 17:26:00 +0800
Junlong Wang <wang.junlong1@zte.com.cn> wrote:

> provided rss hash config/update, reta update/get ops.
> 
> Signed-off-by: Junlong Wang <wang.junlong1@zte.com.cn>
> ---
>  doc/guides/nics/features/zxdh.ini  |   3 +
>  doc/guides/nics/zxdh.rst           |   1 +
>  drivers/net/zxdh/zxdh_ethdev.c     |  52 ++++
>  drivers/net/zxdh/zxdh_ethdev.h     |   3 +
>  drivers/net/zxdh/zxdh_ethdev_ops.c | 410 +++++++++++++++++++++++++++++
>  drivers/net/zxdh/zxdh_ethdev_ops.h |  26 ++
>  drivers/net/zxdh/zxdh_msg.h        |  22 ++
>  drivers/net/zxdh/zxdh_tables.c     |  82 ++++++
>  drivers/net/zxdh/zxdh_tables.h     |   7 +
>  9 files changed, 606 insertions(+)
> 

Some suggestions:

> +int
> +zxdh_dev_rss_reta_update(struct rte_eth_dev *dev,
> +			 struct rte_eth_rss_reta_entry64 *reta_conf,
> +			 uint16_t reta_size)
> +{
> +	struct zxdh_hw *hw = dev->data->dev_private;
> +	struct zxdh_msg_info msg = {0};
> +	uint16_t old_reta[RTE_ETH_RSS_RETA_SIZE_256];
> +	uint16_t idx;
> +	uint16_t i;
> +	uint16_t pos;
> +	int ret;
> +
> +	if (reta_size != RTE_ETH_RSS_RETA_SIZE_256) {
> +		PMD_DRV_LOG(ERR, "reta_size is illegal(%u).reta_size should be 256", reta_size);
> +		return -EINVAL;
> +	}
> +	if (!hw->rss_reta) {
> +		hw->rss_reta = rte_zmalloc(NULL, RTE_ETH_RSS_RETA_SIZE_256 * sizeof(uint16_t), 4);

This could be rte_calloc()

...

> +int
> +zxdh_rss_table_set(uint16_t vport, struct zxdh_rss_reta *rss_reta)
> +{
> +	struct zxdh_rss_to_vqid_table rss_vqid = {0};
> +	union zxdh_virport_num vport_num = (union zxdh_virport_num)vport;
> +	int ret = 0;
> +
> +	for (uint16_t i = 0; i < RTE_ETH_RSS_RETA_SIZE_256 / 8; i++) {
> +		for (uint16_t j = 0; j < 8; j++) {
> +		#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +			if (j % 2 == 0)
> +				rss_vqid.vqm_qid[j + 1] =  rss_reta->reta[i * 8 + j];
> +			else
> +				rss_vqid.vqm_qid[j - 1] =  rss_reta->reta[i * 8 + j];
> +		#else
> +			rss_vqid.vqm_qid[j] = rss_init->reta[i * 8 + j];
> +		#endif

Please put #if in first column not indented.
Better yet, use rte_byteorder functions to elimnate #if code
pattern.

> +		}
> +
> +		#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +			rss_vqid.vqm_qid[1] |= 0x8000;
> +		#else
> +			rss_vqid.vqm_qid[0] |= 0x8000;
> +		#endif