[PATCH 0/6] bugfixes for hns3 PMD

[PATCH 0/6] bugfixes for hns3 PMD

[Min Hu (Connor)]

This patchset contains 6 bugfixes for hns3 PMD.

Huisong Li (5):
  net/hns3: fix inconsistent enabled RSS behavior
  net/hns3: remove unnecessary RSS switch
  net/hns3: fix the time waiting for PF reset completion
  net/hns3: fix RSS TC mode entry
  net/hns3: fix VF RSS TC mode entry

Jie Hai (1):
  net/hns3: remove duplicate macro definition

 drivers/net/hns3/hns3_cmd.h       |  1 -
 drivers/net/hns3/hns3_ethdev.c    |  2 -
 drivers/net/hns3/hns3_ethdev.h    |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c |  9 ++--
 drivers/net/hns3/hns3_flow.c      |  6 ++-
 drivers/net/hns3/hns3_rss.c       | 69 +++++++++++++++++++++++--------
 6 files changed, 62 insertions(+), 26 deletions(-)

-- 
2.33.0

[PATCH 1/6] net/hns3: remove duplicate macro definition

[Min Hu (Connor)]

From: Jie Hai <haijie1@huawei.com>

This patch fixes duplicate macro definition of HNS3_RSS_CFG_TBL_SIZE.

