Re: [PATCH v11 07/13] net/sxe: support rss offload

[Ivan Malov <ivan.malov@arknetworks.am>]

Hi,

(please see below)

On Fri, 25 Jul 2025, Jie Liu wrote:

> Support rss offload.
>
> Signed-off-by: Jie Liu <liujie5@linkdatatechnology.com>
> ---
> drivers/net/sxe/base/sxe_offload_common.c |  11 +-
> drivers/net/sxe/pf/sxe_offload.c          | 299 ++++++++++++++++++++++
> drivers/net/sxe/pf/sxe_offload.h          |  33 +++
> 3 files changed, 338 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/sxe/base/sxe_offload_common.c b/drivers/net/sxe/base/sxe_offload_common.c
> index 9c4ffd3b86..780681a994 100644
> --- a/drivers/net/sxe/base/sxe_offload_common.c
> +++ b/drivers/net/sxe/base/sxe_offload_common.c
> @@ -22,15 +22,16 @@ u64 __sxe_rx_port_offload_capa_get(struct rte_eth_dev *dev)
> 	u64 rx_offload_capa;
>
> 	rx_offload_capa = RTE_ETH_RX_OFFLOAD_IPV4_CKSUM  |
> -		   RTE_ETH_RX_OFFLOAD_UDP_CKSUM   |
> -		   RTE_ETH_RX_OFFLOAD_TCP_CKSUM   |
> -		   RTE_ETH_RX_OFFLOAD_KEEP_CRC	|

Why not fix the indentation in one of the original/previous patch, to avoid
these changes?

> +			RTE_ETH_RX_OFFLOAD_UDP_CKSUM   |
> +			RTE_ETH_RX_OFFLOAD_TCP_CKSUM   |
> +			RTE_ETH_RX_OFFLOAD_KEEP_CRC	|
> #ifdef DEV_RX_JUMBO_FRAME
> -		   DEV_RX_OFFLOAD_JUMBO_FRAME |
> +			DEV_RX_OFFLOAD_JUMBO_FRAME |
> #endif
> 			RTE_ETH_RX_OFFLOAD_VLAN_FILTER |
> 			RTE_ETH_RX_OFFLOAD_VLAN_EXTEND |
> -		   RTE_ETH_RX_OFFLOAD_SCATTER;
> +			RTE_ETH_RX_OFFLOAD_SCATTER |
> +			RTE_ETH_RX_OFFLOAD_RSS_HASH;
>
> 	if (!RTE_ETH_DEV_SRIOV(dev).active)
> 		rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_LRO;
> diff --git a/drivers/net/sxe/pf/sxe_offload.c b/drivers/net/sxe/pf/sxe_offload.c
> index 0fa3f4822a..d34cf7ecc8 100644
> --- a/drivers/net/sxe/pf/sxe_offload.c
> +++ b/drivers/net/sxe/pf/sxe_offload.c
> @@ -9,11 +9,26 @@
> #include "sxe_queue_common.h"
> #include "sxe_offload_common.h"
>
> +static u8 rss_sxe_key[40] = {

Const?

> +	0x6D, 0x5A, 0x56, 0xDA, 0x25, 0x5B, 0x0E, 0xC2,
> +	0x41, 0x67, 0x25, 0x3D, 0x43, 0xA3, 0x8F, 0xB0,
> +	0xD0, 0xCA, 0x2B, 0xCB, 0xAE, 0x7B, 0x30, 0xB4,
> +	0x77, 0xCB, 0x2D, 0xA3, 0x80, 0x30, 0xF2, 0x0C,
> +	0x6A, 0x42, 0xB7, 0x3B, 0xBE, 0xAC, 0x01, 0xFA,
> +};
> +
> #define SXE_4_BIT_WIDTH  (CHAR_BIT / 2)
> #define SXE_4_BIT_MASK   RTE_LEN2MASK(SXE_4_BIT_WIDTH, u8)
> #define SXE_8_BIT_WIDTH  CHAR_BIT
> #define SXE_8_BIT_MASK   UINT8_MAX
>
> +#if defined SXE_DPDK_L4_FEATURES && defined SXE_DPDK_FILTER_CTRL
> +u8 *sxe_rss_hash_key_get(void)

Is this unused in the driver?

> +{
> +	return rss_sxe_key;
> +}
> +#endif
> +
> u64 sxe_rx_queue_offload_capa_get(struct rte_eth_dev *dev)
> {
> 	return __sxe_rx_queue_offload_capa_get(dev);
> @@ -35,3 +50,287 @@ u64 sxe_tx_port_offload_capa_get(struct rte_eth_dev *dev)
> {
> 	return __sxe_tx_port_offload_capa_get(dev);
> }
> +
> +void sxe_rss_disable(struct rte_eth_dev *dev)
> +{
> +	struct sxe_adapter *adapter = dev->data->dev_private;
> +	struct sxe_hw *hw = &adapter->hw;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	sxe_hw_rss_cap_switch(hw, false);
> +}
> +
> +void sxe_rss_hash_set(struct sxe_hw *hw,
> +				struct rte_eth_rss_conf *rss_conf)
> +{
> +	u8  *hash_key;
> +	u32 rss_key[SXE_MAX_RSS_KEY_ENTRIES];
> +	u16 i;
> +	u64 rss_hf;
> +	u32 rss_field = 0;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	hash_key = rss_conf->rss_key;
> +	if (hash_key != NULL) {
> +		for (i = 0; i < SXE_MAX_RSS_KEY_ENTRIES; i++) {
> +			rss_key[i]  = hash_key[(i * 4)];
> +			rss_key[i] |= hash_key[(i * 4) + 1] << 8;
> +			rss_key[i] |= hash_key[(i * 4) + 2] << 16;
> +			rss_key[i] |= hash_key[(i * 4) + 3] << 24;

The code does not seem to check 'rss_conf->rss_key_len' to guarantee it is at
least 'SXE_MAX_RSS_KEY_ENTRIES * 4', to avoid reading past the buffer.

> +		}
> +		sxe_hw_rss_key_set_all(hw, rss_key);
> +	}
> +
> +	rss_hf = rss_conf->rss_hf;
> +	if (rss_hf & RTE_ETH_RSS_IPV4)
> +		rss_field |= SXE_MRQC_RSS_FIELD_IPV4;
> +
> +	if (rss_hf & RTE_ETH_RSS_NONFRAG_IPV4_TCP)
> +		rss_field |= SXE_MRQC_RSS_FIELD_IPV4_TCP;
> +
> +	if (rss_hf & RTE_ETH_RSS_IPV6)
> +		rss_field |= SXE_MRQC_RSS_FIELD_IPV6;
> +
> +	if (rss_hf & RTE_ETH_RSS_NONFRAG_IPV6_TCP)
> +		rss_field |= SXE_MRQC_RSS_FIELD_IPV6_TCP;
> +
> +	if (rss_hf & RTE_ETH_RSS_NONFRAG_IPV4_UDP)
> +		rss_field |= SXE_MRQC_RSS_FIELD_IPV4_UDP;
> +
> +	if (rss_hf & RTE_ETH_RSS_NONFRAG_IPV6_UDP)
> +		rss_field |= SXE_MRQC_RSS_FIELD_IPV6_UDP;
> +
> +	sxe_hw_rss_field_set(hw, rss_field);
> +
> +	sxe_hw_rss_cap_switch(hw, true);
> +}
> +
> +void sxe_rss_configure(struct rte_eth_dev *dev)
> +{
> +	struct rte_eth_rss_conf *rss_conf;
> +	struct sxe_adapter *adapter = dev->data->dev_private;
> +	struct sxe_hw *hw = &adapter->hw;
> +	u16 i;
> +	u16 j;
> +	u8  rss_indir_tbl[SXE_MAX_RETA_ENTRIES];
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (!adapter->rss_reta_updated) {
> +		for (i = 0, j = 0; i < SXE_MAX_RETA_ENTRIES; i++, j++) {
> +			if (j == dev->data->nb_rx_queues)
> +				j = 0;
> +
> +			rss_indir_tbl[i] = j;
> +		}

Why not just
     for (i = 0; i < SXE_MAX_RETA_ENTRIES; ++i)
         rss_indir_tbl[i] = i % dev->data->nb_rx_queues;

> +
> +		sxe_hw_rss_redir_tbl_set_all(hw, rss_indir_tbl);
> +	}
> +
> +	rss_conf = &dev->data->dev_conf.rx_adv_conf.rss_conf;
> +	if ((rss_conf->rss_hf & SXE_RSS_OFFLOAD_ALL) == 0) {
> +		PMD_LOG_INFO(INIT, "user rss config match hw supports is 0");

The info message is a little confusing.

> +		sxe_rss_disable(dev);
> +		return;
> +	}
> +
> +	if (rss_conf->rss_key == NULL)
> +		rss_conf->rss_key = rss_sxe_key;

Perhaps also set 'rss_conf->rss_key_len'?

> +
> +	sxe_rss_hash_set(hw, rss_conf);
> +}
> +
> +s32 sxe_rss_reta_update(struct rte_eth_dev *dev,
> +			struct rte_eth_rss_reta_entry64 *reta_conf,
> +			u16 reta_size)
> +{
> +	u16 i;
> +	u8 j, mask;
> +	u32 reta, r;
> +	u16 idx, shift;
> +	struct sxe_adapter *adapter = dev->data->dev_private;
> +	struct rte_eth_dev_data *dev_data = dev->data;
> +	struct sxe_hw *hw = &adapter->hw;
> +	s32 ret = 0;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (!dev_data->dev_started) {
> +		PMD_LOG_ERR(DRV,
> +			"port %d must be started before rss reta update",
> +			 dev_data->port_id);

Why not memorise the table to be applied later on port start? Why deny it?

> +		ret = -EIO;
> +		goto l_end;
> +	}
> +
> +	if (reta_size != RTE_ETH_RSS_RETA_SIZE_128) {
> +		PMD_LOG_ERR(DRV, "The size of hash lookup table configured "
> +			"(%d) doesn't match the number hardware can supported "
> +			"(%d)", reta_size, RTE_ETH_RSS_RETA_SIZE_128);
> +		ret = -EINVAL;
> +		goto l_end;
> +	}
> +
> +	for (i = 0; i < reta_size; i += SXE_4_BIT_WIDTH) {
> +		idx = i / RTE_ETH_RETA_GROUP_SIZE;
> +		shift = i % RTE_ETH_RETA_GROUP_SIZE;
> +		mask = (u8)((reta_conf[idx].mask >> shift) &
> +						SXE_4_BIT_MASK);
> +		if (!mask)
> +			continue;
> +
> +		if (mask == SXE_4_BIT_MASK)
> +			r = 0;
> +		else
> +			r = sxe_hw_rss_redir_tbl_get_by_idx(hw, i);
> +
> +		for (j = 0, reta = 0; j < SXE_4_BIT_WIDTH; j++) {
> +			if (mask & (0x1 << j)) {
> +				reta |= reta_conf[idx].reta[shift + j] <<
> +						(CHAR_BIT * j);
> +			} else {
> +				reta |= r & (SXE_8_BIT_MASK <<
> +					(CHAR_BIT * j));
> +			}
> +		}
> +
> +		sxe_hw_rss_redir_tbl_set_by_idx(hw, i, reta);
> +	}
> +	adapter->rss_reta_updated = true;
> +
> +l_end:
> +	return ret;
> +}
> +
> +s32 sxe_rss_reta_query(struct rte_eth_dev *dev,
> +			 struct rte_eth_rss_reta_entry64 *reta_conf,
> +			 u16 reta_size)
> +{
> +	u16 i;
> +	u8 j, mask;
> +	u32 reta;
> +	u16 idx, shift;
> +	struct sxe_adapter *adapter = dev->data->dev_private;
> +	struct sxe_hw *hw = &adapter->hw;
> +	s32 ret = 0;
> +
> +	PMD_INIT_FUNC_TRACE();
> +	if (reta_size != RTE_ETH_RSS_RETA_SIZE_128) {
> +		PMD_LOG_ERR(DRV, "the size of hash lookup table configured "
> +			"(%d) doesn't match the number hardware can supported "
> +			"(%d)", reta_size, RTE_ETH_RSS_RETA_SIZE_128);
> +		ret = -EINVAL;
> +		goto l_end;
> +	}
> +
> +	for (i = 0; i < reta_size; i += SXE_4_BIT_WIDTH) {
> +		idx = i / RTE_ETH_RETA_GROUP_SIZE;
> +		shift = i % RTE_ETH_RETA_GROUP_SIZE;
> +		mask = (u8)((reta_conf[idx].mask >> shift) &
> +						SXE_4_BIT_MASK);
> +		if (!mask)
> +			continue;
> +
> +		reta = sxe_hw_rss_redir_tbl_get_by_idx(hw, i);
> +		for (j = 0; j < SXE_4_BIT_WIDTH; j++) {
> +			if (mask & (0x1 << j)) {
> +				reta_conf[idx].reta[shift + j] =
> +					((reta >> (CHAR_BIT * j)) &
> +						SXE_8_BIT_MASK);
> +			}
> +		}
> +	}
> +
> +l_end:
> +	return ret;
> +}
> +
> +s32 sxe_rss_hash_update(struct rte_eth_dev *dev,
> +			struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct sxe_adapter *adapter = dev->data->dev_private;
> +	struct sxe_hw *hw = &adapter->hw;
> +	u64 rss_hf;
> +	s32 ret = 0;
> +
> +	rss_hf = (rss_conf->rss_hf & SXE_RSS_OFFLOAD_ALL);

Shan't one throw an error on '(rss_conf->rss_hf & ~SXE_RSS_OFFLOAD_ALL) != 0'?

> +
> +	if (!sxe_hw_is_rss_enabled(hw)) {
> +		if (rss_hf != 0) {
> +			PMD_LOG_ERR(DRV, "rss not init but want set");
> +			ret = -EINVAL;
> +			goto l_end;
> +		}
> +
> +		goto l_end;
> +	}
> +
> +	if (rss_hf == 0) {
> +		PMD_LOG_ERR(DRV, "rss init but want disable it");
> +		ret = -EINVAL;
> +		goto l_end;
> +	}
> +
> +	sxe_rss_hash_set(hw, rss_conf);
> +
> +l_end:
> +	return ret;
> +}
> +
> +s32 sxe_rss_hash_conf_get(struct rte_eth_dev *dev,
> +				struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct sxe_adapter *adapter = dev->data->dev_private;
> +	struct sxe_hw *hw = &adapter->hw;
> +	u8 *hash_key;
> +	u32 rss_field;
> +	u32 rss_key;
> +	u64 rss_hf;
> +	u16 i;
> +
> +	hash_key = rss_conf->rss_key;

No check of 'rss_conf->rss_key_len'?

Thank you.

> +	if (hash_key != NULL) {
> +		for (i = 0; i < SXE_MAX_RSS_KEY_ENTRIES; i++) {
> +			rss_key = sxe_hw_rss_key_get_by_idx(hw, i);
> +			hash_key[(i * 4)] = rss_key & 0x000000FF;
> +			hash_key[(i * 4) + 1] = (rss_key >> 8) & 0x000000FF;
> +			hash_key[(i * 4) + 2] = (rss_key >> 16) & 0x000000FF;
> +			hash_key[(i * 4) + 3] = (rss_key >> 24) & 0x000000FF;
> +		}
> +	}
> +
> +
> +	if (!sxe_hw_is_rss_enabled(hw)) {
> +		rss_conf->rss_hf = 0;
> +		PMD_LOG_INFO(DRV, "rss not enabled, return 0");
> +		goto l_end;
> +	}
> +
> +	rss_hf = 0;
> +	rss_field = sxe_hw_rss_field_get(hw);
> +	if (rss_field & SXE_MRQC_RSS_FIELD_IPV4)
> +		rss_hf |= RTE_ETH_RSS_IPV4;
> +
> +	if (rss_field & SXE_MRQC_RSS_FIELD_IPV4_TCP)
> +		rss_hf |= RTE_ETH_RSS_NONFRAG_IPV4_TCP;
> +
> +	if (rss_field & SXE_MRQC_RSS_FIELD_IPV4_UDP)
> +		rss_hf |= RTE_ETH_RSS_NONFRAG_IPV4_UDP;
> +
> +	if (rss_field & SXE_MRQC_RSS_FIELD_IPV6)
> +		rss_hf |= RTE_ETH_RSS_IPV6;
> +
> +	if (rss_field & SXE_MRQC_RSS_FIELD_IPV6_TCP)
> +		rss_hf |= RTE_ETH_RSS_NONFRAG_IPV6_TCP;
> +
> +	if (rss_field & SXE_MRQC_RSS_FIELD_IPV6_UDP)
> +		rss_hf |= RTE_ETH_RSS_NONFRAG_IPV6_UDP;
> +
> +	PMD_LOG_DEBUG(DRV, "got rss hash func=0x%" SXE_PRIX64, rss_hf);
> +	rss_conf->rss_hf = rss_hf;
> +
> +l_end:
> +	return 0;
> +}
> diff --git a/drivers/net/sxe/pf/sxe_offload.h b/drivers/net/sxe/pf/sxe_offload.h
> index a70d6bf94b..458b6464c5 100644
> --- a/drivers/net/sxe/pf/sxe_offload.h
> +++ b/drivers/net/sxe/pf/sxe_offload.h
> @@ -7,6 +7,21 @@
>
> #include "sxe_hw.h"
>
> +#define SXE_RSS_OFFLOAD_ALL ( \
> +		RTE_ETH_RSS_IPV4 | \
> +		RTE_ETH_RSS_NONFRAG_IPV4_TCP | \
> +		RTE_ETH_RSS_NONFRAG_IPV4_UDP | \
> +		RTE_ETH_RSS_IPV6 | \
> +		RTE_ETH_RSS_NONFRAG_IPV6_TCP | \
> +		RTE_ETH_RSS_NONFRAG_IPV6_UDP)
> +
> +#if defined SXE_DPDK_L4_FEATURES && defined SXE_DPDK_FILTER_CTRL
> +u8 *sxe_rss_hash_key_get(void);
> +#endif
> +
> +void sxe_rss_hash_set(struct sxe_hw *hw,
> +				struct rte_eth_rss_conf *rss_conf);
> +
> u64 sxe_rx_queue_offload_capa_get(struct rte_eth_dev *dev);
>
> u64 sxe_rx_port_offload_capa_get(struct rte_eth_dev *dev);
> @@ -15,4 +30,22 @@ u64 sxe_tx_queue_offload_capa_get(struct rte_eth_dev *dev);
>
> u64 sxe_tx_port_offload_capa_get(struct rte_eth_dev *dev);
>
> +void sxe_rss_disable(struct rte_eth_dev *dev);
> +
> +void sxe_rss_configure(struct rte_eth_dev *dev);
> +
> +s32 sxe_rss_reta_update(struct rte_eth_dev *dev,
> +			struct rte_eth_rss_reta_entry64 *reta_conf,
> +			u16 reta_size);
> +
> +s32 sxe_rss_reta_query(struct rte_eth_dev *dev,
> +			 struct rte_eth_rss_reta_entry64 *reta_conf,
> +			 u16 reta_size);
> +
> +s32 sxe_rss_hash_update(struct rte_eth_dev *dev,
> +			struct rte_eth_rss_conf *rss_conf);
> +
> +s32 sxe_rss_hash_conf_get(struct rte_eth_dev *dev,
> +				struct rte_eth_rss_conf *rss_conf);
> +
> #endif
> -- 
> 2.18.2
>
>