Fixes: 737f30e1c3ab ("net/hns3: support command interface with firmware")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 drivers/net/hns3/hns3_cmd.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/hns3/hns3_cmd.h b/drivers/net/hns3/hns3_cmd.h
index 81bc9e9d98..f9addc6069 100644
--- a/drivers/net/hns3/hns3_cmd.h
+++ b/drivers/net/hns3/hns3_cmd.h
@@ -603,7 +603,6 @@ struct hns3_cfg_gro_status_cmd {
 
 #define HNS3_RSS_HASH_KEY_OFFSET_B	4
 
-#define HNS3_RSS_CFG_TBL_SIZE	16
 #define HNS3_RSS_HASH_KEY_NUM	16
 /* Configure the algorithm mode and Hash Key, opcode:0x0D01 */
 struct hns3_rss_generic_config_cmd {
-- 
2.33.0

[PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
dev_configure phase. However, if this flag isn't set, RSS can be enabled
through the ethdev ops and rte_flow API. This behavior is contrary to each
other.

Fixes: c37ca66f2b27 ("net/hns3: support RSS")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/net/hns3/hns3_flow.c |  5 +++++
 drivers/net/hns3/hns3_rss.c  | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index aba07aaa6f..46371c4190 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -1371,6 +1371,7 @@ hns3_parse_rss_filter(struct rte_eth_dev *dev,
 		      const struct rte_flow_action *actions,
 		      struct rte_flow_error *error)
 {
+	enum rte_eth_rx_mq_mode mq_mode = dev->data->dev_conf.rxmode.mq_mode;
 	struct hns3_adapter *hns = dev->data->dev_private;
 	struct hns3_hw *hw = &hns->hw;
 	struct hns3_rss_conf *rss_conf = &hw->rss_info;
@@ -1426,6 +1427,10 @@ hns3_parse_rss_filter(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					  &rss->types,
 					  "input RSS types are not supported");
+	if (!((uint32_t)mq_mode & RTE_ETH_MQ_RX_RSS_FLAG))
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+					  act, "multi-queue RSS isn't enabled");
 
 	act_index++;
 
diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 1782d63883..e0eab05fb9 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -399,6 +399,7 @@ int
 hns3_dev_rss_hash_update(struct rte_eth_dev *dev,
 			 struct rte_eth_rss_conf *rss_conf)
 {
+	enum rte_eth_rx_mq_mode mq_mode = dev->data->dev_conf.rxmode.mq_mode;
 	struct hns3_adapter *hns = dev->data->dev_private;
 	struct hns3_hw *hw = &hns->hw;
 	struct hns3_rss_tuple_cfg *tuple = &hw->rss_info.rss_tuple_sets;
@@ -408,6 +409,11 @@ hns3_dev_rss_hash_update(struct rte_eth_dev *dev,
 	uint8_t *key = rss_conf->rss_key;
 	int ret;
 
+	if (!((uint32_t)mq_mode & RTE_ETH_MQ_RX_RSS_FLAG)) {
+		hns3_err(hw, "multi-queue RSS isn't enabled.");
+		return -EOPNOTSUPP;
+	}
+
 	if (hw->rss_dis_flag)
 		return -EINVAL;
 
@@ -498,6 +504,7 @@ hns3_dev_rss_reta_update(struct rte_eth_dev *dev,
 			 struct rte_eth_rss_reta_entry64 *reta_conf,
 			 uint16_t reta_size)
 {
+	enum rte_eth_rx_mq_mode mq_mode = dev->data->dev_conf.rxmode.mq_mode;
 	struct hns3_adapter *hns = dev->data->dev_private;
 	struct hns3_hw *hw = &hns->hw;
 	struct hns3_rss_conf *rss_cfg = &hw->rss_info;
@@ -506,6 +513,11 @@ hns3_dev_rss_reta_update(struct rte_eth_dev *dev,
 	uint16_t i;
 	int ret;
 
+	if (!((uint32_t)mq_mode & RTE_ETH_MQ_RX_RSS_FLAG)) {
+		hns3_err(hw, "multi-queue RSS isn't enabled.");
+		return -EOPNOTSUPP;
+	}
+
 	if (reta_size != hw->rss_ind_tbl_size) {
 		hns3_err(hw, "The size of hash lookup table configured (%u)"
 			 "doesn't match the number hardware can supported"
-- 
2.33.0

[PATCH 3/6] net/hns3: remove unnecessary RSS switch

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

Whether the RSS is enabled depends on RTE_ETH_MQ_RX_RSS_FLAG and packet
tuple are enabled. So the RSS switch is unnecessary.

Fixes: 5e782bc2570c ("net/hns3: fix configuring RSS hash when rules are flushed")
Fixes: fd8196838763 ("net/hns3: fix configuring device with RSS enabled")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 2 --
 drivers/net/hns3/hns3_ethdev.h    | 1 -
 drivers/net/hns3/hns3_ethdev_vf.c | 2 --
 drivers/net/hns3/hns3_flow.c      | 1 -
 drivers/net/hns3/hns3_rss.c       | 3 ---
 5 files changed, 9 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 4e089e682f..24ae5ef929 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2019,7 +2019,6 @@ hns3_dev_configure(struct rte_eth_dev *dev)
 	if ((uint32_t)mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) {
 		conf->rxmode.offloads |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
 		rss_conf = conf->rx_adv_conf.rss_conf;
-		hw->rss_dis_flag = false;
 		ret = hns3_dev_rss_hash_update(dev, &rss_conf);
 		if (ret)
 			goto cfg_err;
@@ -2825,7 +2824,6 @@ hns3_get_board_configuration(struct hns3_hw *hw)
 
 	hw->mac.media_type = cfg.media_type;
 	hw->rss_size_max = cfg.rss_size_max;
-	hw->rss_dis_flag = false;
 	memcpy(hw->mac.mac_addr, cfg.mac_addr, RTE_ETHER_ADDR_LEN);
 	hw->mac.phy_addr = cfg.phy_addr;
 	hw->dcb_info.num_pg = 1;
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 9a0fa09b57..a00f841543 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -520,7 +520,6 @@ struct hns3_hw {
 
 	/* The configuration info of RSS */
 	struct hns3_rss_conf rss_info;
-	bool rss_dis_flag; /* disable rss flag. true: disable, false: enable */
 	uint16_t rss_ind_tbl_size;
 	uint16_t rss_key_size;
 
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 06ddf64184..8f18e7dd54 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -495,7 +495,6 @@ hns3vf_dev_configure(struct rte_eth_dev *dev)
 	/* When RSS is not configured, redirect the packet queue 0 */
 	if ((uint32_t)mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) {
 		conf->rxmode.offloads |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
-		hw->rss_dis_flag = false;
 		rss_conf = conf->rx_adv_conf.rss_conf;
 		ret = hns3_dev_rss_hash_update(dev, &rss_conf);
 		if (ret)
@@ -997,7 +996,6 @@ hns3vf_get_configuration(struct hns3_hw *hw)
 	int ret;
 
 	hw->mac.media_type = HNS3_MEDIA_TYPE_NONE;
-	hw->rss_dis_flag = false;
 
 	/* Get device capability */
 	ret = hns3vf_get_capability(hw);
diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index 46371c4190..485b7be854 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -1458,7 +1458,6 @@ hns3_disable_rss(struct hns3_hw *hw)
 
 	/* Disable RSS */
 	hw->rss_info.conf.types = 0;
-	hw->rss_dis_flag = true;
 
 	return 0;
 }
diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index e0eab05fb9..5be7a05af2 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -414,9 +414,6 @@ hns3_dev_rss_hash_update(struct rte_eth_dev *dev,
 		return -EOPNOTSUPP;
 	}
 
-	if (hw->rss_dis_flag)
-		return -EINVAL;
-
 	rte_spinlock_lock(&hw->lock);
 	ret = hns3_set_rss_tuple_by_rss_hf(hw, tuple, rss_hf);
 	if (ret)
-- 
2.33.0

[PATCH 4/6] net/hns3: fix the time waiting for PF reset completion

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

On the case that PF and VF need to be reset, after the hardware reset is
complete, VF needs wait for 1 second to restore the configuration so that
VF does not fail to recover because PF reset isn't complete. But the
estimated time is not sufficient. This patch fixes it to 5 seconds.

Fixes: 2790c6464725 ("net/hns3: support device reset")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/net/hns3/hns3_ethdev_vf.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 8f18e7dd54..7d414b1af8 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -1875,6 +1875,7 @@ hns3vf_is_reset_pending(struct hns3_adapter *hns)
 static int
 hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
 {
+#define HNS3_WAIT_PF_RESET_READY_TIME 5
 	struct hns3_hw *hw = &hns->hw;
 	struct hns3_wait_data *wait_data = hw->reset.wait_data;
 	struct timeval tv;
@@ -1895,12 +1896,14 @@ hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
 			return 0;
 
 		wait_data->check_completion = NULL;
-		wait_data->interval = 1 * MSEC_PER_SEC * USEC_PER_MSEC;
+		wait_data->interval =
+		HNS3_WAIT_PF_RESET_READY_TIME * MSEC_PER_SEC * USEC_PER_MSEC;
 		wait_data->count = 1;
 		wait_data->result = HNS3_WAIT_REQUEST;
 		rte_eal_alarm_set(wait_data->interval, hns3_wait_callback,
 				  wait_data);
-		hns3_warn(hw, "hardware is ready, delay 1 sec for PF reset complete");
+		hns3_warn(hw, "hardware is ready, delay %d sec for PF reset complete",
+				HNS3_WAIT_PF_RESET_READY_TIME);
 		return -EAGAIN;
 	} else if (wait_data->result == HNS3_WAIT_TIMEOUT) {
 		hns3_clock_gettime(&tv);
-- 
2.33.0

[PATCH 5/6] net/hns3: fix RSS TC mode entry

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

The driver allocates queues only to valid TCs. But the driver also
configure queues for invalid TCs, which is unreasonable.

Fixes: c37ca66f2b27 ("net/hns3: support RSS")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/net/hns3/hns3_rss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 5be7a05af2..5077d49882 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -610,8 +610,8 @@ hns3_set_rss_tc_mode(struct hns3_hw *hw)
 
 	for (i = 0; i < HNS3_MAX_TC_NUM; i++) {
 		tc_valid[i] = !!(hw->hw_tc_map & BIT(i));
-		tc_size[i] = roundup_size;
-		tc_offset[i] = rss_size * i;
+		tc_size[i] = tc_valid[i] ? roundup_size : 0;
+		tc_offset[i] = tc_valid[i] ? rss_size * i : 0;
 	}
 
 	hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_RSS_TC_MODE, false);
-- 
2.33.0

[PATCH 6/6] net/hns3: fix VF RSS TC mode entry

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

For packets with VLAN priorities destined for the VF, hardware still
assign Rx queue based on the Up-to-TC mapping PF configured. But VF has
only one TC. If other TC don't enable, it causes that the priority packets
that aren't destined for TC0 aren't received by RSS hash but is destined
for queue 0. So driver has to enable the unused TC by using TC0 queue
mapping configuration.

Fixes: c37ca66f2b27 ("net/hns3: support RSS")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/net/hns3/hns3_rss.c | 56 +++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 5077d49882..9606801140 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -587,33 +587,59 @@ hns3_dev_rss_reta_query(struct rte_eth_dev *dev,
 	return 0;
 }
 
-/*
- * Used to configure the tc_size and tc_offset.
- */
+static void
+hns3_set_rss_tc_mode_entry(struct hns3_hw *hw, uint8_t *tc_valid,
+			   uint16_t *tc_size, uint16_t *tc_offset,
+			   uint8_t tc_num)
+{
+	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
+	uint16_t rss_size = hw->alloc_rss_size;
+	uint16_t roundup_size;
+	uint16_t i;
+
+	roundup_size = roundup_pow_of_two(rss_size);
+	roundup_size = ilog2(roundup_size);
+
+	for (i = 0; i < tc_num; i++) {
+		if (hns->is_vf) {
+			/*
+			 * For packets with VLAN priorities destined for the VF,
+			 * hardware still assign Rx queue based on the Up-to-TC
+			 * mapping PF configured. But VF has only one TC. If
+			 * other TC don't enable, it causes that the priority
+			 * packets that aren't destined for TC0 aren't received
+			 * by RSS hash but is destined for queue 0. So driver
+			 * has to enable the unused TC by using TC0 queue
+			 * mapping configuration.
+			 */
+			tc_valid[i] = (hw->hw_tc_map & BIT(i)) ?
+					!!(hw->hw_tc_map & BIT(i)) : 1;
+			tc_size[i] = roundup_size;
+			tc_offset[i] = (hw->hw_tc_map & BIT(i)) ?
+					rss_size * i : 0;
+		} else {
+			tc_valid[i] = !!(hw->hw_tc_map & BIT(i));
+			tc_size[i] = tc_valid[i] ? roundup_size : 0;
+			tc_offset[i] = tc_valid[i] ? rss_size * i : 0;
+		}
+	}
+}
+
 static int
 hns3_set_rss_tc_mode(struct hns3_hw *hw)
 {
-	uint16_t rss_size = hw->alloc_rss_size;
 	struct hns3_rss_tc_mode_cmd *req;
 	uint16_t tc_offset[HNS3_MAX_TC_NUM];
 	uint8_t tc_valid[HNS3_MAX_TC_NUM];
 	uint16_t tc_size[HNS3_MAX_TC_NUM];
 	struct hns3_cmd_desc desc;
-	uint16_t roundup_size;
 	uint16_t i;
 	int ret;
 
-	req = (struct hns3_rss_tc_mode_cmd *)desc.data;
-
-	roundup_size = roundup_pow_of_two(rss_size);
-	roundup_size = ilog2(roundup_size);
-
-	for (i = 0; i < HNS3_MAX_TC_NUM; i++) {
-		tc_valid[i] = !!(hw->hw_tc_map & BIT(i));
-		tc_size[i] = tc_valid[i] ? roundup_size : 0;
-		tc_offset[i] = tc_valid[i] ? rss_size * i : 0;
-	}
+	hns3_set_rss_tc_mode_entry(hw, tc_valid, tc_size,
+				   tc_offset, HNS3_MAX_TC_NUM);
 
+	req = (struct hns3_rss_tc_mode_cmd *)desc.data;
 	hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_RSS_TC_MODE, false);
 	for (i = 0; i < HNS3_MAX_TC_NUM; i++) {
 		uint16_t mode = 0;
-- 
2.33.0

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Ferruh Yigit]

On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> dev_configure phase. However, if this flag isn't set, RSS can be enabled
> through the ethdev ops and rte_flow API. This behavior is contrary to each
> other.
> 
> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>


Hi Huisong, Connor,

Let's get a little more feedback for this patch, cc'ed more people.


To enable RSS, multi queue mode should be set to 'RTE_ETH_MQ_RX_RSS_FLAG'.

But I wonder if it is required to configure RSS via flow API,
and if other PMDs check this configuration for flow API?

Re: [PATCH 4/6] net/hns3: fix the time waiting for PF reset completion

[Ferruh Yigit]

On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> @@ -1895,12 +1896,14 @@ hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
>   			return 0;
>   
>   		wait_data->check_completion = NULL;
> -		wait_data->interval = 1 * MSEC_PER_SEC * USEC_PER_MSEC;
> +		wait_data->interval =
> +		HNS3_WAIT_PF_RESET_READY_TIME * MSEC_PER_SEC * USEC_PER_MSEC;

Can you please fix the syntax?
I guess all can fit into single line...

Re: [PATCH 1/6] net/hns3: remove duplicate macro definition

[Ferruh Yigit]

On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> From: Jie Hai <haijie1@huawei.com>
> 
> This patch fixes duplicate macro definition of HNS3_RSS_CFG_TBL_SIZE.
> 
> Fixes: 737f30e1c3ab ("net/hns3: support command interface with firmware")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>

Except from 2/6, 3/6, 4/6
Series applied to dpdk-next-net/main, thanks.


For 2/6, 3/6 can be good to wait for more comment on
RSS configuration.

For 4/6 can you please send a new version.

Re: [PATCH 4/6] net/hns3: fix the time waiting for PF reset completion

[Min Hu (Connor)]

Hi, Ferruh,

在 2022/3/1 1:09, Ferruh Yigit 写道:
> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>> @@ -1895,12 +1896,14 @@ hns3vf_wait_hardware_ready(struct hns3_adapter 
>> *hns)
>>               return 0;
>>           wait_data->check_completion = NULL;
>> -        wait_data->interval = 1 * MSEC_PER_SEC * USEC_PER_MSEC;
>> +        wait_data->interval =
>> +        HNS3_WAIT_PF_RESET_READY_TIME * MSEC_PER_SEC * USEC_PER_MSEC;
> 
> Can you please fix the syntax?
> I guess all can fit into single line...
If not so, the code in single line will exceed 80 characters.

> .

Re: [PATCH 4/6] net/hns3: fix the time waiting for PF reset completion

[Ferruh Yigit]

On 3/1/2022 6:32 AM, Min Hu (Connor) wrote:
> Hi, Ferruh,
> 
> 在 2022/3/1 1:09, Ferruh Yigit 写道:
>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>> @@ -1895,12 +1896,14 @@ hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
>>>               return 0;
>>>           wait_data->check_completion = NULL;
>>> -        wait_data->interval = 1 * MSEC_PER_SEC * USEC_PER_MSEC;
>>> +        wait_data->interval =
>>> +        HNS3_WAIT_PF_RESET_READY_TIME * MSEC_PER_SEC * USEC_PER_MSEC;
>>
>> Can you please fix the syntax?
>> I guess all can fit into single line...
> If not so, the code in single line will exceed 80 characters.
> 

Right, it doesn't fit into single line, still it can be formatted better.

wait_data->interval = HNS3_WAIT_PF_RESET_READY_TIME *
	MSEC_PER_SEC * USEC_PER_MSEC;

[PATCH v2] net/hns3: fix the time waiting for PF reset completion

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

On the case that PF and VF need to be reset, after the hardware reset is
complete, VF needs wait for 1 second to restore the configuration so that
VF does not fail to recover because PF reset isn't complete. But the
estimated time is not sufficient. This patch fixes it to 5 seconds.

Fixes: 2790c6464725 ("net/hns3: support device reset")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
v2:
* fix syntax.
---
 drivers/net/hns3/hns3_ethdev_vf.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 8f18e7dd54..e06a1a41dd 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -1875,6 +1875,7 @@ hns3vf_is_reset_pending(struct hns3_adapter *hns)
 static int
 hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
 {
+#define HNS3_WAIT_PF_RESET_READY_TIME 5
 	struct hns3_hw *hw = &hns->hw;
 	struct hns3_wait_data *wait_data = hw->reset.wait_data;
 	struct timeval tv;
@@ -1895,12 +1896,14 @@ hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
 			return 0;
 
 		wait_data->check_completion = NULL;
-		wait_data->interval = 1 * MSEC_PER_SEC * USEC_PER_MSEC;
+		wait_data->interval = HNS3_WAIT_PF_RESET_READY_TIME *
+			MSEC_PER_SEC * USEC_PER_MSEC;
 		wait_data->count = 1;
 		wait_data->result = HNS3_WAIT_REQUEST;
 		rte_eal_alarm_set(wait_data->interval, hns3_wait_callback,
 				  wait_data);
-		hns3_warn(hw, "hardware is ready, delay 1 sec for PF reset complete");
+		hns3_warn(hw, "hardware is ready, delay %d sec for PF reset complete",
+				HNS3_WAIT_PF_RESET_READY_TIME);
 		return -EAGAIN;
 	} else if (wait_data->result == HNS3_WAIT_TIMEOUT) {
 		hns3_clock_gettime(&tv);
-- 
2.33.0

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

在 2022/3/1 0:42, Ferruh Yigit 写道:
> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
>> dev_configure phase. However, if this flag isn't set, RSS can be enabled
>> through the ethdev ops and rte_flow API. This behavior is contrary to 
>> each
>> other.
>>
>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>
>
> Hi Huisong, Connor,
>
> Let's get a little more feedback for this patch, cc'ed more people.
>
>
> To enable RSS, multi queue mode should be set to 
> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>
> But I wonder if it is required to configure RSS via flow API,

I do not know the original purpose of adding the RSS configuration in 
flow API.

However, as far as I know, the hash algorithm can be configured via this 
API,

but not via ethdev ops API.

> and if other PMDs check this configuration for flow API?

Some PMDs already have similar check in RSS releated ops or rte_flow API.

For example, hinic, axbge, bnxt, cnxk, otx2, and ena.

> .

Re: [PATCH 1/6] net/hns3: remove duplicate macro definition

[Ferruh Yigit]

On 2/28/2022 5:46 PM, Ferruh Yigit wrote:
> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>> From: Jie Hai <haijie1@huawei.com>
>>
>> This patch fixes duplicate macro definition of HNS3_RSS_CFG_TBL_SIZE.
>>
>> Fixes: 737f30e1c3ab ("net/hns3: support command interface with firmware")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
> 
> Except from 2/6, 3/6, 4/6
> Series applied to dpdk-next-net/main, thanks.
> 

Hi Connor,

I am adding your explicit ack in the next-net for merged patches,
please let me if you have an objection.

Acked-by: Min Hu (Connor) <humin29@huawei.com>

> 
> For 2/6, 3/6 can be good to wait for more comment on
> RSS configuration.
> 
> For 4/6 can you please send a new version.

Re: [PATCH v2] net/hns3: fix the time waiting for PF reset completion

[Ferruh Yigit]

On 3/2/2022 12:35 AM, Min Hu (Connor) wrote:
> From: Huisong Li<lihuisong@huawei.com>
> 
> On the case that PF and VF need to be reset, after the hardware reset is
> complete, VF needs wait for 1 second to restore the configuration so that
> VF does not fail to recover because PF reset isn't complete. But the
> estimated time is not sufficient. This patch fixes it to 5 seconds.
> 
> Fixes: 2790c6464725 ("net/hns3: support device reset")
> Cc:stable@dpdk.org
> 
> Signed-off-by: Huisong Li<lihuisong@huawei.com>

Converting to explicit ack:
Acked-by: Min Hu (Connor) <humin29@huawei.com>

Applied to dpdk-next-net/main, thanks.

RE: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Ori Kam]

Hi Lihuisong,

> -----Original Message-----
> From: lihuisong (C) <lihuisong@huawei.com>
> Sent: Wednesday, March 2, 2022 4:10 AM
> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior
> 
> 
> 在 2022/3/1 0:42, Ferruh Yigit 写道:
> > On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> >> From: Huisong Li <lihuisong@huawei.com>
> >>
> >> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> >> dev_configure phase. However, if this flag isn't set, RSS can be enabled
> >> through the ethdev ops and rte_flow API. This behavior is contrary to
> >> each
> >> other.
> >>
> >> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >
> >
> > Hi Huisong, Connor,
> >
> > Let's get a little more feedback for this patch, cc'ed more people.
> >
> >
> > To enable RSS, multi queue mode should be set to
> > 'RTE_ETH_MQ_RX_RSS_FLAG'.
> >
> > But I wonder if it is required to configure RSS via flow API,
> 
> I do not know the original purpose of adding the RSS configuration in
> flow API.
> 
The purpose is simple, this allow to create RSS per rule and not a global one.
For example create RSS that sends TCP to some queues while othe RSS will send
UDP traffic to different queues.

> However, as far as I know, the hash algorithm can be configured via this
> API,
> 
> but not via ethdev ops API.
> 
> > and if other PMDs check this configuration for flow API?
> 
> Some PMDs already have similar check in RSS releated ops or rte_flow API.
> 
> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
> 
> > .
From my view point those are two different settings.
The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
rte_flow is per rule.

I think, that if a PMD needs this flag, in order to enable it also for rte_flow then
it should be documented in the release note of the PMD.
It is a valid use case that the default traffic will not have RSS and only rules created by
rte_flow will have the RSS, for matching traffc.

Best,
Ori

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Thomas Monjalon]

02/03/2022 15:07, Ori Kam:
> From: lihuisong (C) <lihuisong@huawei.com>
> > 在 2022/3/1 0:42, Ferruh Yigit 写道:
> > > On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> > >> From: Huisong Li <lihuisong@huawei.com>
> > >>
> > >> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> > >> dev_configure phase. However, if this flag isn't set, RSS can be enabled
> > >> through the ethdev ops and rte_flow API. This behavior is contrary to
> > >> each
> > >> other.
> > >>
> > >> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> > >
> > >
> > > Hi Huisong, Connor,
> > >
> > > Let's get a little more feedback for this patch, cc'ed more people.
> > >
> > >
> > > To enable RSS, multi queue mode should be set to
> > > 'RTE_ETH_MQ_RX_RSS_FLAG'.
> > >
> > > But I wonder if it is required to configure RSS via flow API,
> > 
> > I do not know the original purpose of adding the RSS configuration in
> > flow API.
> > 
> The purpose is simple, this allow to create RSS per rule and not a global one.
> For example create RSS that sends TCP to some queues while othe RSS will send
> UDP traffic to different queues.
> 
> > However, as far as I know, the hash algorithm can be configured via this
> > API,
> > 
> > but not via ethdev ops API.
> > 
> > > and if other PMDs check this configuration for flow API?
> > 
> > Some PMDs already have similar check in RSS releated ops or rte_flow API.
> > 
> > For example, hinic, axbge, bnxt, cnxk, otx2, and ena.

I suggest removing these checks.

> From my view point those are two different settings.
> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
> rte_flow is per rule.
> 
> I think, that if a PMD needs this flag, in order to enable it also for rte_flow then
> it should be documented in the release note of the PMD.
> It is a valid use case that the default traffic will not have RSS and only rules created by
> rte_flow will have the RSS, for matching traffc.

I think RTE_ETH_MQ_RX_RSS_FLAG should not be required by any PMD
for rte_flow RSS rules.

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Stephen Hemminger]

On Wed, 02 Mar 2022 15:46:37 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 02/03/2022 15:07, Ori Kam:
> > From: lihuisong (C) <lihuisong@huawei.com>  
> > > 在 2022/3/1 0:42, Ferruh Yigit 写道:  
> > > > On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:  
> > > >> From: Huisong Li <lihuisong@huawei.com>
> > > >>
> > > >> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> > > >> dev_configure phase. However, if this flag isn't set, RSS can be enabled
> > > >> through the ethdev ops and rte_flow API. This behavior is contrary to
> > > >> each
> > > >> other.
> > > >>
> > > >> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> > > >> Cc: stable@dpdk.org
> > > >>
> > > >> Signed-off-by: Huisong Li <lihuisong@huawei.com>  
> > > >
> > > >
> > > > Hi Huisong, Connor,
> > > >
> > > > Let's get a little more feedback for this patch, cc'ed more people.
> > > >
> > > >
> > > > To enable RSS, multi queue mode should be set to
> > > > 'RTE_ETH_MQ_RX_RSS_FLAG'.
> > > >
> > > > But I wonder if it is required to configure RSS via flow API,  
> > > 
> > > I do not know the original purpose of adding the RSS configuration in
> > > flow API.
> > >   
> > The purpose is simple, this allow to create RSS per rule and not a global one.
> > For example create RSS that sends TCP to some queues while othe RSS will send
> > UDP traffic to different queues.
> >   
> > > However, as far as I know, the hash algorithm can be configured via this
> > > API,
> > > 
> > > but not via ethdev ops API.
> > >   
> > > > and if other PMDs check this configuration for flow API?  
> > > 
> > > Some PMDs already have similar check in RSS releated ops or rte_flow API.
> > > 
> > > For example, hinic, axbge, bnxt, cnxk, otx2, and ena.  
> 
> I suggest removing these checks.
> 
> > From my view point those are two different settings.
> > The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
> > rte_flow is per rule.
> > 
> > I think, that if a PMD needs this flag, in order to enable it also for rte_flow then
> > it should be documented in the release note of the PMD.
> > It is a valid use case that the default traffic will not have RSS and only rules created by
> > rte_flow will have the RSS, for matching traffc.  
> 
> I think RTE_ETH_MQ_RX_RSS_FLAG should not be required by any PMD
> for rte_flow RSS rules.
> 
> 

Agreed.

The ideal state around RSS would be:
   - global settings affect the default started queues when device is started.
     if RSS is enabled and 4 queues are started, then RSS would happen for received
     packets across those 4 queues.
   - when adding new flow related rx:
      - new queues could be started - BUT not part of original RSS
      - new rte_flow rule can do RSS against the new flow:
            example:
              start queues 4,5,6,7
              rte_flow match TCP port 80 and process on queues 4,5,6,7 with RSS

This is a mess today, most devices work differently and there is no reference software
implementation.  In an ideal state, there would be a reference software implementation
that implements all of rte_flow, and all new hardware support would have to work the same
as the reference implementation. And there would be a full CI test suite around rte_flow.

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

Hi, Ori,

在 2022/3/2 22:07, Ori Kam 写道:
> Hi Lihuisong,
>
>> -----Original Message-----
>> From: lihuisong (C) <lihuisong@huawei.com>
>> Sent: Wednesday, March 2, 2022 4:10 AM
>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior
>>
>>
>> 在 2022/3/1 0:42, Ferruh Yigit 写道:
>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
>>>> dev_configure phase. However, if this flag isn't set, RSS can be enabled
>>>> through the ethdev ops and rte_flow API. This behavior is contrary to
>>>> each
>>>> other.
>>>>
>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>
>>> Hi Huisong, Connor,
>>>
>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>
>>>
>>> To enable RSS, multi queue mode should be set to
>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>
>>> But I wonder if it is required to configure RSS via flow API,
>> I do not know the original purpose of adding the RSS configuration in
>> flow API.
>>
> The purpose is simple, this allow to create RSS per rule and not a global one.
> For example create RSS that sends TCP to some queues while othe RSS will send
> UDP traffic to different queues.
I'm a little confused now. The "per rule" also seems to be a global 
configuration.
Example:
  - start PMD with 0,1,2,3
  - create TCP packets to 2,3 queues. At this moment, only 2,3 queues 
can be received for other types of packets.
Because this rule is implemented by modifying the entry of the 
redirection table which is global for this device.
>
>> However, as far as I know, the hash algorithm can be configured via this
>> API,
>>
>> but not via ethdev ops API.
>>
>>> and if other PMDs check this configuration for flow API?
>> Some PMDs already have similar check in RSS releated ops or rte_flow API.
>>
>> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
>>
>>> .
>  From my view point those are two different settings.
> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
> rte_flow is per rule.
>
> I think, that if a PMD needs this flag, in order to enable it also for rte_flow then
> it should be documented in the release note of the PMD.
> It is a valid use case that the default traffic will not have RSS and only rules created by
> rte_flow will have the RSS, for matching traffc.
>
> Best,
> Ori

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

Thanks, Stephen.

在 2022/3/3 0:59, Stephen Hemminger 写道:
> On Wed, 02 Mar 2022 15:46:37 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
>
>> 02/03/2022 15:07, Ori Kam:
>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>> 在 2022/3/1 0:42, Ferruh Yigit 写道:
>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>>
>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be enabled
>>>>>> through the ethdev ops and rte_flow API. This behavior is contrary to
>>>>>> each
>>>>>> other.
>>>>>>
>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>
>>>>> Hi Huisong, Connor,
>>>>>
>>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>>
>>>>>
>>>>> To enable RSS, multi queue mode should be set to
>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>>
>>>>> But I wonder if it is required to configure RSS via flow API,
>>>> I do not know the original purpose of adding the RSS configuration in
>>>> flow API.
>>>>    
>>> The purpose is simple, this allow to create RSS per rule and not a global one.
>>> For example create RSS that sends TCP to some queues while othe RSS will send
>>> UDP traffic to different queues.
>>>    
>>>> However, as far as I know, the hash algorithm can be configured via this
>>>> API,
>>>>
>>>> but not via ethdev ops API.
>>>>    
>>>>> and if other PMDs check this configuration for flow API?
>>>> Some PMDs already have similar check in RSS releated ops or rte_flow API.
>>>>
>>>> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
>> I suggest removing these checks.
>>
>>>  From my view point those are two different settings.
>>> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
>>> rte_flow is per rule.
>>>
>>> I think, that if a PMD needs this flag, in order to enable it also for rte_flow then
>>> it should be documented in the release note of the PMD.
>>> It is a valid use case that the default traffic will not have RSS and only rules created by
>>> rte_flow will have the RSS, for matching traffc.
>> I think RTE_ETH_MQ_RX_RSS_FLAG should not be required by any PMD
>> for rte_flow RSS rules.
>>
>>
> Agreed.
>
> The ideal state around RSS would be:
>     - global settings affect the default started queues when device is started.
>       if RSS is enabled and 4 queues are started, then RSS would happen for received
>       packets across those 4 queues.
>     - when adding new flow related rx:
>        - new queues could be started - BUT not part of original RSS
How should I understand this? If do this, we have to increase queue numbers.
>        - new rte_flow rule can do RSS against the new flow:
>              example:
>                start queues 4,5,6,7
>                rte_flow match TCP port 80 and process on queues 4,5,6,7 with RSS
I think you have the same view as Ori. But some original RSS hash 
configuration
has been overwritten by a new rule, such as redirection table.
>
> This is a mess today, most devices work differently and there is no reference software
> implementation.  In an ideal state, there would be a reference software implementation
> that implements all of rte_flow, and all new hardware support would have to work the same
> as the reference implementation. And there would be a full CI test suite around rte_flow.
> .

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

在 2022/3/3 10:47, lihuisong (C) 写道:
> Hi, Ori,
>
> 在 2022/3/2 22:07, Ori Kam 写道:
>> Hi Lihuisong,
>>
>>> -----Original Message-----
>>> From: lihuisong (C) <lihuisong@huawei.com>
>>> Sent: Wednesday, March 2, 2022 4:10 AM
>>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS 
>>> behavior
>>>
>>>
>>> 在 2022/3/1 0:42, Ferruh Yigit 写道:
>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>
>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
>>>>> dev_configure phase. However, if this flag isn't set, RSS can be 
>>>>> enabled
>>>>> through the ethdev ops and rte_flow API. This behavior is contrary to
>>>>> each
>>>>> other.
>>>>>
>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> Hi Huisong, Connor,
>>>>
>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>
>>>>
>>>> To enable RSS, multi queue mode should be set to
>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>
>>>> But I wonder if it is required to configure RSS via flow API,
>>> I do not know the original purpose of adding the RSS configuration in
>>> flow API.
>>>
>> The purpose is simple, this allow to create RSS per rule and not a 
>> global one.
>> For example create RSS that sends TCP to some queues while othe RSS 
>> will send
>> UDP traffic to different queues.
> I'm a little confused now. The "per rule" also seems to be a global 
> configuration.
> Example:
>  - start PMD with 0,1,2,3
>  - create TCP packets to 2,3 queues. At this moment, only 2,3 queues 
> can be received for other types of packets.
> Because this rule is implemented by modifying the entry of the 
> redirection table which is global for this device.
Hi, Ori and Stephen.
Can you help me clear up the confusion above? If some NICs behave like 
this, what should we do about it?
>>
>>> However, as far as I know, the hash algorithm can be configured via 
>>> this
>>> API,
>>>
>>> but not via ethdev ops API.
>>>
>>>> and if other PMDs check this configuration for flow API?
>>> Some PMDs already have similar check in RSS releated ops or rte_flow 
>>> API.
>>>
>>> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
>>>
>>>> .
>>  From my view point those are two different settings.
>> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
>> rte_flow is per rule.
>>
>> I think, that if a PMD needs this flag, in order to enable it also 
>> for rte_flow then
>> it should be documented in the release note of the PMD.
>> It is a valid use case that the default traffic will not have RSS and 
>> only rules created by
>> rte_flow will have the RSS, for matching traffc.
>>
>> Best,
>> Ori

RE: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Ori Kam]

Hi Lihusiong,

> -----Original Message-----
> From: lihuisong (C) <lihuisong@huawei.com>
> Sent: Wednesday, March 9, 2022 10:03 AM
> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior
> 
> 
> 在 2022/3/3 10:47, lihuisong (C) 写道:
> > Hi, Ori,
> >
> > 在 2022/3/2 22:07, Ori Kam 写道:
> >> Hi Lihuisong,
> >>
> >>> -----Original Message-----
> >>> From: lihuisong (C) <lihuisong@huawei.com>
> >>> Sent: Wednesday, March 2, 2022 4:10 AM
> >>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS
> >>> behavior
> >>>
> >>>
> >>> 在 2022/3/1 0:42, Ferruh Yigit 写道:
> >>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> >>>>> From: Huisong Li <lihuisong@huawei.com>
> >>>>>
> >>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> >>>>> dev_configure phase. However, if this flag isn't set, RSS can be
> >>>>> enabled
> >>>>> through the ethdev ops and rte_flow API. This behavior is contrary to
> >>>>> each
> >>>>> other.
> >>>>>
> >>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> >>>>> Cc: stable@dpdk.org
> >>>>>
> >>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >>>>
> >>>> Hi Huisong, Connor,
> >>>>
> >>>> Let's get a little more feedback for this patch, cc'ed more people.
> >>>>
> >>>>
> >>>> To enable RSS, multi queue mode should be set to
> >>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
> >>>>
> >>>> But I wonder if it is required to configure RSS via flow API,
> >>> I do not know the original purpose of adding the RSS configuration in
> >>> flow API.
> >>>
> >> The purpose is simple, this allow to create RSS per rule and not a
> >> global one.
> >> For example create RSS that sends TCP to some queues while othe RSS
> >> will send
> >> UDP traffic to different queues.
> > I'm a little confused now. The "per rule" also seems to be a global
> > configuration.
> > Example:
> >  - start PMD with 0,1,2,3
> >  - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
> > can be received for other types of packets.
> > Because this rule is implemented by modifying the entry of the
> > redirection table which is global for this device.
> Hi, Ori and Stephen.
> Can you help me clear up the confusion above? If some NICs behave like
> this, what should we do about it?

I'm not sure I understand the issue, maybe it is releated to some HW/PMD limitation.
In your example non TCP traffic will be routed to one of the 4 queues (0,1,2,3),
While TCP traffic will only be routed to queues 2,3.

Now I can add new rule that matches on UDP packet and RSS to queue 0 and 3 in this case:
TCP packets will be routed to queues 0,3.
UDP packets will be routed to queues 2,3.
All the rest of the traffic will be routed to queues 0,1,2,3

And just to be clear if now I add a rule to match all packets in higher priority,
with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.

At least this is what is expected, from API point of view.

Best,
Ori

> >>
> >>> However, as far as I know, the hash algorithm can be configured via
> >>> this
> >>> API,
> >>>
> >>> but not via ethdev ops API.
> >>>
> >>>> and if other PMDs check this configuration for flow API?
> >>> Some PMDs already have similar check in RSS releated ops or rte_flow
> >>> API.
> >>>
> >>> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
> >>>
> >>>> .
> >>  From my view point those are two different settings.
> >> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
> >> rte_flow is per rule.
> >>
> >> I think, that if a PMD needs this flag, in order to enable it also
> >> for rte_flow then
> >> it should be documented in the release note of the PMD.
> >> It is a valid use case that the default traffic will not have RSS and
> >> only rules created by
> >> rte_flow will have the RSS, for matching traffc.
> >>
> >> Best,
> >> Ori

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

在 2022/3/9 17:55, Ori Kam 写道:
> Hi Lihusiong,
>
>> -----Original Message-----
>> From: lihuisong (C) <lihuisong@huawei.com>
>> Sent: Wednesday, March 9, 2022 10:03 AM
>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior
>>
>>
>> 在 2022/3/3 10:47, lihuisong (C) 写道:
>>> Hi, Ori,
>>>
>>> 在 2022/3/2 22:07, Ori Kam 写道:
>>>> Hi Lihuisong,
>>>>
>>>>> -----Original Message-----
>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>> Sent: Wednesday, March 2, 2022 4:10 AM
>>>>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS
>>>>> behavior
>>>>>
>>>>>
>>>>> 在 2022/3/1 0:42, Ferruh Yigit 写道:
>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>>>
>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
>>>>>>> enabled
>>>>>>> through the ethdev ops and rte_flow API. This behavior is contrary to
>>>>>>> each
>>>>>>> other.
>>>>>>>
>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>>>> Cc: stable@dpdk.org
>>>>>>>
>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>> Hi Huisong, Connor,
>>>>>>
>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>>>
>>>>>>
>>>>>> To enable RSS, multi queue mode should be set to
>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>>>
>>>>>> But I wonder if it is required to configure RSS via flow API,
>>>>> I do not know the original purpose of adding the RSS configuration in
>>>>> flow API.
>>>>>
>>>> The purpose is simple, this allow to create RSS per rule and not a
>>>> global one.
>>>> For example create RSS that sends TCP to some queues while othe RSS
>>>> will send
>>>> UDP traffic to different queues.
>>> I'm a little confused now. The "per rule" also seems to be a global
>>> configuration.
>>> Example:
>>>   - start PMD with 0,1,2,3
>>>   - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
>>> can be received for other types of packets.
>>> Because this rule is implemented by modifying the entry of the
>>> redirection table which is global for this device.
>> Hi, Ori and Stephen.
>> Can you help me clear up the confusion above? If some NICs behave like
>> this, what should we do about it?
> I'm not sure I understand the issue, maybe it is releated to some HW/PMD limitation.
> In your example non TCP traffic will be routed to one of the 4 queues (0,1,2,3),
> While TCP traffic will only be routed to queues 2,3.
>
> Now I can add new rule that matches on UDP packet and RSS to queue 0 and 3 in this case:
> TCP packets will be routed to queues 0,3.
> UDP packets will be routed to queues 2,3.
> All the rest of the traffic will be routed to queues 0,1,2,3
>
> And just to be clear if now I add a rule to match all packets in higher priority,
> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
>
> At least this is what is expected, from API point of view.
>
> Best,
> Ori
Thank you for your answer. I understand it.
hns3 PMD cannot implement the above functions due to hardware limitation.
we may need add a check that specified RSS queues cannot be supported 
when specified packets types.
And only the packet type is specified, which meets the requirements of 
rte_flow API.
The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
Thanks, Ori and Stephen😁

But, I think, it is necessary for the '.rss_hash_update' and 
'.reta_update' APIs
in eth_dev_ops to verify this flag. What do you think? @Thomas, @Ferruh, 
@Ori and @Stephen.
>
>>>>> However, as far as I know, the hash algorithm can be configured via
>>>>> this
>>>>> API,
>>>>>
>>>>> but not via ethdev ops API.
>>>>>
>>>>>> and if other PMDs check this configuration for flow API?
>>>>> Some PMDs already have similar check in RSS releated ops or rte_flow
>>>>> API.
>>>>>
>>>>> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
>>>>>
>>>>>> .
>>>>   From my view point those are two different settings.
>>>> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
>>>> rte_flow is per rule.
>>>>
>>>> I think, that if a PMD needs this flag, in order to enable it also
>>>> for rte_flow then
>>>> it should be documented in the release note of the PMD.
>>>> It is a valid use case that the default traffic will not have RSS and
>>>> only rules created by
>>>> rte_flow will have the RSS, for matching traffc.
>>>>
>>>> Best,
>>>> Ori

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

Hi, all,

在 2022/3/10 16:08, lihuisong (C) 写道:
>
> 在 2022/3/9 17:55, Ori Kam 写道:
>> Hi Lihusiong,
>>
>>> -----Original Message-----
>>> From: lihuisong (C) <lihuisong@huawei.com>
>>> Sent: Wednesday, March 9, 2022 10:03 AM
>>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS 
>>> behavior
>>>
>>>
>>> 在 2022/3/3 10:47, lihuisong (C) 写道:
>>>> Hi, Ori,
>>>>
>>>> 在 2022/3/2 22:07, Ori Kam 写道:
>>>>> Hi Lihuisong,
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>>> Sent: Wednesday, March 2, 2022 4:10 AM
>>>>>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS
>>>>>> behavior
>>>>>>
>>>>>>
>>>>>> 在 2022/3/1 0:42, Ferruh Yigit 写道:
>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>>>>
>>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be 
>>>>>>>> set in
>>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
>>>>>>>> enabled
>>>>>>>> through the ethdev ops and rte_flow API. This behavior is 
>>>>>>>> contrary to
>>>>>>>> each
>>>>>>>> other.
>>>>>>>>
>>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>
>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>>> Hi Huisong, Connor,
>>>>>>>
>>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>>>>
>>>>>>>
>>>>>>> To enable RSS, multi queue mode should be set to
>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>>>>
>>>>>>> But I wonder if it is required to configure RSS via flow API,
>>>>>> I do not know the original purpose of adding the RSS 
>>>>>> configuration in
>>>>>> flow API.
>>>>>>
>>>>> The purpose is simple, this allow to create RSS per rule and not a
>>>>> global one.
>>>>> For example create RSS that sends TCP to some queues while othe RSS
>>>>> will send
>>>>> UDP traffic to different queues.
>>>> I'm a little confused now. The "per rule" also seems to be a global
>>>> configuration.
>>>> Example:
>>>>   - start PMD with 0,1,2,3
>>>>   - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
>>>> can be received for other types of packets.
>>>> Because this rule is implemented by modifying the entry of the
>>>> redirection table which is global for this device.
>>> Hi, Ori and Stephen.
>>> Can you help me clear up the confusion above? If some NICs behave like
>>> this, what should we do about it?
>> I'm not sure I understand the issue, maybe it is releated to some 
>> HW/PMD limitation.
>> In your example non TCP traffic will be routed to one of the 4 queues 
>> (0,1,2,3),
>> While TCP traffic will only be routed to queues 2,3.
>>
>> Now I can add new rule that matches on UDP packet and RSS to queue 0 
>> and 3 in this case:
>> TCP packets will be routed to queues 0,3.
>> UDP packets will be routed to queues 2,3.
>> All the rest of the traffic will be routed to queues 0,1,2,3
>>
>> And just to be clear if now I add a rule to match all packets in 
>> higher priority,
>> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
>>
>> At least this is what is expected, from API point of view.
>>
>> Best,
>> Ori
> Thank you for your answer. I understand it.
> hns3 PMD cannot implement the above functions due to hardware limitation.
> we may need add a check that specified RSS queues cannot be supported 
> when specified packets types.
> And only the packet type is specified, which meets the requirements of 
> rte_flow API.
> The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
> Thanks, Ori and Stephen😁
>
> But, I think, it is necessary for the '.rss_hash_update' and 
> '.reta_update' APIs
> in eth_dev_ops to verify this flag. What do you think? @Thomas, 
> @Ferruh, @Ori and @Stephen.
What's your take on it? I am looking forward to your reply. Thanks!
>>
>>>>>> However, as far as I know, the hash algorithm can be configured via
>>>>>> this
>>>>>> API,
>>>>>>
>>>>>> but not via ethdev ops API.
>>>>>>
>>>>>>> and if other PMDs check this configuration for flow API?
>>>>>> Some PMDs already have similar check in RSS releated ops or rte_flow
>>>>>> API.
>>>>>>
>>>>>> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
>>>>>>
>>>>>>> .
>>>>>   From my view point those are two different settings.
>>>>> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
>>>>> rte_flow is per rule.
>>>>>
>>>>> I think, that if a PMD needs this flag, in order to enable it also
>>>>> for rte_flow then
>>>>> it should be documented in the release note of the PMD.
>>>>> It is a valid use case that the default traffic will not have RSS and
>>>>> only rules created by
>>>>> rte_flow will have the RSS, for matching traffc.
>>>>>
>>>>> Best,
>>>>> Ori
> .

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Thomas Monjalon]

21/03/2022 08:14, lihuisong (C):
> 2022/3/10 16:08, lihuisong (C):
> > 2022/3/9 17:55, Ori Kam:
> >> From: lihuisong (C) <lihuisong@huawei.com>
> >>> 2022/3/3 10:47, lihuisong (C):
> >>>> 2022/3/2 22:07, Ori Kam:
> >>>>> From: lihuisong (C) <lihuisong@huawei.com>
> >>>>>> 2022/3/1 0:42, Ferruh Yigit:
> >>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> >>>>>>>> From: Huisong Li <lihuisong@huawei.com>
> >>>>>>>>
> >>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be 
> >>>>>>>> set in
> >>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
> >>>>>>>> enabled
> >>>>>>>> through the ethdev ops and rte_flow API. This behavior is 
> >>>>>>>> contrary to
> >>>>>>>> each
> >>>>>>>> other.
> >>>>>>>>
> >>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> >>>>>>>> Cc: stable@dpdk.org
> >>>>>>>>
> >>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >>>>>>> Hi Huisong, Connor,
> >>>>>>>
> >>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
> >>>>>>>
> >>>>>>> To enable RSS, multi queue mode should be set to
> >>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
> >>>>>>>
> >>>>>>> But I wonder if it is required to configure RSS via flow API,
> >>>>>> I do not know the original purpose of adding the RSS 
> >>>>>> configuration in
> >>>>>> flow API.
> >>>>>>
> >>>>> The purpose is simple, this allow to create RSS per rule and not a
> >>>>> global one.
> >>>>> For example create RSS that sends TCP to some queues while othe RSS
> >>>>> will send
> >>>>> UDP traffic to different queues.
> >>>> I'm a little confused now. The "per rule" also seems to be a global
> >>>> configuration.
> >>>> Example:
> >>>>   - start PMD with 0,1,2,3
> >>>>   - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
> >>>> can be received for other types of packets.
> >>>> Because this rule is implemented by modifying the entry of the
> >>>> redirection table which is global for this device.
> >>> Hi, Ori and Stephen.
> >>> Can you help me clear up the confusion above? If some NICs behave like
> >>> this, what should we do about it?
> >> I'm not sure I understand the issue, maybe it is releated to some 
> >> HW/PMD limitation.
> >> In your example non TCP traffic will be routed to one of the 4 queues 
> >> (0,1,2,3),
> >> While TCP traffic will only be routed to queues 2,3.
> >>
> >> Now I can add new rule that matches on UDP packet and RSS to queue 0 
> >> and 3 in this case:
> >> TCP packets will be routed to queues 0,3.
> >> UDP packets will be routed to queues 2,3.
> >> All the rest of the traffic will be routed to queues 0,1,2,3
> >>
> >> And just to be clear if now I add a rule to match all packets in 
> >> higher priority,
> >> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
> >>
> >> At least this is what is expected, from API point of view.
> >>
> >> Best,
> >> Ori
> > Thank you for your answer. I understand it.
> > hns3 PMD cannot implement the above functions due to hardware limitation.
> > we may need add a check that specified RSS queues cannot be supported 
> > when specified packets types.
> > And only the packet type is specified, which meets the requirements of 
> > rte_flow API.
> > The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
> > Thanks, Ori and Stephen😁
> >
> > But, I think, it is necessary for the '.rss_hash_update' and 
> > '.reta_update' APIs
> > in eth_dev_ops to verify this flag. What do you think? @Thomas, 
> > @Ferruh, @Ori and @Stephen.
> What's your take on it? I am looking forward to your reply. Thanks!

I am not sure why you want to check this flag.
I can imagine we configure the hash and the table before enabling RSS
with the RTE_ETH_MQ_RX_RSS_FLAG flag.

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

在 2022/3/23 1:13, Thomas Monjalon 写道:
> 21/03/2022 08:14, lihuisong (C):
>> 2022/3/10 16:08, lihuisong (C):
>>> 2022/3/9 17:55, Ori Kam:
>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>> 2022/3/3 10:47, lihuisong (C):
>>>>>> 2022/3/2 22:07, Ori Kam:
>>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>>>>> 2022/3/1 0:42, Ferruh Yigit:
>>>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>>>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>>>>>>
>>>>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be
>>>>>>>>>> set in
>>>>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
>>>>>>>>>> enabled
>>>>>>>>>> through the ethdev ops and rte_flow API. This behavior is
>>>>>>>>>> contrary to
>>>>>>>>>> each
>>>>>>>>>> other.
>>>>>>>>>>
>>>>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>>>>> Hi Huisong, Connor,
>>>>>>>>>
>>>>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>>>>>>
>>>>>>>>> To enable RSS, multi queue mode should be set to
>>>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>>>>>>
>>>>>>>>> But I wonder if it is required to configure RSS via flow API,
>>>>>>>> I do not know the original purpose of adding the RSS
>>>>>>>> configuration in
>>>>>>>> flow API.
>>>>>>>>
>>>>>>> The purpose is simple, this allow to create RSS per rule and not a
>>>>>>> global one.
>>>>>>> For example create RSS that sends TCP to some queues while othe RSS
>>>>>>> will send
>>>>>>> UDP traffic to different queues.
>>>>>> I'm a little confused now. The "per rule" also seems to be a global
>>>>>> configuration.
>>>>>> Example:
>>>>>>    - start PMD with 0,1,2,3
>>>>>>    - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
>>>>>> can be received for other types of packets.
>>>>>> Because this rule is implemented by modifying the entry of the
>>>>>> redirection table which is global for this device.
>>>>> Hi, Ori and Stephen.
>>>>> Can you help me clear up the confusion above? If some NICs behave like
>>>>> this, what should we do about it?
>>>> I'm not sure I understand the issue, maybe it is releated to some
>>>> HW/PMD limitation.
>>>> In your example non TCP traffic will be routed to one of the 4 queues
>>>> (0,1,2,3),
>>>> While TCP traffic will only be routed to queues 2,3.
>>>>
>>>> Now I can add new rule that matches on UDP packet and RSS to queue 0
>>>> and 3 in this case:
>>>> TCP packets will be routed to queues 0,3.
>>>> UDP packets will be routed to queues 2,3.
>>>> All the rest of the traffic will be routed to queues 0,1,2,3
>>>>
>>>> And just to be clear if now I add a rule to match all packets in
>>>> higher priority,
>>>> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
>>>>
>>>> At least this is what is expected, from API point of view.
>>>>
>>>> Best,
>>>> Ori
>>> Thank you for your answer. I understand it.
>>> hns3 PMD cannot implement the above functions due to hardware limitation.
>>> we may need add a check that specified RSS queues cannot be supported
>>> when specified packets types.
>>> And only the packet type is specified, which meets the requirements of
>>> rte_flow API.
>>> The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
>>> Thanks, Ori and Stephen😁
>>>
>>> But, I think, it is necessary for the '.rss_hash_update' and
>>> '.reta_update' APIs
>>> in eth_dev_ops to verify this flag. What do you think? @Thomas,
>>> @Ferruh, @Ori and @Stephen.
>> What's your take on it? I am looking forward to your reply. Thanks!
> I am not sure why you want to check this flag.
I want to make sure that the behavior that PMD configured RSS is
consistent across different interfaces. The RTE_ETH_MQ_RX_RSS_FLAG
flag is a switch to enable RSS hash. If the switch isn't open, some
PMD do not configure RSS function. I think the consistency is necessary.
If not set RSS muti-queue mode, it is unnecessary to configure RSS.
> I can imagine we configure the hash and the table before enabling RSS
> with the RTE_ETH_MQ_RX_RSS_FLAG flag.
The flag is derived from dev_configure() which also configures
hash and key. I don't think it makes sense to configure hash and
reta before calling dev_configure. Because they'll be updated.
This is similar to configuring mtu.
>
>
> .

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Thomas Monjalon]

23/03/2022 04:05, lihuisong (C):
> 在 2022/3/23 1:13, Thomas Monjalon 写道:
> > 21/03/2022 08:14, lihuisong (C):
> >> 2022/3/10 16:08, lihuisong (C):
> >>> 2022/3/9 17:55, Ori Kam:
> >>>> From: lihuisong (C) <lihuisong@huawei.com>
> >>>>> 2022/3/3 10:47, lihuisong (C):
> >>>>>> 2022/3/2 22:07, Ori Kam:
> >>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
> >>>>>>>> 2022/3/1 0:42, Ferruh Yigit:
> >>>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> >>>>>>>>>> From: Huisong Li <lihuisong@huawei.com>
> >>>>>>>>>>
> >>>>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be
> >>>>>>>>>> set in
> >>>>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
> >>>>>>>>>> enabled
> >>>>>>>>>> through the ethdev ops and rte_flow API. This behavior is
> >>>>>>>>>> contrary to
> >>>>>>>>>> each
> >>>>>>>>>> other.
> >>>>>>>>>>
> >>>>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> >>>>>>>>>> Cc: stable@dpdk.org
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >>>>>>>>> Hi Huisong, Connor,
> >>>>>>>>>
> >>>>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
> >>>>>>>>>
> >>>>>>>>> To enable RSS, multi queue mode should be set to
> >>>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
> >>>>>>>>>
> >>>>>>>>> But I wonder if it is required to configure RSS via flow API,
> >>>>>>>> I do not know the original purpose of adding the RSS
> >>>>>>>> configuration in
> >>>>>>>> flow API.
> >>>>>>>>
> >>>>>>> The purpose is simple, this allow to create RSS per rule and not a
> >>>>>>> global one.
> >>>>>>> For example create RSS that sends TCP to some queues while othe RSS
> >>>>>>> will send
> >>>>>>> UDP traffic to different queues.
> >>>>>> I'm a little confused now. The "per rule" also seems to be a global
> >>>>>> configuration.
> >>>>>> Example:
> >>>>>>    - start PMD with 0,1,2,3
> >>>>>>    - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
> >>>>>> can be received for other types of packets.
> >>>>>> Because this rule is implemented by modifying the entry of the
> >>>>>> redirection table which is global for this device.
> >>>>> Hi, Ori and Stephen.
> >>>>> Can you help me clear up the confusion above? If some NICs behave like
> >>>>> this, what should we do about it?
> >>>> I'm not sure I understand the issue, maybe it is releated to some
> >>>> HW/PMD limitation.
> >>>> In your example non TCP traffic will be routed to one of the 4 queues
> >>>> (0,1,2,3),
> >>>> While TCP traffic will only be routed to queues 2,3.
> >>>>
> >>>> Now I can add new rule that matches on UDP packet and RSS to queue 0
> >>>> and 3 in this case:
> >>>> TCP packets will be routed to queues 0,3.
> >>>> UDP packets will be routed to queues 2,3.
> >>>> All the rest of the traffic will be routed to queues 0,1,2,3
> >>>>
> >>>> And just to be clear if now I add a rule to match all packets in
> >>>> higher priority,
> >>>> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
> >>>>
> >>>> At least this is what is expected, from API point of view.
> >>>>
> >>>> Best,
> >>>> Ori
> >>> Thank you for your answer. I understand it.
> >>> hns3 PMD cannot implement the above functions due to hardware limitation.
> >>> we may need add a check that specified RSS queues cannot be supported
> >>> when specified packets types.
> >>> And only the packet type is specified, which meets the requirements of
> >>> rte_flow API.
> >>> The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
> >>> Thanks, Ori and Stephen😁
> >>>
> >>> But, I think, it is necessary for the '.rss_hash_update' and
> >>> '.reta_update' APIs
> >>> in eth_dev_ops to verify this flag. What do you think? @Thomas,
> >>> @Ferruh, @Ori and @Stephen.
> >> What's your take on it? I am looking forward to your reply. Thanks!
> > 
> > I am not sure why you want to check this flag.
> 
> I want to make sure that the behavior that PMD configured RSS is
> consistent across different interfaces. The RTE_ETH_MQ_RX_RSS_FLAG
> flag is a switch to enable RSS hash. If the switch isn't open, some
> PMD do not configure RSS function. I think the consistency is necessary.
> If not set RSS muti-queue mode, it is unnecessary to configure RSS.
> 
> > I can imagine we configure the hash and the table before enabling RSS
> > with the RTE_ETH_MQ_RX_RSS_FLAG flag.
> 
> The flag is derived from dev_configure() which also configures
> hash and key. I don't think it makes sense to configure hash and
> reta before calling dev_configure. Because they'll be updated.
> This is similar to configuring mtu.

OK I see your point.
So you would like to return an error in RSS functions
if the flag RTE_ETH_MQ_RX_RSS_FLAG is not set?
Should it be checked in ethdev library or PMDs?

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

在 2022/3/23 17:14, Thomas Monjalon 写道:
> 23/03/2022 04:05, lihuisong (C):
>> 在 2022/3/23 1:13, Thomas Monjalon 写道:
>>> 21/03/2022 08:14, lihuisong (C):
>>>> 2022/3/10 16:08, lihuisong (C):
>>>>> 2022/3/9 17:55, Ori Kam:
>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>>>> 2022/3/3 10:47, lihuisong (C):
>>>>>>>> 2022/3/2 22:07, Ori Kam:
>>>>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>>>>>>> 2022/3/1 0:42, Ferruh Yigit:
>>>>>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>>>>>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>>>>>>>>
>>>>>>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be
>>>>>>>>>>>> set in
>>>>>>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
>>>>>>>>>>>> enabled
>>>>>>>>>>>> through the ethdev ops and rte_flow API. This behavior is
>>>>>>>>>>>> contrary to
>>>>>>>>>>>> each
>>>>>>>>>>>> other.
>>>>>>>>>>>>
>>>>>>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>>>>>>> Hi Huisong, Connor,
>>>>>>>>>>>
>>>>>>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>>>>>>>>
>>>>>>>>>>> To enable RSS, multi queue mode should be set to
>>>>>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>>>>>>>>
>>>>>>>>>>> But I wonder if it is required to configure RSS via flow API,
>>>>>>>>>> I do not know the original purpose of adding the RSS
>>>>>>>>>> configuration in
>>>>>>>>>> flow API.
>>>>>>>>>>
>>>>>>>>> The purpose is simple, this allow to create RSS per rule and not a
>>>>>>>>> global one.
>>>>>>>>> For example create RSS that sends TCP to some queues while othe RSS
>>>>>>>>> will send
>>>>>>>>> UDP traffic to different queues.
>>>>>>>> I'm a little confused now. The "per rule" also seems to be a global
>>>>>>>> configuration.
>>>>>>>> Example:
>>>>>>>>     - start PMD with 0,1,2,3
>>>>>>>>     - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
>>>>>>>> can be received for other types of packets.
>>>>>>>> Because this rule is implemented by modifying the entry of the
>>>>>>>> redirection table which is global for this device.
>>>>>>> Hi, Ori and Stephen.
>>>>>>> Can you help me clear up the confusion above? If some NICs behave like
>>>>>>> this, what should we do about it?
>>>>>> I'm not sure I understand the issue, maybe it is releated to some
>>>>>> HW/PMD limitation.
>>>>>> In your example non TCP traffic will be routed to one of the 4 queues
>>>>>> (0,1,2,3),
>>>>>> While TCP traffic will only be routed to queues 2,3.
>>>>>>
>>>>>> Now I can add new rule that matches on UDP packet and RSS to queue 0
>>>>>> and 3 in this case:
>>>>>> TCP packets will be routed to queues 0,3.
>>>>>> UDP packets will be routed to queues 2,3.
>>>>>> All the rest of the traffic will be routed to queues 0,1,2,3
>>>>>>
>>>>>> And just to be clear if now I add a rule to match all packets in
>>>>>> higher priority,
>>>>>> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
>>>>>>
>>>>>> At least this is what is expected, from API point of view.
>>>>>>
>>>>>> Best,
>>>>>> Ori
>>>>> Thank you for your answer. I understand it.
>>>>> hns3 PMD cannot implement the above functions due to hardware limitation.
>>>>> we may need add a check that specified RSS queues cannot be supported
>>>>> when specified packets types.
>>>>> And only the packet type is specified, which meets the requirements of
>>>>> rte_flow API.
>>>>> The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
>>>>> Thanks, Ori and Stephen😁
>>>>>
>>>>> But, I think, it is necessary for the '.rss_hash_update' and
>>>>> '.reta_update' APIs
>>>>> in eth_dev_ops to verify this flag. What do you think? @Thomas,
>>>>> @Ferruh, @Ori and @Stephen.
>>>> What's your take on it? I am looking forward to your reply. Thanks!
>>> I am not sure why you want to check this flag.
>> I want to make sure that the behavior that PMD configured RSS is
>> consistent across different interfaces. The RTE_ETH_MQ_RX_RSS_FLAG
>> flag is a switch to enable RSS hash. If the switch isn't open, some
>> PMD do not configure RSS function. I think the consistency is necessary.
>> If not set RSS muti-queue mode, it is unnecessary to configure RSS.
>>
>>> I can imagine we configure the hash and the table before enabling RSS
>>> with the RTE_ETH_MQ_RX_RSS_FLAG flag.
>> The flag is derived from dev_configure() which also configures
>> hash and key. I don't think it makes sense to configure hash and
>> reta before calling dev_configure. Because they'll be updated.
>> This is similar to configuring mtu.
> OK I see your point.
> So you would like to return an error in RSS functions
> if the flag RTE_ETH_MQ_RX_RSS_FLAG is not set?
Yes
> Should it be checked in ethdev library or PMDs?
I think it's better to put it in the ethdev layer if we do it.
Should we add this check in 'rss_hash_update' and 'reta_update' APIs?
>
>
>
> .

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Thomas Monjalon]

23/03/2022 12:04, lihuisong (C):
> 在 2022/3/23 17:14, Thomas Monjalon 写道:
> > 23/03/2022 04:05, lihuisong (C):
> >> 在 2022/3/23 1:13, Thomas Monjalon 写道:
> >>> 21/03/2022 08:14, lihuisong (C):
> >>>> 2022/3/10 16:08, lihuisong (C):
> >>>>> 2022/3/9 17:55, Ori Kam:
> >>>>>> From: lihuisong (C) <lihuisong@huawei.com>
> >>>>>>> 2022/3/3 10:47, lihuisong (C):
> >>>>>>>> 2022/3/2 22:07, Ori Kam:
> >>>>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
> >>>>>>>>>> 2022/3/1 0:42, Ferruh Yigit:
> >>>>>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> >>>>>>>>>>>> From: Huisong Li <lihuisong@huawei.com>
> >>>>>>>>>>>>
> >>>>>>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be
> >>>>>>>>>>>> set in
> >>>>>>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
> >>>>>>>>>>>> enabled
> >>>>>>>>>>>> through the ethdev ops and rte_flow API. This behavior is
> >>>>>>>>>>>> contrary to
> >>>>>>>>>>>> each
> >>>>>>>>>>>> other.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> >>>>>>>>>>>> Cc: stable@dpdk.org
> >>>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >>>>>>>>>>> Hi Huisong, Connor,
> >>>>>>>>>>>
> >>>>>>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
> >>>>>>>>>>>
> >>>>>>>>>>> To enable RSS, multi queue mode should be set to
> >>>>>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
> >>>>>>>>>>>
> >>>>>>>>>>> But I wonder if it is required to configure RSS via flow API,
> >>>>>>>>>> I do not know the original purpose of adding the RSS
> >>>>>>>>>> configuration in
> >>>>>>>>>> flow API.
> >>>>>>>>>>
> >>>>>>>>> The purpose is simple, this allow to create RSS per rule and not a
> >>>>>>>>> global one.
> >>>>>>>>> For example create RSS that sends TCP to some queues while othe RSS
> >>>>>>>>> will send
> >>>>>>>>> UDP traffic to different queues.
> >>>>>>>> I'm a little confused now. The "per rule" also seems to be a global
> >>>>>>>> configuration.
> >>>>>>>> Example:
> >>>>>>>>     - start PMD with 0,1,2,3
> >>>>>>>>     - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
> >>>>>>>> can be received for other types of packets.
> >>>>>>>> Because this rule is implemented by modifying the entry of the
> >>>>>>>> redirection table which is global for this device.
> >>>>>>> Hi, Ori and Stephen.
> >>>>>>> Can you help me clear up the confusion above? If some NICs behave like
> >>>>>>> this, what should we do about it?
> >>>>>> I'm not sure I understand the issue, maybe it is releated to some
> >>>>>> HW/PMD limitation.
> >>>>>> In your example non TCP traffic will be routed to one of the 4 queues
> >>>>>> (0,1,2,3),
> >>>>>> While TCP traffic will only be routed to queues 2,3.
> >>>>>>
> >>>>>> Now I can add new rule that matches on UDP packet and RSS to queue 0
> >>>>>> and 3 in this case:
> >>>>>> TCP packets will be routed to queues 0,3.
> >>>>>> UDP packets will be routed to queues 2,3.
> >>>>>> All the rest of the traffic will be routed to queues 0,1,2,3
> >>>>>>
> >>>>>> And just to be clear if now I add a rule to match all packets in
> >>>>>> higher priority,
> >>>>>> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
> >>>>>>
> >>>>>> At least this is what is expected, from API point of view.
> >>>>>>
> >>>>>> Best,
> >>>>>> Ori
> >>>>> Thank you for your answer. I understand it.
> >>>>> hns3 PMD cannot implement the above functions due to hardware limitation.
> >>>>> we may need add a check that specified RSS queues cannot be supported
> >>>>> when specified packets types.
> >>>>> And only the packet type is specified, which meets the requirements of
> >>>>> rte_flow API.
> >>>>> The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
> >>>>> Thanks, Ori and Stephen😁
> >>>>>
> >>>>> But, I think, it is necessary for the '.rss_hash_update' and
> >>>>> '.reta_update' APIs
> >>>>> in eth_dev_ops to verify this flag. What do you think? @Thomas,
> >>>>> @Ferruh, @Ori and @Stephen.
> >>>> What's your take on it? I am looking forward to your reply. Thanks!
> >>> I am not sure why you want to check this flag.
> >> I want to make sure that the behavior that PMD configured RSS is
> >> consistent across different interfaces. The RTE_ETH_MQ_RX_RSS_FLAG
> >> flag is a switch to enable RSS hash. If the switch isn't open, some
> >> PMD do not configure RSS function. I think the consistency is necessary.
> >> If not set RSS muti-queue mode, it is unnecessary to configure RSS.
> >>
> >>> I can imagine we configure the hash and the table before enabling RSS
> >>> with the RTE_ETH_MQ_RX_RSS_FLAG flag.
> >> The flag is derived from dev_configure() which also configures
> >> hash and key. I don't think it makes sense to configure hash and
> >> reta before calling dev_configure. Because they'll be updated.
> >> This is similar to configuring mtu.
> > OK I see your point.
> > So you would like to return an error in RSS functions
> > if the flag RTE_ETH_MQ_RX_RSS_FLAG is not set?
> Yes
> > Should it be checked in ethdev library or PMDs?
> I think it's better to put it in the ethdev layer if we do it.
> Should we add this check in 'rss_hash_update' and 'reta_update' APIs?

I'm OK with adding the check in ethdev.

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 5721 bytes --]

On Wed, Mar 23, 2022 at 12:04 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 23/03/2022 12:04, lihuisong (C):
> > 在 2022/3/23 17:14, Thomas Monjalon 写道:
> > > 23/03/2022 04:05, lihuisong (C):
> > >> 在 2022/3/23 1:13, Thomas Monjalon 写道:
> > >>> 21/03/2022 08:14, lihuisong (C):
> > >>>> 2022/3/10 16:08, lihuisong (C):
> > >>>>> 2022/3/9 17:55, Ori Kam:
> > >>>>>> From: lihuisong (C) <lihuisong@huawei.com>
> > >>>>>>> 2022/3/3 10:47, lihuisong (C):
> > >>>>>>>> 2022/3/2 22:07, Ori Kam:
> > >>>>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
> > >>>>>>>>>> 2022/3/1 0:42, Ferruh Yigit:
> > >>>>>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> > >>>>>>>>>>>> From: Huisong Li <lihuisong@huawei.com>
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be
> > >>>>>>>>>>>> set in
> > >>>>>>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
> > >>>>>>>>>>>> enabled
> > >>>>>>>>>>>> through the ethdev ops and rte_flow API. This behavior is
> > >>>>>>>>>>>> contrary to
> > >>>>>>>>>>>> each
> > >>>>>>>>>>>> other.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> > >>>>>>>>>>>> Cc: stable@dpdk.org
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> > >>>>>>>>>>> Hi Huisong, Connor,
> > >>>>>>>>>>>
> > >>>>>>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
> > >>>>>>>>>>>
> > >>>>>>>>>>> To enable RSS, multi queue mode should be set to
> > >>>>>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
> > >>>>>>>>>>>
> > >>>>>>>>>>> But I wonder if it is required to configure RSS via flow API,
> > >>>>>>>>>> I do not know the original purpose of adding the RSS
> > >>>>>>>>>> configuration in
> > >>>>>>>>>> flow API.
> > >>>>>>>>>>
> > >>>>>>>>> The purpose is simple, this allow to create RSS per rule and not a
> > >>>>>>>>> global one.
> > >>>>>>>>> For example create RSS that sends TCP to some queues while othe RSS
> > >>>>>>>>> will send
> > >>>>>>>>> UDP traffic to different queues.
> > >>>>>>>> I'm a little confused now. The "per rule" also seems to be a global
> > >>>>>>>> configuration.
> > >>>>>>>> Example:
> > >>>>>>>>     - start PMD with 0,1,2,3
> > >>>>>>>>     - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
> > >>>>>>>> can be received for other types of packets.
> > >>>>>>>> Because this rule is implemented by modifying the entry of the
> > >>>>>>>> redirection table which is global for this device.
> > >>>>>>> Hi, Ori and Stephen.
> > >>>>>>> Can you help me clear up the confusion above? If some NICs behave like
> > >>>>>>> this, what should we do about it?
> > >>>>>> I'm not sure I understand the issue, maybe it is releated to some
> > >>>>>> HW/PMD limitation.
> > >>>>>> In your example non TCP traffic will be routed to one of the 4 queues
> > >>>>>> (0,1,2,3),
> > >>>>>> While TCP traffic will only be routed to queues 2,3.
> > >>>>>>
> > >>>>>> Now I can add new rule that matches on UDP packet and RSS to queue 0
> > >>>>>> and 3 in this case:
> > >>>>>> TCP packets will be routed to queues 0,3.
> > >>>>>> UDP packets will be routed to queues 2,3.
> > >>>>>> All the rest of the traffic will be routed to queues 0,1,2,3
> > >>>>>>
> > >>>>>> And just to be clear if now I add a rule to match all packets in
> > >>>>>> higher priority,
> > >>>>>> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
> > >>>>>>
> > >>>>>> At least this is what is expected, from API point of view.
> > >>>>>>
> > >>>>>> Best,
> > >>>>>> Ori
> > >>>>> Thank you for your answer. I understand it.
> > >>>>> hns3 PMD cannot implement the above functions due to hardware limitation.
> > >>>>> we may need add a check that specified RSS queues cannot be supported
> > >>>>> when specified packets types.
> > >>>>> And only the packet type is specified, which meets the requirements of
> > >>>>> rte_flow API.
> > >>>>> The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
> > >>>>> Thanks, Ori and Stephen😁
> > >>>>>
> > >>>>> But, I think, it is necessary for the '.rss_hash_update' and
> > >>>>> '.reta_update' APIs
> > >>>>> in eth_dev_ops to verify this flag. What do you think? @Thomas,
> > >>>>> @Ferruh, @Ori and @Stephen.
> > >>>> What's your take on it? I am looking forward to your reply. Thanks!
> > >>> I am not sure why you want to check this flag.
> > >> I want to make sure that the behavior that PMD configured RSS is
> > >> consistent across different interfaces. The RTE_ETH_MQ_RX_RSS_FLAG
> > >> flag is a switch to enable RSS hash. If the switch isn't open, some
> > >> PMD do not configure RSS function. I think the consistency is necessary.
> > >> If not set RSS muti-queue mode, it is unnecessary to configure RSS.
> > >>
> > >>> I can imagine we configure the hash and the table before enabling RSS
> > >>> with the RTE_ETH_MQ_RX_RSS_FLAG flag.
> > >> The flag is derived from dev_configure() which also configures
> > >> hash and key. I don't think it makes sense to configure hash and
> > >> reta before calling dev_configure. Because they'll be updated.
> > >> This is similar to configuring mtu.
> > > OK I see your point.
> > > So you would like to return an error in RSS functions
> > > if the flag RTE_ETH_MQ_RX_RSS_FLAG is not set?
> > Yes
> > > Should it be checked in ethdev library or PMDs?
> > I think it's better to put it in the ethdev layer if we do it.
> > Should we add this check in 'rss_hash_update' and 'reta_update' APIs?
>
> I'm OK with adding the check in ethdev.
+1

Apologies for getting late to this thread.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Harold Huang]

Hi, all,

On Thu, Mar 3, 2022 at 12:59 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Wed, 02 Mar 2022 15:46:37 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > 02/03/2022 15:07, Ori Kam:
> > > From: lihuisong (C) <lihuisong@huawei.com>
> > > > 在 2022/3/1 0:42, Ferruh Yigit 写道:
> > > > > On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> > > > >> From: Huisong Li <lihuisong@huawei.com>
> > > > >>
> > > > >> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> > > > >> dev_configure phase. However, if this flag isn't set, RSS can be enabled
> > > > >> through the ethdev ops and rte_flow API. This behavior is contrary to
> > > > >> each
> > > > >> other.
> > > > >>
> > > > >> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> > > > >> Cc: stable@dpdk.org
> > > > >>
> > > > >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> > > > >
> > > > >
> > > > > Hi Huisong, Connor,
> > > > >
> > > > > Let's get a little more feedback for this patch, cc'ed more people.
> > > > >
> > > > >
> > > > > To enable RSS, multi queue mode should be set to
> > > > > 'RTE_ETH_MQ_RX_RSS_FLAG'.
> > > > >
> > > > > But I wonder if it is required to configure RSS via flow API,
> > > >
> > > > I do not know the original purpose of adding the RSS configuration in
> > > > flow API.

In OVS-DPDK, there is a use case to add mark+rss action via flow API. See [1]:
[1]: https://github.com/openvswitch/ovs/blob/master/lib/netdev-offload-dpdk.c#L1677

> > > >
> > > The purpose is simple, this allow to create RSS per rule and not a global one.
> > > For example create RSS that sends TCP to some queues while othe RSS will send
> > > UDP traffic to different queues.
> > >
> > > > However, as far as I know, the hash algorithm can be configured via this
> > > > API,
> > > >
> > > > but not via ethdev ops API.
> > > >
> > > > > and if other PMDs check this configuration for flow API?
> > > >
> > > > Some PMDs already have similar check in RSS releated ops or rte_flow API.
> > > >
> > > > For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
> >
> > I suggest removing these checks.
> >
> > > From my view point those are two different settings.
> > > The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
> > > rte_flow is per rule.
> > >
> > > I think, that if a PMD needs this flag, in order to enable it also for rte_flow then
> > > it should be documented in the release note of the PMD.
> > > It is a valid use case that the default traffic will not have RSS and only rules created by
> > > rte_flow will have the RSS, for matching traffc.
> >
> > I think RTE_ETH_MQ_RX_RSS_FLAG should not be required by any PMD
> > for rte_flow RSS rules.
> >
> >
>
> Agreed.
>
> The ideal state around RSS would be:
>    - global settings affect the default started queues when device is started.
>      if RSS is enabled and 4 queues are started, then RSS would happen for received
>      packets across those 4 queues.
>    - when adding new flow related rx:
>       - new queues could be started - BUT not part of original RSS
>       - new rte_flow rule can do RSS against the new flow:
>             example:
>               start queues 4,5,6,7
>               rte_flow match TCP port 80 and process on queues 4,5,6,7 with RSS
>
> This is a mess today, most devices work differently and there is no reference software
> implementation.  In an ideal state, there would be a reference software implementation
> that implements all of rte_flow, and all new hardware support would have to work the same
> as the reference implementation. And there would be a full CI test suite around rte_flow.

When adding a new flow with RSS action, does the RSS hash type have to
be set or it could inherit the global setting? I see there are some
different behaviors when we use RSS actions without hash types. In my
test, the E810 and Broadcom NetXtreme-E work well but there are some
problems in MLX5. There is a patch in [2]. Anyone could give some
suggestions?

[2]: https://patchwork.ozlabs.org/project/openvswitch/patch/20220316080842.811478-1-baymaxhuang@gmail.com/

-- 
Thanks, Harold.

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[lihuisong (C)]

在 2022/3/24 3:50, Ajit Khaparde 写道:
> On Wed, Mar 23, 2022 at 12:04 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>> 23/03/2022 12:04, lihuisong (C):
>>> 在 2022/3/23 17:14, Thomas Monjalon 写道:
>>>> 23/03/2022 04:05, lihuisong (C):
>>>>> 在 2022/3/23 1:13, Thomas Monjalon 写道:
>>>>>> 21/03/2022 08:14, lihuisong (C):
>>>>>>> 2022/3/10 16:08, lihuisong (C):
>>>>>>>> 2022/3/9 17:55, Ori Kam:
>>>>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>>>>>>> 2022/3/3 10:47, lihuisong (C):
>>>>>>>>>>> 2022/3/2 22:07, Ori Kam:
>>>>>>>>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>>>>>>>>>> 2022/3/1 0:42, Ferruh Yigit:
>>>>>>>>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>>>>>>>>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be
>>>>>>>>>>>>>>> set in
>>>>>>>>>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
>>>>>>>>>>>>>>> enabled
>>>>>>>>>>>>>>> through the ethdev ops and rte_flow API. This behavior is
>>>>>>>>>>>>>>> contrary to
>>>>>>>>>>>>>>> each
>>>>>>>>>>>>>>> other.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>>>>>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>>>>>>>>>> Hi Huisong, Connor,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> To enable RSS, multi queue mode should be set to
>>>>>>>>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> But I wonder if it is required to configure RSS via flow API,
>>>>>>>>>>>>> I do not know the original purpose of adding the RSS
>>>>>>>>>>>>> configuration in
>>>>>>>>>>>>> flow API.
>>>>>>>>>>>>>
>>>>>>>>>>>> The purpose is simple, this allow to create RSS per rule and not a
>>>>>>>>>>>> global one.
>>>>>>>>>>>> For example create RSS that sends TCP to some queues while othe RSS
>>>>>>>>>>>> will send
>>>>>>>>>>>> UDP traffic to different queues.
>>>>>>>>>>> I'm a little confused now. The "per rule" also seems to be a global
>>>>>>>>>>> configuration.
>>>>>>>>>>> Example:
>>>>>>>>>>>      - start PMD with 0,1,2,3
>>>>>>>>>>>      - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
>>>>>>>>>>> can be received for other types of packets.
>>>>>>>>>>> Because this rule is implemented by modifying the entry of the
>>>>>>>>>>> redirection table which is global for this device.
>>>>>>>>>> Hi, Ori and Stephen.
>>>>>>>>>> Can you help me clear up the confusion above? If some NICs behave like
>>>>>>>>>> this, what should we do about it?
>>>>>>>>> I'm not sure I understand the issue, maybe it is releated to some
>>>>>>>>> HW/PMD limitation.
>>>>>>>>> In your example non TCP traffic will be routed to one of the 4 queues
>>>>>>>>> (0,1,2,3),
>>>>>>>>> While TCP traffic will only be routed to queues 2,3.
>>>>>>>>>
>>>>>>>>> Now I can add new rule that matches on UDP packet and RSS to queue 0
>>>>>>>>> and 3 in this case:
>>>>>>>>> TCP packets will be routed to queues 0,3.
>>>>>>>>> UDP packets will be routed to queues 2,3.
>>>>>>>>> All the rest of the traffic will be routed to queues 0,1,2,3
>>>>>>>>>
>>>>>>>>> And just to be clear if now I add a rule to match all packets in
>>>>>>>>> higher priority,
>>>>>>>>> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
>>>>>>>>>
>>>>>>>>> At least this is what is expected, from API point of view.
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>> Ori
>>>>>>>> Thank you for your answer. I understand it.
>>>>>>>> hns3 PMD cannot implement the above functions due to hardware limitation.
>>>>>>>> we may need add a check that specified RSS queues cannot be supported
>>>>>>>> when specified packets types.
>>>>>>>> And only the packet type is specified, which meets the requirements of
>>>>>>>> rte_flow API.
>>>>>>>> The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
>>>>>>>> Thanks, Ori and Stephen😁
>>>>>>>>
>>>>>>>> But, I think, it is necessary for the '.rss_hash_update' and
>>>>>>>> '.reta_update' APIs
>>>>>>>> in eth_dev_ops to verify this flag. What do you think? @Thomas,
>>>>>>>> @Ferruh, @Ori and @Stephen.
>>>>>>> What's your take on it? I am looking forward to your reply. Thanks!
>>>>>> I am not sure why you want to check this flag.
>>>>> I want to make sure that the behavior that PMD configured RSS is
>>>>> consistent across different interfaces. The RTE_ETH_MQ_RX_RSS_FLAG
>>>>> flag is a switch to enable RSS hash. If the switch isn't open, some
>>>>> PMD do not configure RSS function. I think the consistency is necessary.
>>>>> If not set RSS muti-queue mode, it is unnecessary to configure RSS.
>>>>>
>>>>>> I can imagine we configure the hash and the table before enabling RSS
>>>>>> with the RTE_ETH_MQ_RX_RSS_FLAG flag.
>>>>> The flag is derived from dev_configure() which also configures
>>>>> hash and key. I don't think it makes sense to configure hash and
>>>>> reta before calling dev_configure. Because they'll be updated.
>>>>> This is similar to configuring mtu.
>>>> OK I see your point.
>>>> So you would like to return an error in RSS functions
>>>> if the flag RTE_ETH_MQ_RX_RSS_FLAG is not set?
>>> Yes
>>>> Should it be checked in ethdev library or PMDs?
>>> I think it's better to put it in the ethdev layer if we do it.
>>> Should we add this check in 'rss_hash_update' and 'reta_update' APIs?
>> I'm OK with adding the check in ethdev.
> +1
>
> Apologies for getting late to this thread.
Thanks. I'll fix it.

[PATCH v2 0/2] fix RSS bugs

[Min Hu (Connor)]

This patch contains following patches:
ethdev: fix enabling RSS behavior inconsistent
net/hns3: remove unnecessary RSS switch

Huisong Li (2):
  ethdev: fix enabling RSS behavior inconsistent
  net/hns3: remove unnecessary RSS switch

 drivers/net/hns3/hns3_ethdev.c    |  2 --
 drivers/net/hns3/hns3_ethdev.h    |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c |  2 --
 drivers/net/hns3/hns3_flow.c      |  1 -
 drivers/net/hns3/hns3_rss.c       |  3 ---
 lib/ethdev/rte_ethdev.c           | 15 +++++++++++++++
 6 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.33.0

[PATCH v2 1/2] ethdev: fix enabling RSS behavior inconsistent

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

The RTE_ETH_MQ_RX_RSS_FLAG flag is a switch to enable RSS. If the flag is
not set in dev_configure, RSS will be not configured and enabled. However,
RSS hash and reta can still be configured by ethdev ops to enable RSS if
the flag isn't set. The behavior is inconsistent.

Fixes: 99a2dd955fba ("lib: remove librte_ prefix from directory names")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 29a3d80466..8520aec561 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3867,6 +3867,7 @@ rte_eth_dev_rss_reta_update(uint16_t port_id,
 			    struct rte_eth_rss_reta_entry64 *reta_conf,
 			    uint16_t reta_size)
 {
+	enum rte_eth_rx_mq_mode mq_mode;
 	struct rte_eth_dev *dev;
 	int ret;
 
@@ -3898,6 +3899,12 @@ rte_eth_dev_rss_reta_update(uint16_t port_id,
 	if (ret < 0)
 		return ret;
 
+	mq_mode = dev->data->dev_conf.rxmode.mq_mode;
+	if (!(mq_mode & RTE_ETH_MQ_RX_RSS_FLAG)) {
+		RTE_ETHDEV_LOG(ERR, "Multi-queue RSS mode isn't enabled.\n");
+		return -ENOTSUP;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->reta_update, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->reta_update)(dev, reta_conf,
 							     reta_size));
@@ -3937,6 +3944,7 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
 {
 	struct rte_eth_dev *dev;
 	struct rte_eth_dev_info dev_info = { .flow_type_rss_offloads = 0, };
+	enum rte_eth_rx_mq_mode mq_mode;
 	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
@@ -3962,6 +3970,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
 			dev_info.flow_type_rss_offloads);
 		return -EINVAL;
 	}
+
+	mq_mode = dev->data->dev_conf.rxmode.mq_mode;
+	if (!(mq_mode & RTE_ETH_MQ_RX_RSS_FLAG)) {
+		RTE_ETHDEV_LOG(ERR, "Multi-queue RSS mode isn't enabled.\n");
+		return -ENOTSUP;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_update, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
 								 rss_conf));
-- 
2.33.0

[PATCH v2 2/2] net/hns3: remove unnecessary RSS switch

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

Whether the RSS is enabled depends on RTE_ETH_MQ_RX_RSS_FLAG and packet
tuple are enabled. So the RSS switch is unnecessary.

Fixes: 5e782bc2570c ("net/hns3: fix configuring RSS hash when rules are flushed")
Fixes: fd8196838763 ("net/hns3: fix configuring device with RSS enabled")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 2 --
 drivers/net/hns3/hns3_ethdev.h    | 1 -
 drivers/net/hns3/hns3_ethdev_vf.c | 2 --
 drivers/net/hns3/hns3_flow.c      | 1 -
 drivers/net/hns3/hns3_rss.c       | 3 ---
 5 files changed, 9 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 4e089e682f..24ae5ef929 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2019,7 +2019,6 @@ hns3_dev_configure(struct rte_eth_dev *dev)
 	if ((uint32_t)mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) {
 		conf->rxmode.offloads |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
 		rss_conf = conf->rx_adv_conf.rss_conf;
-		hw->rss_dis_flag = false;
 		ret = hns3_dev_rss_hash_update(dev, &rss_conf);
 		if (ret)
 			goto cfg_err;
@@ -2825,7 +2824,6 @@ hns3_get_board_configuration(struct hns3_hw *hw)
 
 	hw->mac.media_type = cfg.media_type;
 	hw->rss_size_max = cfg.rss_size_max;
-	hw->rss_dis_flag = false;
 	memcpy(hw->mac.mac_addr, cfg.mac_addr, RTE_ETHER_ADDR_LEN);
 	hw->mac.phy_addr = cfg.phy_addr;
 	hw->dcb_info.num_pg = 1;
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 9a0fa09b57..a00f841543 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -520,7 +520,6 @@ struct hns3_hw {
 
 	/* The configuration info of RSS */
 	struct hns3_rss_conf rss_info;
-	bool rss_dis_flag; /* disable rss flag. true: disable, false: enable */
 	uint16_t rss_ind_tbl_size;
 	uint16_t rss_key_size;
 
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 9091706fe5..e06a1a41dd 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -495,7 +495,6 @@ hns3vf_dev_configure(struct rte_eth_dev *dev)
 	/* When RSS is not configured, redirect the packet queue 0 */
 	if ((uint32_t)mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) {
 		conf->rxmode.offloads |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
-		hw->rss_dis_flag = false;
 		rss_conf = conf->rx_adv_conf.rss_conf;
 		ret = hns3_dev_rss_hash_update(dev, &rss_conf);
 		if (ret)
@@ -997,7 +996,6 @@ hns3vf_get_configuration(struct hns3_hw *hw)
 	int ret;
 
 	hw->mac.media_type = HNS3_MEDIA_TYPE_NONE;
-	hw->rss_dis_flag = false;
 
 	/* Get device capability */
 	ret = hns3vf_get_capability(hw);
diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index aba07aaa6f..c5a37ee5eb 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -1453,7 +1453,6 @@ hns3_disable_rss(struct hns3_hw *hw)
 
 	/* Disable RSS */
 	hw->rss_info.conf.types = 0;
-	hw->rss_dis_flag = true;
 
 	return 0;
 }
diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 1493b10f96..eb3c7cceec 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -408,9 +408,6 @@ hns3_dev_rss_hash_update(struct rte_eth_dev *dev,
 	uint8_t *key = rss_conf->rss_key;
 	int ret;
 
-	if (hw->rss_dis_flag)
-		return -EINVAL;
-
 	rte_spinlock_lock(&hw->lock);
 	ret = hns3_set_rss_tuple_by_rss_hf(hw, tuple, rss_hf);
 	if (ret)
-- 
2.33.0

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Ferruh Yigit]

On 3/9/2022 9:55 AM, Ori Kam wrote:
> Hi Lihusiong,
> 
>> -----Original Message-----
>> From: lihuisong (C) <lihuisong@huawei.com>
>> Sent: Wednesday, March 9, 2022 10:03 AM
>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior
>>
>>
>> 在 2022/3/3 10:47, lihuisong (C) 写道:
>>> Hi, Ori,
>>>
>>> 在 2022/3/2 22:07, Ori Kam 写道:
>>>> Hi Lihuisong,
>>>>
>>>>> -----Original Message-----
>>>>> From: lihuisong (C) <lihuisong@huawei.com>
>>>>> Sent: Wednesday, March 2, 2022 4:10 AM
>>>>> Subject: Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS
>>>>> behavior
>>>>>
>>>>>
>>>>> 在 2022/3/1 0:42, Ferruh Yigit 写道:
>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
>>>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>>>
>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
>>>>>>> enabled
>>>>>>> through the ethdev ops and rte_flow API. This behavior is contrary to
>>>>>>> each
>>>>>>> other.
>>>>>>>
>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>>>>>> Cc: stable@dpdk.org
>>>>>>>
>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>>
>>>>>> Hi Huisong, Connor,
>>>>>>
>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
>>>>>>
>>>>>>
>>>>>> To enable RSS, multi queue mode should be set to
>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
>>>>>>
>>>>>> But I wonder if it is required to configure RSS via flow API,
>>>>> I do not know the original purpose of adding the RSS configuration in
>>>>> flow API.
>>>>>
>>>> The purpose is simple, this allow to create RSS per rule and not a
>>>> global one.
>>>> For example create RSS that sends TCP to some queues while othe RSS
>>>> will send
>>>> UDP traffic to different queues.
>>> I'm a little confused now. The "per rule" also seems to be a global
>>> configuration.
>>> Example:
>>>   - start PMD with 0,1,2,3
>>>   - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
>>> can be received for other types of packets.
>>> Because this rule is implemented by modifying the entry of the
>>> redirection table which is global for this device.
>> Hi, Ori and Stephen.
>> Can you help me clear up the confusion above? If some NICs behave like
>> this, what should we do about it?
> 
> I'm not sure I understand the issue, maybe it is releated to some HW/PMD limitation.
> In your example non TCP traffic will be routed to one of the 4 queues (0,1,2,3),
> While TCP traffic will only be routed to queues 2,3.
> 
> Now I can add new rule that matches on UDP packet and RSS to queue 0 and 3 in this case:
> TCP packets will be routed to queues 0,3.
> UDP packets will be routed to queues 2,3.
> All the rest of the traffic will be routed to queues 0,1,2,3
> 

Hi Ori,

How RETA is managed for per flow RSS configuration? And is there a limit 
on how many per flow RSS rule can be created?

> And just to be clear if now I add a rule to match all packets in higher priority,
> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
> 
> At least this is what is expected, from API point of view.
> 
> Best,
> Ori
> 
>>>>
>>>>> However, as far as I know, the hash algorithm can be configured via
>>>>> this
>>>>> API,
>>>>>
>>>>> but not via ethdev ops API.
>>>>>
>>>>>> and if other PMDs check this configuration for flow API?
>>>>> Some PMDs already have similar check in RSS releated ops or rte_flow
>>>>> API.
>>>>>
>>>>> For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
>>>>>
>>>>>> .
>>>>   From my view point those are two different settings.
>>>> The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
>>>> rte_flow is per rule.
>>>>
>>>> I think, that if a PMD needs this flag, in order to enable it also
>>>> for rte_flow then
>>>> it should be documented in the release note of the PMD.
>>>> It is a valid use case that the default traffic will not have RSS and
>>>> only rules created by
>>>> rte_flow will have the RSS, for matching traffc.
>>>>
>>>> Best,
>>>> Ori

Re: [PATCH v2 1/2] ethdev: fix enabling RSS behavior inconsistent

[Ferruh Yigit]

On 4/6/2022 7:57 AM, Min Hu (Connor) wrote:
> From: Huisong Li<lihuisong@huawei.com>
> 
> The RTE_ETH_MQ_RX_RSS_FLAG flag is a switch to enable RSS. If the flag is
> not set in dev_configure, RSS will be not configured and enabled. However,
> RSS hash and reta can still be configured by ethdev ops to enable RSS if
> the flag isn't set. The behavior is inconsistent.
> 
> Fixes: 99a2dd955fba ("lib: remove librte_ prefix from directory names")
> Cc:stable@dpdk.org
> 
> Signed-off-by: Huisong Li<lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor)<humin29@huawei.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@xilinx.com>

Re: [PATCH v2 0/2] fix RSS bugs

[Ferruh Yigit]

On 4/6/2022 7:56 AM, Min Hu (Connor) wrote:
> This patch contains following patches:
> ethdev: fix enabling RSS behavior inconsistent
> net/hns3: remove unnecessary RSS switch
> 
> Huisong Li (2):
>    ethdev: fix enabling RSS behavior inconsistent
>    net/hns3: remove unnecessary RSS switch
> 

Series applied to dpdk-next-net/main, thanks.