[PATCH 0/5] doc/features: fix some features and add new features

[PATCH 0/5] doc/features: fix some features and add new features

[Huisong Li]

This series add the configuration interface for some features and add
serval new features had beed supported.

Huisong Li (5):
  doc/features: add RSS hash algorithm feature
  doc/features: add link up/down feature
  doc/features: add config interface for speed capabilities
  doc/features: add Traffic Manager features
  doc/features: add dump device private information feature

 doc/guides/nics/features.rst | 56 ++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 5 deletions(-)

-- 
2.33.0

[PATCH 1/5] doc/features: add RSS hash algorithm feature

[Huisong Li]

Add hash algorithm feature introduced by 23.11 and fix some RSS features
description.

Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 1a1dc16c1e..dfa6f087c8 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -277,10 +277,12 @@ RSS hash
 Supports RSS hashing on RX.
 
 * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
-* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
+* **[uses]     user config**: ``rss_conf.rss_hf``.
 * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
 * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
 * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
+* **[related]  API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update``
+  ``rte_eth_dev_rss_hash_conf_get()``.
 
 
 .. _nic_features_inner_rss:
@@ -288,7 +290,7 @@ Supports RSS hashing on RX.
 Inner RSS
 ---------
 
-Supports RX RSS hashing on Inner headers.
+Supports RX RSS hashing on Inner headers by rte_flow API.
 
 * **[uses]    rte_flow_action_rss**: ``level``.
 * **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
@@ -303,9 +305,24 @@ RSS key update
 Supports configuration of Receive Side Scaling (RSS) hash computation. Updating
 Receive Side Scaling (RSS) hash key.
 
-* **[implements] eth_dev_ops**: ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.rss_key``, ``rss_conf.rss_key_len``
 * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
-* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
+* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
+  ``rte_eth_dev_rss_hash_conf_get()``.
+
+.. _nic_features_rss_hash_algo_update:
+
+RSS hash algorithm update
+--------------
+
+Supports configuration of Receive Side Scaling (RSS) hash algorithm. Updating
+RSS hash algorithm.
+
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.algorithm``
+* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
+* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
   ``rte_eth_dev_rss_hash_conf_get()``.
 
 
-- 
2.33.0

[PATCH 2/5] doc/features: add link up/down feature

[Huisong Li]

Add link up/down feature for features.rst.

Fixes: 915e67837586 ("ethdev: API for link up and down")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index dfa6f087c8..1c0cf2cea7 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -41,10 +41,13 @@ Link status
 
 Supports getting the link speed, duplex mode and link state (up/down).
 
-* **[implements] eth_dev_ops**: ``link_update``.
+* **[implements] eth_dev_ops**: ``link_update``, ``dev_set_link_up``, ``dev_set_link_down``.
 * **[implements] rte_eth_dev_data**: ``dev_link``.
 * **[related]    API**: ``rte_eth_link_get()``, ``rte_eth_link_get_nowait()``.
 
+Set link up/down an Ethernet device.
+
+* **[related]    API**: ``rte_eth_dev_set_link_up()``, ``rte_eth_dev_set_link_down()``.
 
 .. _nic_features_link_status_event:
 
-- 
2.33.0

[PATCH 3/5] doc/features: add config interface for speed capabilities

[Huisong Li]

Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 1c0cf2cea7..a5026a0aa8 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -31,6 +31,7 @@ Speed capabilities
 Supports getting the speed capabilities that the current device is capable of.
 
 * **[provides] rte_eth_dev_info**: ``speed_capa:RTE_ETH_LINK_SPEED_*``.
+* **[uses]     user config**: ``dev_conf.link_speeds``.
 * **[related]  API**: ``rte_eth_dev_info_get()``.
 
 
-- 
2.33.0

[PATCH 4/5] doc/features: add Traffic Manager features

[Huisong Li]

Add Traffic Manager features.

Fixes: 5d109deffa87 ("ethdev: add traffic management API")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index a5026a0aa8..69fbf737b8 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -761,6 +761,19 @@ Supports congestion management.
   ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
 
 
+.. _nic_features_traffic_manager:
+
+Traffic manager
+---------------------
+
+Supports Traffic manager.
+
+* **[implements] rte_tm_ops**: ``capabilities_get``, ``shaper_profile_add``,
+  ``hierarchy_commit`` and so on.
+* **[related]    API**: ``rte_tm_capabilities_get()``, ``rte_tm_shaper_profile_add()``,
+  ``rte_tm_hierarchy_commit()`` and so on.
+
+
 .. _nic_features_fw_version:
 
 FW version
-- 
2.33.0

[PATCH 5/5] doc/features: add dump device private information feature

[Huisong Li]

Add dump device private information feature.

Fixes: edcf22c6d389 ("ethdev: introduce dump API")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 69fbf737b8..bae1a1faab 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -820,6 +820,18 @@ registers and register size).
 * **[related]    API**: ``rte_eth_dev_get_reg_info()``.
 
 
+.. _nic_features_device_private_info_dump:
+
+Device private information dump
+--------------
+
+Supports retrieving device private information to a file. Provided data and
+the order depends on PMD.
+
+* **[mplements] eth_dev_ops**: ``eth_dev_priv_dump``.
+* **[related]    API**: ``rte_eth_dev_priv_dump()``.
+
+
 .. _nic_features_led:
 
 LED
-- 
2.33.0

Re: [PATCH 5/5] doc/features: add dump device private information feature

[Thomas Monjalon]

23/11/2023 14:59, Huisong Li:
> +.. _nic_features_device_private_info_dump:
> +
> +Device private information dump
> +--------------

Why underlining is too short?

> +
> +Supports retrieving device private information to a file. Provided data and
> +the order depends on PMD.
> +
> +* **[mplements] eth_dev_ops**: ``eth_dev_priv_dump``.

looks like a typo here

> +* **[related]    API**: ``rte_eth_dev_priv_dump()``.

Re: [PATCH 5/5] doc/features: add dump device private information feature

[lihuisong (C)]

在 2023/11/23 22:18, Thomas Monjalon 写道:
> 23/11/2023 14:59, Huisong Li:
>> +.. _nic_features_device_private_info_dump:
>> +
>> +Device private information dump
>> +--------------
> Why underlining is too short?
You are right.
Then I got this warning email which is the same as you said.😁
will fix it in v2.
>
>> +
>> +Supports retrieving device private information to a file. Provided data and
>> +the order depends on PMD.
>> +
>> +* **[mplements] eth_dev_ops**: ``eth_dev_priv_dump``.
> looks like a typo here
Ack.
>
>> +* **[related]    API**: ``rte_eth_dev_priv_dump()``.
>
>
> .

[PATCH 0/6] doc/features: fix some features and add new features

[Huisong Li]

This series fix the configuration interface for RSS feature and add
serval features had beed supported.

---
 -v2:
   - fix the short underline warning.
   - add loopback mode feature.

Huisong Li (6):
  doc/features: add RSS hash algorithm feature
  doc/features: add link up/down feature
  doc/features: add features for link speeds
  doc/features: add Traffic Manager features
  doc/features: add dump device private information feature
  doc/features: add feature for loopback mode

 doc/guides/nics/features.rst | 78 ++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 4 deletions(-)

-- 
2.33.0

[PATCH 1/6] doc/features: add RSS hash algorithm feature

[Huisong Li]

Add hash algorithm feature introduced by 23.11 and fix some RSS features
description.

Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 1a1dc16c1e..0d38c5c525 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -277,10 +277,12 @@ RSS hash
 Supports RSS hashing on RX.
 
 * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
-* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
+* **[uses]     user config**: ``rss_conf.rss_hf``.
 * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
 * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
 * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
+* **[related]  API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update``
+  ``rte_eth_dev_rss_hash_conf_get()``.
 
 
 .. _nic_features_inner_rss:
@@ -288,7 +290,7 @@ Supports RSS hashing on RX.
 Inner RSS
 ---------
 
-Supports RX RSS hashing on Inner headers.
+Supports RX RSS hashing on Inner headers by rte_flow API.
 
 * **[uses]    rte_flow_action_rss**: ``level``.
 * **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
@@ -303,9 +305,25 @@ RSS key update
 Supports configuration of Receive Side Scaling (RSS) hash computation. Updating
 Receive Side Scaling (RSS) hash key.
 
-* **[implements] eth_dev_ops**: ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.rss_key``, ``rss_conf.rss_key_len``
 * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
-* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
+* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
+  ``rte_eth_dev_rss_hash_conf_get()``.
+
+
+.. _nic_features_rss_hash_algo_update:
+
+RSS hash algorithm update
+-------------------------
+
+Supports configuration of Receive Side Scaling (RSS) hash algorithm. Updating
+RSS hash algorithm.
+
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.algorithm``
+* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
+* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
   ``rte_eth_dev_rss_hash_conf_get()``.
 
 
-- 
2.33.0

[PATCH 2/6] doc/features: add link up/down feature

[Huisong Li]

Add link up/down feature for features.rst.

Fixes: 915e67837586 ("ethdev: API for link up and down")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 0d38c5c525..f14962a6c3 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -45,6 +45,10 @@ Supports getting the link speed, duplex mode and link state (up/down).
 * **[implements] rte_eth_dev_data**: ``dev_link``.
 * **[related]    API**: ``rte_eth_link_get()``, ``rte_eth_link_get_nowait()``.
 
+Set link up/down an Ethernet device.
+
+* **[implements] eth_dev_ops**: ``dev_set_link_up``, ``dev_set_link_down``.
+* **[related]    API**: ``rte_eth_dev_set_link_up()``, ``rte_eth_dev_set_link_down()``.
 
 .. _nic_features_link_status_event:
 
-- 
2.33.0

[PATCH 3/6] doc/features: add features for link speeds

[Huisong Li]

Add features for link speeds.

Fixes: 82113036e4e5 ("ethdev: redesign link speed config")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index f14962a6c3..d13e43ae81 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -34,6 +34,17 @@ Supports getting the speed capabilities that the current device is capable of.
 * **[related]  API**: ``rte_eth_dev_info_get()``.
 
 
+.. _nic_features_link_speeds_config:
+
+Link speed configuration
+------------------------
+
+Supports configurating fixed speed and link autonegotiation.
+
+* **[uses]     user config**: ``dev_conf.link_speeds:RTE_ETH_LINK_SPEED_*``.
+* **[related]  API**: ``rte_eth_dev_configure()``.
+
+
 .. _nic_features_link_status:
 
 Link status
-- 
2.33.0

[PATCH 4/6] doc/features: add Traffic Manager features

[Huisong Li]

Add Traffic Manager features.

Fixes: 5d109deffa87 ("ethdev: add traffic management API")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index d13e43ae81..ef061759c7 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -773,6 +773,19 @@ Supports congestion management.
   ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
 
 
+.. _nic_features_traffic_manager:
+
+Traffic manager
+---------------
+
+Supports Traffic manager.
+
+* **[implements] rte_tm_ops**: ``capabilities_get``, ``shaper_profile_add``,
+  ``hierarchy_commit`` and so on.
+* **[related]    API**: ``rte_tm_capabilities_get()``, ``rte_tm_shaper_profile_add()``,
+  ``rte_tm_hierarchy_commit()`` and so on.
+
+
 .. _nic_features_fw_version:
 
 FW version
-- 
2.33.0

[PATCH 5/6] doc/features: add dump device private information feature

[Huisong Li]

Add dump device private information feature.

Fixes: edcf22c6d389 ("ethdev: introduce dump API")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index ef061759c7..c5c4dbf745 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -832,6 +832,18 @@ registers and register size).
 * **[related]    API**: ``rte_eth_dev_get_reg_info()``.
 
 
+.. _nic_features_device_private_info_dump:
+
+Device private information dump
+-------------------------------
+
+Supports retrieving device private information to a file. Provided data and
+the order depends on PMD.
+
+* **[implements] eth_dev_ops**: ``eth_dev_priv_dump``.
+* **[related]    API**: ``rte_eth_dev_priv_dump()``.
+
+
 .. _nic_features_led:
 
 LED
-- 
2.33.0

[PATCH 6/6] doc/features: add feature for loopback mode

[Huisong Li]

Add feature for loopback mode.

Fixes: db0359256170 ("ixgbe: add Tx->Rx loopback mode for 82599")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 doc/guides/nics/features.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index c5c4dbf745..caf1258554 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -45,6 +45,18 @@ Supports configurating fixed speed and link autonegotiation.
 * **[related]  API**: ``rte_eth_dev_configure()``.
 
 
+.. _nic_features_loopback:
+
+Loopback configuration
+----------------------
+
+Supports configurating loopback mode. The default value 0 is to disable
+loopback mode and other value is defined by given Ethernet controller.
+
+* **[uses]     user config**: ``dev_conf.lpbk_mode``.
+* **[related]  API**: ``rte_eth_dev_configure()``.
+
+
 .. _nic_features_link_status:
 
 Link status
-- 
2.33.0

Re: [PATCH 0/6] doc/features: fix some features and add new features

[fengchengwen]

LGTM
Series-acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/11/24 11:12, Huisong Li wrote:
> This series fix the configuration interface for RSS feature and add
> serval features had beed supported.
> 
> ---
>  -v2:
>    - fix the short underline warning.
>    - add loopback mode feature.
> 
> Huisong Li (6):
>   doc/features: add RSS hash algorithm feature
>   doc/features: add link up/down feature
>   doc/features: add features for link speeds
>   doc/features: add Traffic Manager features
>   doc/features: add dump device private information feature
>   doc/features: add feature for loopback mode
> 
>  doc/guides/nics/features.rst | 78 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 74 insertions(+), 4 deletions(-)
> 

[PATCH v3 0/6] doc/features: fix some features and add new features

[Huisong Li]

The support for setting RSS hash algorithm has been introduced to 23.11,
but doesn't add feature for it. So this series add this feature and perfect
RSS other features by the way.

In addition, serval features, like "set link up/down", "TM", "dump device
private information" and "loopback mode", had beed supported, but they are
absent from the freatures list.

---
 -v3:
   - fix description of the patchset cover
   - remove '/features' in 'doc/features' tag for every patch.
   - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
   - retrigger CI build because of wrong CI warning

 -v2:
   - add loopback mode feature.

Huisong Li (6):
  doc: add RSS hash algorithm feature
  doc: add link up/down feature
  doc: add features for link speeds
  doc: add Traffic Manager feature
  doc: add dump device private information feature
  doc: add feature for loopback mode

 doc/guides/nics/features.rst | 78 ++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 4 deletions(-)

-- 
2.33.0

[PATCH v3 1/6] doc: add RSS hash algorithm feature

[Huisong Li]

Add hash algorithm feature introduced by 23.11 and fix some RSS features
description.

Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 1a1dc16c1e..0d38c5c525 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -277,10 +277,12 @@ RSS hash
 Supports RSS hashing on RX.
 
 * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
-* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
+* **[uses]     user config**: ``rss_conf.rss_hf``.
 * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
 * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
 * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
+* **[related]  API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update``
+  ``rte_eth_dev_rss_hash_conf_get()``.
 
 
 .. _nic_features_inner_rss:
@@ -288,7 +290,7 @@ Supports RSS hashing on RX.
 Inner RSS
 ---------
 
-Supports RX RSS hashing on Inner headers.
+Supports RX RSS hashing on Inner headers by rte_flow API.
 
 * **[uses]    rte_flow_action_rss**: ``level``.
 * **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
@@ -303,9 +305,25 @@ RSS key update
 Supports configuration of Receive Side Scaling (RSS) hash computation. Updating
 Receive Side Scaling (RSS) hash key.
 
-* **[implements] eth_dev_ops**: ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.rss_key``, ``rss_conf.rss_key_len``
 * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
-* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
+* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
+  ``rte_eth_dev_rss_hash_conf_get()``.
+
+
+.. _nic_features_rss_hash_algo_update:
+
+RSS hash algorithm update
+-------------------------
+
+Supports configuration of Receive Side Scaling (RSS) hash algorithm. Updating
+RSS hash algorithm.
+
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.algorithm``
+* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
+* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
   ``rte_eth_dev_rss_hash_conf_get()``.
 
 
-- 
2.33.0

[PATCH v3 2/6] doc: add link up/down feature

[Huisong Li]

Add link up/down feature for features.rst.

Fixes: 915e67837586 ("ethdev: API for link up and down")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 0d38c5c525..f14962a6c3 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -45,6 +45,10 @@ Supports getting the link speed, duplex mode and link state (up/down).
 * **[implements] rte_eth_dev_data**: ``dev_link``.
 * **[related]    API**: ``rte_eth_link_get()``, ``rte_eth_link_get_nowait()``.
 
+Set link up/down an Ethernet device.
+
+* **[implements] eth_dev_ops**: ``dev_set_link_up``, ``dev_set_link_down``.
+* **[related]    API**: ``rte_eth_dev_set_link_up()``, ``rte_eth_dev_set_link_down()``.
 
 .. _nic_features_link_status_event:
 
-- 
2.33.0

[PATCH v3 3/6] doc: add features for link speeds

[Huisong Li]

Add features for link speeds.

Fixes: 82113036e4e5 ("ethdev: redesign link speed config")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index f14962a6c3..d13e43ae81 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -34,6 +34,17 @@ Supports getting the speed capabilities that the current device is capable of.
 * **[related]  API**: ``rte_eth_dev_info_get()``.
 
 
+.. _nic_features_link_speeds_config:
+
+Link speed configuration
+------------------------
+
+Supports configurating fixed speed and link autonegotiation.
+
+* **[uses]     user config**: ``dev_conf.link_speeds:RTE_ETH_LINK_SPEED_*``.
+* **[related]  API**: ``rte_eth_dev_configure()``.
+
+
 .. _nic_features_link_status:
 
 Link status
-- 
2.33.0

[PATCH v3 4/6] doc: add Traffic Manager feature

[Huisong Li]

Add Traffic Manager feature.

Fixes: 5d109deffa87 ("ethdev: add traffic management API")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index d13e43ae81..ef061759c7 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -773,6 +773,19 @@ Supports congestion management.
   ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
 
 
+.. _nic_features_traffic_manager:
+
+Traffic manager
+---------------
+
+Supports Traffic manager.
+
+* **[implements] rte_tm_ops**: ``capabilities_get``, ``shaper_profile_add``,
+  ``hierarchy_commit`` and so on.
+* **[related]    API**: ``rte_tm_capabilities_get()``, ``rte_tm_shaper_profile_add()``,
+  ``rte_tm_hierarchy_commit()`` and so on.
+
+
 .. _nic_features_fw_version:
 
 FW version
-- 
2.33.0

[PATCH v3 5/6] doc: add dump device private information feature

[Huisong Li]

Add dump device private information feature.

Fixes: edcf22c6d389 ("ethdev: introduce dump API")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index ef061759c7..c5c4dbf745 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -832,6 +832,18 @@ registers and register size).
 * **[related]    API**: ``rte_eth_dev_get_reg_info()``.
 
 
+.. _nic_features_device_private_info_dump:
+
+Device private information dump
+-------------------------------
+
+Supports retrieving device private information to a file. Provided data and
+the order depends on PMD.
+
+* **[implements] eth_dev_ops**: ``eth_dev_priv_dump``.
+* **[related]    API**: ``rte_eth_dev_priv_dump()``.
+
+
 .. _nic_features_led:
 
 LED
-- 
2.33.0

[PATCH v3 6/6] doc: add feature for loopback mode

[Huisong Li]

Add feature for loopback mode.

Fixes: db0359256170 ("ixgbe: add Tx->Rx loopback mode for 82599")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index c5c4dbf745..caf1258554 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -45,6 +45,18 @@ Supports configurating fixed speed and link autonegotiation.
 * **[related]  API**: ``rte_eth_dev_configure()``.
 
 
+.. _nic_features_loopback:
+
+Loopback configuration
+----------------------
+
+Supports configurating loopback mode. The default value 0 is to disable
+loopback mode and other value is defined by given Ethernet controller.
+
+* **[uses]     user config**: ``dev_conf.lpbk_mode``.
+* **[related]  API**: ``rte_eth_dev_configure()``.
+
+
 .. _nic_features_link_status:
 
 Link status
-- 
2.33.0

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[Ferruh Yigit]

On 11/25/2023 1:47 AM, Huisong Li wrote:
> Add hash algorithm feature introduced by 23.11 and fix some RSS features
> description.
> 
> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index 1a1dc16c1e..0d38c5c525 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -277,10 +277,12 @@ RSS hash
>  Supports RSS hashing on RX.
>  
>  * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>

Feature title is "RSS hash", it can be two things,
1. "Receive Side Scaling" support
2. Provide RSS hash to application

When this document first prepared RSS hash value was always provided to
the application when RSS enabled.
So intention with this feature was "Receive Side Scaling" support, hence
'RTE_ETH_MQ_RX_RSS_FLAG' added.

Later providing RSS has to the application separated as optimization,
'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
support.


As the intention of this feature is "Receive Side Scaling" support, we
shouldn't reduce configuration struct to 'rss_conf.rss_hf'.

Instead perhaps can expand to:
'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'



>  * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>  * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>  * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
> +* **[related]  API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update``
> +  ``rte_eth_dev_rss_hash_conf_get()``.
>  

ack

>  
>  .. _nic_features_inner_rss:
> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>  Inner RSS
>  ---------
>  
> -Supports RX RSS hashing on Inner headers.
> +Supports RX RSS hashing on Inner headers by rte_flow API.
>  

This should be clarified with details below, not sure if it required to
limit description to rte_flow.


And I guess similar confusion exist with the providing hash to user.
Need to check if rte_flow implementation puts hash to mbuf along with
doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
and update below items accordingly.


>  * **[uses]    rte_flow_action_rss**: ``level``.
>  * **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
> @@ -303,9 +305,25 @@ RSS key update
>  Supports configuration of Receive Side Scaling (RSS) hash computation. Updating
>  Receive Side Scaling (RSS) hash key.
>  
> -* **[implements] eth_dev_ops**: ``rss_hash_update``, ``rss_hash_conf_get``.
> +* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
> +* **[uses]     user config**: ``rss_conf.rss_key``, ``rss_conf.rss_key_len``
>  * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
> +* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
> +  ``rte_eth_dev_rss_hash_conf_get()``.
> +

ack

There is an inconsistency in the documentation but I think it is good to
use '()' when documenting API, like: 'rte_eth_dev_configure()'


> +
> +.. _nic_features_rss_hash_algo_update:
> +
> +RSS hash algorithm update
> +-------------------------
> +
> +Supports configuration of Receive Side Scaling (RSS) hash algorithm. Updating
> +RSS hash algorithm.
> +
> +* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
> +* **[uses]     user config**: ``rss_conf.algorithm``
> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
> +* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
>    ``rte_eth_dev_rss_hash_conf_get()``.
>  
>  


This document describes features listed in the 'default.ini', so we
shouldn't have above.

And I don't think RSS hash algorithm update is a big enough feature to
list in the feature list, perhaps it can be embedded in the RSS support
block, what do you think?

Re: [PATCH v3 2/6] doc: add link up/down feature

[Ferruh Yigit]

On 11/25/2023 1:47 AM, Huisong Li wrote:
> Add link up/down feature for features.rst.
> 
> Fixes: 915e67837586 ("ethdev: API for link up and down")
> Cc: stable@dpdk.org
> 

Documentation is added after feature, so above fixes line is not correct.

I am not sure if this is fix, I think more like it is extending the
document.


With fixes line fix,
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>


> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  doc/guides/nics/features.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index 0d38c5c525..f14962a6c3 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -45,6 +45,10 @@ Supports getting the link speed, duplex mode and link state (up/down).
>  * **[implements] rte_eth_dev_data**: ``dev_link``.
>  * **[related]    API**: ``rte_eth_link_get()``, ``rte_eth_link_get_nowait()``.
>  
> +Set link up/down an Ethernet device.
> +
> +* **[implements] eth_dev_ops**: ``dev_set_link_up``, ``dev_set_link_down``.
> +* **[related]    API**: ``rte_eth_dev_set_link_up()``, ``rte_eth_dev_set_link_down()``.
>  
>  .. _nic_features_link_status_event:
>  

Re: [PATCH v3 0/6] doc/features: fix some features and add new features

[Ferruh Yigit]

On 11/25/2023 1:47 AM, Huisong Li wrote:
> The support for setting RSS hash algorithm has been introduced to 23.11,
> but doesn't add feature for it. So this series add this feature and perfect
> RSS other features by the way.
> 
> In addition, serval features, like "set link up/down", "TM", "dump device
> private information" and "loopback mode", had beed supported, but they are
> absent from the freatures list.
> 
> ---
>  -v3:
>    - fix description of the patchset cover
>    - remove '/features' in 'doc/features' tag for every patch.
>    - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>    - retrigger CI build because of wrong CI warning
> 
>  -v2:
>    - add loopback mode feature.
> 
> Huisong Li (6):
>   doc: add RSS hash algorithm feature
>   doc: add link up/down feature
>   doc: add features for link speeds
>   doc: add Traffic Manager feature
>   doc: add dump device private information feature
>   doc: add feature for loopback mode
> 
>  

Hi Huisong,

Some of the patches above adds new features to the 'features.rst', but
features.rst document describes features that drivers mark in feature
list in .ini file, all features are listed in the 'default.ini'.

When a new feature is added into 'default.ini', it can be described in
'features.rst' and later all driver .ini files should be updated to
reflect if they support this feature or not.


And I think no need to document all features, but ones can be important
enough or differentiating features for users to know, this line is not
easy to draw, so perhaps we can use community consensus to select.

In this patch following features are added:
- Link speed configuration
- Traffic manager
- Device private information dump
- Loopback configuration


I think 'Traffic manager' can be a new feature, but I am not sure about
rest. It would be nice to get more comments.

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[lihuisong (C)]

在 2023/11/27 20:19, Ferruh Yigit 写道:
> On 11/25/2023 1:47 AM, Huisong Li wrote:
>> Add hash algorithm feature introduced by 23.11 and fix some RSS features
>> description.
>>
>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>   doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>   1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
>> index 1a1dc16c1e..0d38c5c525 100644
>> --- a/doc/guides/nics/features.rst
>> +++ b/doc/guides/nics/features.rst
>> @@ -277,10 +277,12 @@ RSS hash
>>   Supports RSS hashing on RX.
>>   
>>   * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>
> Feature title is "RSS hash", it can be two things,
> 1. "Receive Side Scaling" support
> 2. Provide RSS hash to application
>
> When this document first prepared RSS hash value was always provided to
> the application when RSS enabled.
> So intention with this feature was "Receive Side Scaling" support, hence
> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>
> Later providing RSS has to the application separated as optimization,
> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
> support.
What should I do for above two comments？
To tell application how to use it?
>
> As the intention of this feature is "Receive Side Scaling" support, we
> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>
> Instead perhaps can expand to:
> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'

  I just pick their common part.😁

ok, will fix it.

>
>
>>   * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>   * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>   * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
>> +* **[related]  API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update``
>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>   
> ack
>
>>   
>>   .. _nic_features_inner_rss:
>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>   Inner RSS
>>   ---------
>>   
>> -Supports RX RSS hashing on Inner headers.
>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>   
> This should be clarified with details below, not sure if it required to
> limit description to rte_flow.
But this block like rte_flow_action_rss is from rte_flow.
And ethdev ops doesn't support inner RSS.
So I think it is ok.
>
>
> And I guess similar confusion exist with the providing hash to user.
> Need to check if rte_flow implementation puts hash to mbuf along with
> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
> and update below items accordingly.
Do we need to tell user how to use it here?
I feel this document is a little simple and main to list interface for user.
In addition, it is better that the more detail about RSS should be 
presented  in rte_flow features.
>
>
>>   * **[uses]    rte_flow_action_rss**: ``level``.
>>   * **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>> @@ -303,9 +305,25 @@ RSS key update
>>   Supports configuration of Receive Side Scaling (RSS) hash computation. Updating
>>   Receive Side Scaling (RSS) hash key.
>>   
>> -* **[implements] eth_dev_ops**: ``rss_hash_update``, ``rss_hash_conf_get``.
>> +* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
>> +* **[uses]     user config**: ``rss_conf.rss_key``, ``rss_conf.rss_key_len``
>>   * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>> +* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>> +
> ack
>
> There is an inconsistency in the documentation but I think it is good to
> use '()' when documenting API, like: 'rte_eth_dev_configure()'
+1 will fix it.
>
>
>> +
>> +.. _nic_features_rss_hash_algo_update:
>> +
>> +RSS hash algorithm update
>> +-------------------------
>> +
>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm. Updating
>> +RSS hash algorithm.
>> +
>> +* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
>> +* **[uses]     user config**: ``rss_conf.algorithm``
>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>> +* **[related]    API**: ``rte_eth_dev_configure``, ``rte_eth_dev_rss_hash_update()``,
>>     ``rte_eth_dev_rss_hash_conf_get()``.
>>   
>>   
>
> This document describes features listed in the 'default.ini', so we
> shouldn't have above.
>
> And I don't think RSS hash algorithm update is a big enough feature to
> list in the feature list, perhaps it can be embedded in the RSS support
> block, what do you think?
Yes it is not a bit feature.
so put it to RSS hash, right?


>
>
> .

Re: [PATCH v3 2/6] doc: add link up/down feature

[lihuisong (C)]

在 2023/11/27 20:24, Ferruh Yigit 写道:
> On 11/25/2023 1:47 AM, Huisong Li wrote:
>> Add link up/down feature for features.rst.
>>
>> Fixes: 915e67837586 ("ethdev: API for link up and down")
>> Cc: stable@dpdk.org
>>
> Documentation is added after feature, so above fixes line is not correct.
>
> I am not sure if this is fix, I think more like it is extending the
> document.
>
>
> With fixes line fix,
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
>
Ok  just to add Cc stable.
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>   doc/guides/nics/features.rst | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
>> index 0d38c5c525..f14962a6c3 100644
>> --- a/doc/guides/nics/features.rst
>> +++ b/doc/guides/nics/features.rst
>> @@ -45,6 +45,10 @@ Supports getting the link speed, duplex mode and link state (up/down).
>>   * **[implements] rte_eth_dev_data**: ``dev_link``.
>>   * **[related]    API**: ``rte_eth_link_get()``, ``rte_eth_link_get_nowait()``.
>>   
>> +Set link up/down an Ethernet device.
>> +
>> +* **[implements] eth_dev_ops**: ``dev_set_link_up``, ``dev_set_link_down``.
>> +* **[related]    API**: ``rte_eth_dev_set_link_up()``, ``rte_eth_dev_set_link_down()``.
>>   
>>   .. _nic_features_link_status_event:
>>   
>
> .

Re: [PATCH v3 0/6] doc/features: fix some features and add new features

[lihuisong (C)]

在 2023/11/27 20:35, Ferruh Yigit 写道:
> On 11/25/2023 1:47 AM, Huisong Li wrote:
>> The support for setting RSS hash algorithm has been introduced to 23.11,
>> but doesn't add feature for it. So this series add this feature and perfect
>> RSS other features by the way.
>>
>> In addition, serval features, like "set link up/down", "TM", "dump device
>> private information" and "loopback mode", had beed supported, but they are
>> absent from the freatures list.
>>
>> ---
>>   -v3:
>>     - fix description of the patchset cover
>>     - remove '/features' in 'doc/features' tag for every patch.
>>     - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>     - retrigger CI build because of wrong CI warning
>>
>>   -v2:
>>     - add loopback mode feature.
>>
>> Huisong Li (6):
>>    doc: add RSS hash algorithm feature
>>    doc: add link up/down feature
>>    doc: add features for link speeds
>>    doc: add Traffic Manager feature
>>    doc: add dump device private information feature
>>    doc: add feature for loopback mode
>>
>>   
> Hi Huisong,
>
> Some of the patches above adds new features to the 'features.rst', but
> features.rst document describes features that drivers mark in feature
> list in .ini file, all features are listed in the 'default.ini'.
>
> When a new feature is added into 'default.ini', it can be described in
> 'features.rst' and later all driver .ini files should be updated to
> reflect if they support this feature or not.
correct.
>
> And I think no need to document all features, but ones can be important
> enough or differentiating features for users to know, this line is not
agree with you.
> easy to draw, so perhaps we can use community consensus to select.
>
> In this patch following features are added:
> - Link speed configuration
> - Traffic manager
> - Device private information dump
> - Loopback configuration
>
>
> I think 'Traffic manager' can be a new feature, but I am not sure about
> rest. It would be nice to get more comments.
Link speed is also important for NIC. Fixed speed and Link autoneg is 
from this.
I think it can be a new feature. what do you think?
How about add others to the following block?
"Other dev ops not represented by a Feature"
> .

[PATCH v4 0/5] doc/features: fix some features and add new features

[Huisong Li]

The support for setting RSS hash algorithm has been introduced to 23.11,
but doesn't add feature for it. So this series add this feature to RSS hash
and perfect RSS other features by the way.

In addition, serval features or interface, like "set link up/down", "TM",
"link speeds" and "dump device private information" had beed supported,
but they are absent from the freatures list.

---
 -v4:
   - fix RSS releated feaures description based on Ferruh
   - add new features to default.ini
   - remove loopback patch.
   - move "dump device private information" to "dev ops" block.

 -v3:
   - fix description of the patchset cover
   - remove '/features' in 'doc/features' tag for every patch.
   - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
   - retrigger CI build because of wrong CI warning

 -v2:
   - add loopback mode feature.

Huisong Li (5):
  doc: fix the description of RSS related feature
  doc: add link up/down feature
  doc: add features for link speeds
  doc: add Traffic Manager feature
  doc: add dump device private information ops

 doc/guides/nics/features.rst         | 47 +++++++++++++++++++++++++---
 doc/guides/nics/features/default.ini |  2 ++
 2 files changed, 45 insertions(+), 4 deletions(-)

-- 
2.33.0

[PATCH v4 1/5] doc: fix the description of RSS related feature

[Huisong Li]

This patch fixes the description of RSS feature.
And the settinf ot hash algorithm is introduced by 23.11, so add it.

Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 1a1dc16c1e..307dfc162e 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -277,10 +277,19 @@ RSS hash
 Supports RSS hashing on RX.
 
 * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
-* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
+* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf.rss_hf``. ``rss_conf.rss_hf``
 * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
 * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
 * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
+* **[related]  API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update``
+  ``rte_eth_dev_rss_hash_conf_get()``.
+
+Support RSS hash algorithm on Rx.
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.algorithm``
+* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
+* **[related]    API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update()``,
+  ``rte_eth_dev_rss_hash_conf_get()``.
 
 
 .. _nic_features_inner_rss:
@@ -288,7 +297,7 @@ Supports RSS hashing on RX.
 Inner RSS
 ---------
 
-Supports RX RSS hashing on Inner headers.
+Supports RX RSS hashing on Inner headers by rte_flow API.
 
 * **[uses]    rte_flow_action_rss**: ``level``.
 * **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
@@ -303,9 +312,10 @@ RSS key update
 Supports configuration of Receive Side Scaling (RSS) hash computation. Updating
 Receive Side Scaling (RSS) hash key.
 
-* **[implements] eth_dev_ops**: ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.rss_key``, ``rss_conf.rss_key_len``
 * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
-* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
+* **[related]    API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update()``,
   ``rte_eth_dev_rss_hash_conf_get()``.
 
 
-- 
2.33.0

[PATCH v4 2/5] doc: add link up/down feature

[Huisong Li]

Add link up/down feature for features.rst.

Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 doc/guides/nics/features.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 307dfc162e..1372ede021 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -45,6 +45,10 @@ Supports getting the link speed, duplex mode and link state (up/down).
 * **[implements] rte_eth_dev_data**: ``dev_link``.
 * **[related]    API**: ``rte_eth_link_get()``, ``rte_eth_link_get_nowait()``.
 
+Set link up/down an Ethernet device.
+
+* **[implements] eth_dev_ops**: ``dev_set_link_up``, ``dev_set_link_down``.
+* **[related]    API**: ``rte_eth_dev_set_link_up()``, ``rte_eth_dev_set_link_down()``.
 
 .. _nic_features_link_status_event:
 
-- 
2.33.0

[PATCH v4 3/5] doc: add features for link speeds

[Huisong Li]

Add features for link speeds.

Fixes: 82113036e4e5 ("ethdev: redesign link speed config")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst         | 11 +++++++++++
 doc/guides/nics/features/default.ini |  1 +
 2 files changed, 12 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 1372ede021..3a4625f079 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -34,6 +34,17 @@ Supports getting the speed capabilities that the current device is capable of.
 * **[related]  API**: ``rte_eth_dev_info_get()``.
 
 
+.. _nic_features_link_speeds_config:
+
+Link speed configuration
+------------------------
+
+Supports configurating fixed speed and link autonegotiation.
+
+* **[uses]     user config**: ``dev_conf.link_speeds:RTE_ETH_LINK_SPEED_*``.
+* **[related]  API**: ``rte_eth_dev_configure()``.
+
+
 .. _nic_features_link_status:
 
 Link status
diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index 806cb033ff..4de7d9765f 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -8,6 +8,7 @@
 ;
 [Features]
 Speed capabilities   =
+Link speed configuration =
 Link status          =
 Link status event    =
 Removal event        =
-- 
2.33.0

[PATCH v4 4/5] doc: add Traffic Manager feature

[Huisong Li]

Add Traffic Manager feature.

Fixes: 5d109deffa87 ("ethdev: add traffic management API")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst         | 13 +++++++++++++
 doc/guides/nics/features/default.ini |  1 +
 2 files changed, 14 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 3a4625f079..3d5fd36617 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -765,6 +765,19 @@ Supports congestion management.
   ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
 
 
+.. _nic_features_traffic_manager:
+
+Traffic manager
+---------------
+
+Supports Traffic manager.
+
+* **[implements] rte_tm_ops**: ``capabilities_get``, ``shaper_profile_add``,
+  ``hierarchy_commit`` and so on.
+* **[related]    API**: ``rte_tm_capabilities_get()``, ``rte_tm_shaper_profile_add()``,
+  ``rte_tm_hierarchy_commit()`` and so on.
+
+
 .. _nic_features_fw_version:
 
 FW version
diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index 4de7d9765f..df00f980cd 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -63,6 +63,7 @@ Tx descriptor status =
 Basic stats          =
 Extended stats       =
 Stats per queue      =
+Traffic manager      =
 FW version           =
 EEPROM dump          =
 Module EEPROM dump   =
-- 
2.33.0

[PATCH v4 5/5] doc: add dump device private information ops

[Huisong Li]

Add dump device private information to dev ops.

Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/nics/features.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 3d5fd36617..868cf8f6d9 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -1005,3 +1005,4 @@ Other dev ops not represented by a Feature
 * ``udp_tunnel_port_add``
 * ``udp_tunnel_port_del``
 * ``tx_pkt_prepare``
+* ``eth_dev_priv_dump``
-- 
2.33.0

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[Ferruh Yigit]

On 11/27/2023 1:12 PM, lihuisong (C) wrote:
> 
> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>> Add hash algorithm feature introduced by 23.11 and fix some RSS features
>>> description.
>>>
>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>> ---
>>>   doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>   1 file changed, 22 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
>>> index 1a1dc16c1e..0d38c5c525 100644
>>> --- a/doc/guides/nics/features.rst
>>> +++ b/doc/guides/nics/features.rst
>>> @@ -277,10 +277,12 @@ RSS hash
>>>   Supports RSS hashing on RX.
>>>     * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>
>> Feature title is "RSS hash", it can be two things,
>> 1. "Receive Side Scaling" support
>> 2. Provide RSS hash to application
>>
>> When this document first prepared RSS hash value was always provided to
>> the application when RSS enabled.
>> So intention with this feature was "Receive Side Scaling" support, hence
>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>
>> Later providing RSS has to the application separated as optimization,
>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
>> support.
> What should I do for above two comments？
> To tell application how to use it?
>

Just tried to give some context.


>>
>> As the intention of this feature is "Receive Side Scaling" support, we
>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>
>> Instead perhaps can expand to:
>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
> 
>  I just pick their common part.😁
> 
> ok, will fix it.
> 
>>
>>
>>>   * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>   * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>   * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>> ``mbuf.rss``.
>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>> ``rte_eth_dev_rss_hash_update``
>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>   
>> ack
>>
>>>     .. _nic_features_inner_rss:
>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>   Inner RSS
>>>   ---------
>>>   -Supports RX RSS hashing on Inner headers.
>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>   
>> This should be clarified with details below, not sure if it required to
>> limit description to rte_flow.
> But this block like rte_flow_action_rss is from rte_flow.
> And ethdev ops doesn't support inner RSS.
> So I think it is ok.
>

Yes it is supported by rte_flow, and '[uses]' information should already
clarify it.


>>
>>
>> And I guess similar confusion exist with the providing hash to user.
>> Need to check if rte_flow implementation puts hash to mbuf along with
>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>> and update below items accordingly.
> Do we need to tell user how to use it here?
> I feel this document is a little simple and main to list interface for
> user.
> In addition, it is better that the more detail about RSS should be
> presented  in rte_flow features.
>

No, I am not suggesting to add more detail.

My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
correct, ethdev APIs checks offload flags, but does rte_flow
implementation check it?

My suggestion is double check that piece of information and fix it if
required.


>>
>>
>>>   * **[uses]    rte_flow_action_rss**: ``level``.
>>>   * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>> @@ -303,9 +305,25 @@ RSS key update
>>>   Supports configuration of Receive Side Scaling (RSS) hash
>>> computation. Updating
>>>   Receive Side Scaling (RSS) hash key.
>>>   -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>> ``rss_hash_conf_get``.
>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>> ``rss_conf.rss_key_len``
>>>   * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>> ``rte_eth_dev_rss_hash_update()``,
>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>> +
>> ack
>>
>> There is an inconsistency in the documentation but I think it is good to
>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
> +1 will fix it.
>>
>>
>>> +
>>> +.. _nic_features_rss_hash_algo_update:
>>> +
>>> +RSS hash algorithm update
>>> +-------------------------
>>> +
>>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm.
>>> Updating
>>> +RSS hash algorithm.
>>> +
>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>> ``rte_eth_dev_rss_hash_update()``,
>>>     ``rte_eth_dev_rss_hash_conf_get()``.
>>>     
>>
>> This document describes features listed in the 'default.ini', so we
>> shouldn't have above.
>>
>> And I don't think RSS hash algorithm update is a big enough feature to
>> list in the feature list, perhaps it can be embedded in the RSS support
>> block, what do you think?
> Yes it is not a bit feature.
> so put it to RSS hash, right?
> 

Yes please.

Re: [PATCH v3 0/6] doc/features: fix some features and add new features

[Ferruh Yigit]

On 11/27/2023 1:37 PM, lihuisong (C) wrote:
> 
> 在 2023/11/27 20:35, Ferruh Yigit 写道:
>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>> The support for setting RSS hash algorithm has been introduced to 23.11,
>>> but doesn't add feature for it. So this series add this feature and
>>> perfect
>>> RSS other features by the way.
>>>
>>> In addition, serval features, like "set link up/down", "TM", "dump
>>> device
>>> private information" and "loopback mode", had beed supported, but
>>> they are
>>> absent from the freatures list.
>>>
>>> ---
>>>   -v3:
>>>     - fix description of the patchset cover
>>>     - remove '/features' in 'doc/features' tag for every patch.
>>>     - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>     - retrigger CI build because of wrong CI warning
>>>
>>>   -v2:
>>>     - add loopback mode feature.
>>>
>>> Huisong Li (6):
>>>    doc: add RSS hash algorithm feature
>>>    doc: add link up/down feature
>>>    doc: add features for link speeds
>>>    doc: add Traffic Manager feature
>>>    doc: add dump device private information feature
>>>    doc: add feature for loopback mode
>>>
>>>   
>> Hi Huisong,
>>
>> Some of the patches above adds new features to the 'features.rst', but
>> features.rst document describes features that drivers mark in feature
>> list in .ini file, all features are listed in the 'default.ini'.
>>
>> When a new feature is added into 'default.ini', it can be described in
>> 'features.rst' and later all driver .ini files should be updated to
>> reflect if they support this feature or not.
> correct.
>>
>> And I think no need to document all features, but ones can be important
>> enough or differentiating features for users to know, this line is not
> agree with you.
>> easy to draw, so perhaps we can use community consensus to select.
>>
>> In this patch following features are added:
>> - Link speed configuration
>> - Traffic manager
>> - Device private information dump
>> - Loopback configuration
>>
>>
>> I think 'Traffic manager' can be a new feature, but I am not sure about
>> rest. It would be nice to get more comments.
> Link speed is also important for NIC. Fixed speed and Link autoneg is
> from this.
> I think it can be a new feature. what do you think?
>

Target is to document what a device/driver supports in DPDK, this may
help end user quickly check and see what to expect from a device/driver.

Also helps to gauge how feature complete a driver implementation is.


Is there a value to document if setting fixed speed supported for end
user? My assumption is mostly autoneg is used.


> How about add others to the following block?
> "Other dev ops not represented by a Feature"
> 

Yes there are some dev_ops, implemented by drivers but not documented in
the feature.rst .

Indeed it would be nice to document them too somehow, perhaps not as
part of the feature.rst, but it has its own document for these dev_ops,
and feature.rst can have link to them?

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[Ferruh Yigit]

On 11/27/2023 3:43 PM, Ferruh Yigit wrote:
> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>>
>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS features
>>>> description.
>>>>
>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>> ---
>>>>   doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>   1 file changed, 22 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>> --- a/doc/guides/nics/features.rst
>>>> +++ b/doc/guides/nics/features.rst
>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>   Supports RSS hashing on RX.
>>>>     * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>
>>> Feature title is "RSS hash", it can be two things,
>>> 1. "Receive Side Scaling" support
>>> 2. Provide RSS hash to application
>>>
>>> When this document first prepared RSS hash value was always provided to
>>> the application when RSS enabled.
>>> So intention with this feature was "Receive Side Scaling" support, hence
>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>
>>> Later providing RSS has to the application separated as optimization,
>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
>>> support.
>> What should I do for above two comments？
>> To tell application how to use it?
>>
> 
> Just tried to give some context.
> 
> 
>>>
>>> As the intention of this feature is "Receive Side Scaling" support, we
>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>
>>> Instead perhaps can expand to:
>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>
>>  I just pick their common part.😁
>>
>> ok, will fix it.
>>
>>>
>>>
>>>>   * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>   * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>   * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>> ``mbuf.rss``.
>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>> ``rte_eth_dev_rss_hash_update``
>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>   
>>> ack
>>>
>>>>     .. _nic_features_inner_rss:
>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>   Inner RSS
>>>>   ---------
>>>>   -Supports RX RSS hashing on Inner headers.
>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>   
>>> This should be clarified with details below, not sure if it required to
>>> limit description to rte_flow.
>> But this block like rte_flow_action_rss is from rte_flow.
>> And ethdev ops doesn't support inner RSS.
>> So I think it is ok.
>>
> 
> Yes it is supported by rte_flow, and '[uses]' information should already
> clarify it.
> 
> 
>>>
>>>
>>> And I guess similar confusion exist with the providing hash to user.
>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>> and update below items accordingly.
>> Do we need to tell user how to use it here?
>> I feel this document is a little simple and main to list interface for
>> user.
>> In addition, it is better that the more detail about RSS should be
>> presented  in rte_flow features.
>>
> 
> No, I am not suggesting to add more detail.
> 
> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
> correct, ethdev APIs checks offload flags, but does rte_flow
> implementation check it?
> 
> My suggestion is double check that piece of information and fix it if
> required.
> 

Thinking twice, ethdev API or rte_flow or different ways to configure
RSS, but datapath that puts hash value to mbuf is same.
So same 'RTE_ETH_RX_OFFLOAD_RSS_HASH' check is used for both method, and
it is OK to have it documented.



> 
>>>
>>>
>>>>   * **[uses]    rte_flow_action_rss**: ``level``.
>>>>   * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>   Supports configuration of Receive Side Scaling (RSS) hash
>>>> computation. Updating
>>>>   Receive Side Scaling (RSS) hash key.
>>>>   -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>> ``rss_hash_conf_get``.
>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>> ``rss_conf.rss_key_len``
>>>>   * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>> ``rte_eth_dev_rss_hash_update()``,
>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>> +
>>> ack
>>>
>>> There is an inconsistency in the documentation but I think it is good to
>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>> +1 will fix it.
>>>
>>>
>>>> +
>>>> +.. _nic_features_rss_hash_algo_update:
>>>> +
>>>> +RSS hash algorithm update
>>>> +-------------------------
>>>> +
>>>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm.
>>>> Updating
>>>> +RSS hash algorithm.
>>>> +
>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>     ``rte_eth_dev_rss_hash_conf_get()``.
>>>>     
>>>
>>> This document describes features listed in the 'default.ini', so we
>>> shouldn't have above.
>>>
>>> And I don't think RSS hash algorithm update is a big enough feature to
>>> list in the feature list, perhaps it can be embedded in the RSS support
>>> block, what do you think?
>> Yes it is not a bit feature.
>> so put it to RSS hash, right?
>>
> 
> Yes please.
> 

Re: [PATCH v4 1/5] doc: fix the description of RSS related feature

[Ferruh Yigit]

On 11/27/2023 2:16 PM, Huisong Li wrote:
> This patch fixes the description of RSS feature.
> And the settinf ot hash algorithm is introduced by 23.11, so add it.
> 
> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> 

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Re: [PATCH v4 5/5] doc: add dump device private information ops

[Ferruh Yigit]

On 11/27/2023 2:16 PM, Huisong Li wrote:
> Add dump device private information to dev ops.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> 

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Re: [PATCH v4 0/5] doc/features: fix some features and add new features

[Ferruh Yigit]

On 11/27/2023 2:16 PM, Huisong Li wrote:
> The support for setting RSS hash algorithm has been introduced to 23.11,
> but doesn't add feature for it. So this series add this feature to RSS hash
> and perfect RSS other features by the way.
> 
> In addition, serval features or interface, like "set link up/down", "TM",
> "link speeds" and "dump device private information" had beed supported,
> but they are absent from the freatures list.
> 
> ---
>  -v4:
>    - fix RSS releated feaures description based on Ferruh
>    - add new features to default.ini
>    - remove loopback patch.
>    - move "dump device private information" to "dev ops" block.
> 
>  -v3:
>    - fix description of the patchset cover
>    - remove '/features' in 'doc/features' tag for every patch.
>    - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>    - retrigger CI build because of wrong CI warning
> 
>  -v2:
>    - add loopback mode feature.
> 
> Huisong Li (5):
>   doc: fix the description of RSS related feature
>   doc: add link up/down feature
>   doc: add features for link speeds
>   doc: add Traffic Manager feature
>   doc: add dump device private information ops
> 

As mentioned before I don't know about adding link speed feature or not,
would like to get more comment.

But for the new features added, driver .ini files also needs to be
updated for that new feature, and drivers support it should mark the
support.
That is why it is best to start documenting a feature when it is first
introduced, otherwise it is more work to update it later.

Re: [PATCH v4 0/5] doc/features: fix some features and add new features

[Ferruh Yigit]

On 11/27/2023 2:16 PM, Huisong Li wrote:
> The support for setting RSS hash algorithm has been introduced to 23.11,
> but doesn't add feature for it. So this series add this feature to RSS hash
> and perfect RSS other features by the way.
> 
> In addition, serval features or interface, like "set link up/down", "TM",
> "link speeds" and "dump device private information" had beed supported,
> but they are absent from the freatures list.
> 
> ---
>  -v4:
>    - fix RSS releated feaures description based on Ferruh
>    - add new features to default.ini
>    - remove loopback patch.
>    - move "dump device private information" to "dev ops" block.
> 
>  -v3:
>    - fix description of the patchset cover
>    - remove '/features' in 'doc/features' tag for every patch.
>    - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>    - retrigger CI build because of wrong CI warning
> 
>  -v2:
>    - add loopback mode feature.
> 
> Huisong Li (5):
>   doc: fix the description of RSS related feature
>   doc: add link up/down feature
>   doc: add features for link speeds
>   doc: add Traffic Manager feature
>   doc: add dump device private information ops
> 

Please check the warning on document generation:
https://mails.dpdk.org/archives/test-report/2023-November/519586.html

/home/runner/work/dpdk/dpdk/doc/guides/nics/features.rst:307:Unexpected
indentation.

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[lihuisong (C)]

在 2023/11/27 23:43, Ferruh Yigit 写道:
> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS features
>>>> description.
>>>>
>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>> ---
>>>>    doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>    1 file changed, 22 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>> --- a/doc/guides/nics/features.rst
>>>> +++ b/doc/guides/nics/features.rst
>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>    Supports RSS hashing on RX.
>>>>      * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>
>>> Feature title is "RSS hash", it can be two things,
>>> 1. "Receive Side Scaling" support
>>> 2. Provide RSS hash to application
>>>
>>> When this document first prepared RSS hash value was always provided to
>>> the application when RSS enabled.
>>> So intention with this feature was "Receive Side Scaling" support, hence
>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>
>>> Later providing RSS has to the application separated as optimization,
>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
>>> support.
>> What should I do for above two comments？
>> To tell application how to use it?
>>
> Just tried to give some context.
got it.
>   
>
>
>>> As the intention of this feature is "Receive Side Scaling" support, we
>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>
>>> Instead perhaps can expand to:
>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>   I just pick their common part.😁
>>
>> ok, will fix it.
>>
>>>
>>>>    * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>    * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>    * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>> ``mbuf.rss``.
>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>> ``rte_eth_dev_rss_hash_update``
>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>    
>>> ack
>>>
>>>>      .. _nic_features_inner_rss:
>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>    Inner RSS
>>>>    ---------
>>>>    -Supports RX RSS hashing on Inner headers.
>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>    
>>> This should be clarified with details below, not sure if it required to
>>> limit description to rte_flow.
>> But this block like rte_flow_action_rss is from rte_flow.
>> And ethdev ops doesn't support inner RSS.
>> So I think it is ok.
>>
> Yes it is supported by rte_flow, and '[uses]' information should already
> clarify it.
Should we remove the 'rte_flow API' wrods I added in above description?
>
>>>
>>> And I guess similar confusion exist with the providing hash to user.
>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>> and update below items accordingly.
>> Do we need to tell user how to use it here?
>> I feel this document is a little simple and main to list interface for
>> user.
>> In addition, it is better that the more detail about RSS should be
>> presented  in rte_flow features.
>>
> No, I am not suggesting to add more detail.
>
> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
> correct, ethdev APIs checks offload flags, but does rte_flow
> implementation check it?
As far as I know, It is possibly verified in PMD if have or required.
>
> My suggestion is double check that piece of information and fix it if
> required.
>
>
>>>
>>>>    * **[uses]    rte_flow_action_rss**: ``level``.
>>>>    * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>    Supports configuration of Receive Side Scaling (RSS) hash
>>>> computation. Updating
>>>>    Receive Side Scaling (RSS) hash key.
>>>>    -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>> ``rss_hash_conf_get``.
>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>> ``rss_conf.rss_key_len``
>>>>    * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>> ``rte_eth_dev_rss_hash_update()``,
>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>> +
>>> ack
>>>
>>> There is an inconsistency in the documentation but I think it is good to
>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>> +1 will fix it.
>>>
>>>> +
>>>> +.. _nic_features_rss_hash_algo_update:
>>>> +
>>>> +RSS hash algorithm update
>>>> +-------------------------
>>>> +
>>>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm.
>>>> Updating
>>>> +RSS hash algorithm.
>>>> +
>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>      ``rte_eth_dev_rss_hash_conf_get()``.
>>>>      
>>> This document describes features listed in the 'default.ini', so we
>>> shouldn't have above.
>>>
>>> And I don't think RSS hash algorithm update is a big enough feature to
>>> list in the feature list, perhaps it can be embedded in the RSS support
>>> block, what do you think?
>> Yes it is not a bit feature.
>> so put it to RSS hash, right?
>>
> Yes please.
>
> .

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[lihuisong (C)]

在 2023/11/28 0:35, Ferruh Yigit 写道:
> On 11/27/2023 3:43 PM, Ferruh Yigit wrote:
>> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS features
>>>>> description.
>>>>>
>>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>>
>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>> ---
>>>>>    doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>>    1 file changed, 22 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
>>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>>> --- a/doc/guides/nics/features.rst
>>>>> +++ b/doc/guides/nics/features.rst
>>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>>    Supports RSS hashing on RX.
>>>>>      * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>>
>>>> Feature title is "RSS hash", it can be two things,
>>>> 1. "Receive Side Scaling" support
>>>> 2. Provide RSS hash to application
>>>>
>>>> When this document first prepared RSS hash value was always provided to
>>>> the application when RSS enabled.
>>>> So intention with this feature was "Receive Side Scaling" support, hence
>>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>>
>>>> Later providing RSS has to the application separated as optimization,
>>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
>>>> support.
>>> What should I do for above two comments？
>>> To tell application how to use it?
>>>
>> Just tried to give some context.
>>
>>
>>>> As the intention of this feature is "Receive Side Scaling" support, we
>>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>>
>>>> Instead perhaps can expand to:
>>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>>   I just pick their common part.😁
>>>
>>> ok, will fix it.
>>>
>>>>
>>>>>    * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>    * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>>    * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>>> ``mbuf.rss``.
>>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>>> ``rte_eth_dev_rss_hash_update``
>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>    
>>>> ack
>>>>
>>>>>      .. _nic_features_inner_rss:
>>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>>    Inner RSS
>>>>>    ---------
>>>>>    -Supports RX RSS hashing on Inner headers.
>>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>>    
>>>> This should be clarified with details below, not sure if it required to
>>>> limit description to rte_flow.
>>> But this block like rte_flow_action_rss is from rte_flow.
>>> And ethdev ops doesn't support inner RSS.
>>> So I think it is ok.
>>>
>> Yes it is supported by rte_flow, and '[uses]' information should already
>> clarify it.
>>
>>
>>>>
>>>> And I guess similar confusion exist with the providing hash to user.
>>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>>> and update below items accordingly.
>>> Do we need to tell user how to use it here?
>>> I feel this document is a little simple and main to list interface for
>>> user.
>>> In addition, it is better that the more detail about RSS should be
>>> presented  in rte_flow features.
>>>
>> No, I am not suggesting to add more detail.
>>
>> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
>> correct, ethdev APIs checks offload flags, but does rte_flow
>> implementation check it?
>>
>> My suggestion is double check that piece of information and fix it if
>> required.
>>
> Thinking twice, ethdev API or rte_flow or different ways to configure
> RSS, but datapath that puts hash value to mbuf is same.
> So same 'RTE_ETH_RX_OFFLOAD_RSS_HASH' check is used for both method, and
> it is OK to have it documented.
There was a check for  RTE_ETH_RX_OFFLOAD_RSS_HASH in 
rte_eth_dev_configure().
This offload flag depends on the RTE_ETH_MQ_RX_RSS_FLAG mode.
As far as I know, ethdev ops allows to enable RSS hash only when 
RTE_ETH_MQ_RX_RSS_FLAG mode is set.
But rte flow API enable RSS hash doesn't depend on this MQ mode.
So I guess that the mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH and mbuf.rss 
also be set to report application when create RSS hash flow, even if 
application don't set RTE_ETH_MQ_RX_RSS_FLAG mode by ethdev ops.
>
>
>
>>>>
>>>>>    * **[uses]    rte_flow_action_rss**: ``level``.
>>>>>    * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>>    Supports configuration of Receive Side Scaling (RSS) hash
>>>>> computation. Updating
>>>>>    Receive Side Scaling (RSS) hash key.
>>>>>    -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>>> ``rss_hash_conf_get``.
>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>>> ``rss_conf.rss_key_len``
>>>>>    * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>> +
>>>> ack
>>>>
>>>> There is an inconsistency in the documentation but I think it is good to
>>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>>> +1 will fix it.
>>>>
>>>>> +
>>>>> +.. _nic_features_rss_hash_algo_update:
>>>>> +
>>>>> +RSS hash algorithm update
>>>>> +-------------------------
>>>>> +
>>>>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm.
>>>>> Updating
>>>>> +RSS hash algorithm.
>>>>> +
>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>      ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>      
>>>> This document describes features listed in the 'default.ini', so we
>>>> shouldn't have above.
>>>>
>>>> And I don't think RSS hash algorithm update is a big enough feature to
>>>> list in the feature list, perhaps it can be embedded in the RSS support
>>>> block, what do you think?
>>> Yes it is not a bit feature.
>>> so put it to RSS hash, right?
>>>
>> Yes please.
>>
> .

Re: [PATCH v4 0/5] doc/features: fix some features and add new features

[lihuisong (C)]

在 2023/11/28 0:44, Ferruh Yigit 写道:
> On 11/27/2023 2:16 PM, Huisong Li wrote:
>> The support for setting RSS hash algorithm has been introduced to 23.11,
>> but doesn't add feature for it. So this series add this feature to RSS hash
>> and perfect RSS other features by the way.
>>
>> In addition, serval features or interface, like "set link up/down", "TM",
>> "link speeds" and "dump device private information" had beed supported,
>> but they are absent from the freatures list.
>>
>> ---
>>   -v4:
>>     - fix RSS releated feaures description based on Ferruh
>>     - add new features to default.ini
>>     - remove loopback patch.
>>     - move "dump device private information" to "dev ops" block.
>>
>>   -v3:
>>     - fix description of the patchset cover
>>     - remove '/features' in 'doc/features' tag for every patch.
>>     - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>     - retrigger CI build because of wrong CI warning
>>
>>   -v2:
>>     - add loopback mode feature.
>>
>> Huisong Li (5):
>>    doc: fix the description of RSS related feature
>>    doc: add link up/down feature
>>    doc: add features for link speeds
>>    doc: add Traffic Manager feature
>>    doc: add dump device private information ops
>>
> As mentioned before I don't know about adding link speed feature or not,
> would like to get more comment.
ok, let us to get more comment.
it is better that we send a separated patch about link speed feature to 
discuss and get more comment.
In this way, other patch in this series also go ahead to upload.
what do you think?
>
> But for the new features added, driver .ini files also needs to be
> updated for that new feature, and drivers support it should mark the
> support.
agree.
I will add "Traffic Manager" feature to driver.ini for the supported TM 
driver.
But I'm not sure if I am supposed to do this in patch 4/5 or send 
independent patch for every supported driver.
what do you think of that?
> That is why it is best to start documenting a feature when it is first
> introduced, otherwise it is more work to update it later.
agree with you.
we should remind someone to add it to this feature doc when a new 
feature is introduced.
>
> .

Re: [PATCH v4 0/5] doc/features: fix some features and add new features

[lihuisong (C)]

在 2023/11/28 0:56, Ferruh Yigit 写道:
> On 11/27/2023 2:16 PM, Huisong Li wrote:
>> The support for setting RSS hash algorithm has been introduced to 23.11,
>> but doesn't add feature for it. So this series add this feature to RSS hash
>> and perfect RSS other features by the way.
>>
>> In addition, serval features or interface, like "set link up/down", "TM",
>> "link speeds" and "dump device private information" had beed supported,
>> but they are absent from the freatures list.
>>
>> ---
>>   -v4:
>>     - fix RSS releated feaures description based on Ferruh
>>     - add new features to default.ini
>>     - remove loopback patch.
>>     - move "dump device private information" to "dev ops" block.
>>
>>   -v3:
>>     - fix description of the patchset cover
>>     - remove '/features' in 'doc/features' tag for every patch.
>>     - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>     - retrigger CI build because of wrong CI warning
>>
>>   -v2:
>>     - add loopback mode feature.
>>
>> Huisong Li (5):
>>    doc: fix the description of RSS related feature
>>    doc: add link up/down feature
>>    doc: add features for link speeds
>>    doc: add Traffic Manager feature
>>    doc: add dump device private information ops
>>
> Please check the warning on document generation:
> https://mails.dpdk.org/archives/test-report/2023-November/519586.html
>
> /home/runner/work/dpdk/dpdk/doc/guides/nics/features.rst:307:Unexpected
> indentation.
Here has a format issue. I will fix it next version.
Thanks.
>
> .

Re: [PATCH v4 0/5] doc/features: fix some features and add new features

[lihuisong (C)]

在 2023/11/28 10:09, lihuisong (C) 写道:
>
> 在 2023/11/28 0:44, Ferruh Yigit 写道:
>> On 11/27/2023 2:16 PM, Huisong Li wrote:
>>> The support for setting RSS hash algorithm has been introduced to 
>>> 23.11,
>>> but doesn't add feature for it. So this series add this feature to 
>>> RSS hash
>>> and perfect RSS other features by the way.
>>>
>>> In addition, serval features or interface, like "set link up/down", 
>>> "TM",
>>> "link speeds" and "dump device private information" had beed supported,
>>> but they are absent from the freatures list.
>>>
>>> ---
>>>   -v4:
>>>     - fix RSS releated feaures description based on Ferruh
>>>     - add new features to default.ini
>>>     - remove loopback patch.
>>>     - move "dump device private information" to "dev ops" block.
>>>
>>>   -v3:
>>>     - fix description of the patchset cover
>>>     - remove '/features' in 'doc/features' tag for every patch.
>>>     - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>     - retrigger CI build because of wrong CI warning
>>>
>>>   -v2:
>>>     - add loopback mode feature.
>>>
>>> Huisong Li (5):
>>>    doc: fix the description of RSS related feature
>>>    doc: add link up/down feature
>>>    doc: add features for link speeds
>>>    doc: add Traffic Manager feature
>>>    doc: add dump device private information ops
>>>
>> As mentioned before I don't know about adding link speed feature or not,
>> would like to get more comment.
> ok, let us to get more comment.
> it is better that we send a separated patch about link speed feature 
> to discuss and get more comment.
> In this way, other patch in this series also go ahead to upload.
> what do you think?
>>
>> But for the new features added, driver .ini files also needs to be
>> updated for that new feature, and drivers support it should mark the
>> support.
> agree.
> I will add "Traffic Manager" feature to driver.ini for the supported 
> TM driver.
> But I'm not sure if I am supposed to do this in patch 4/5 or send 
> independent patch for every supported driver.
Thinking twice, I will separate this "TM" patch from this series and 
send a independent patchset to do this.
Then we can take a look at that series.
> what do you think of that?
>> That is why it is best to start documenting a feature when it is first
>> introduced, otherwise it is more work to update it later.
> agree with you.
> we should remind someone to add it to this feature doc when a new 
> feature is introduced.
>>
>> .
> .

[PATCH v5 0/3] doc/features: fix some features and add new features

[Huisong Li]

The support for setting RSS hash algorithm has been introduced to 23.11,
but doesn't add feature for it. So this series add this feature to RSS hash
and perfect RSS other features by the way.

In addition, "set link up/down" and "dump device private information" had
beed supported, but they are absent from the freatures list.

---
 -v5:
   - separate "link speed" and "Traffic Manager" feature from this series
   - fix CI warning 

 -v4:
   - fix RSS releated feaures description based on Ferruh
   - add new features to default.ini
   - remove loopback patch.
   - move "dump device private information" to "dev ops" block.

 -v3:
   - fix description of the patchset cover
   - remove '/features' in 'doc/features' tag for every patch.
   - add Acked-by: Chengwen Feng <fengchengwen@huawei.com>
   - retrigger CI build because of wrong CI warning

 -v2:
   - add loopback mode feature.

Huisong Li (3):
  doc: fix the description of RSS related feature
  doc: add link up/down feature
  doc: add dump device private information ops

 doc/guides/nics/features.rst | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

-- 
2.33.0

[PATCH v5 1/3] doc: fix the description of RSS related feature

[Huisong Li]

This patch fixes the description of RSS feature.
And the settinf ot hash algorithm is introduced by 23.11, so add it.

Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 doc/guides/nics/features.rst | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 1a1dc16c1e..18b4c9637e 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -277,10 +277,20 @@ RSS hash
 Supports RSS hashing on RX.
 
 * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
-* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
+* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf.rss_hf``. ``rss_conf.rss_hf``
 * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
 * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
 * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
+* **[related]  API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update``
+  ``rte_eth_dev_rss_hash_conf_get()``.
+
+Support RSS hash algorithm on Rx.
+
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.algorithm``
+* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
+* **[related]    API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update()``,
+  ``rte_eth_dev_rss_hash_conf_get()``.
 
 
 .. _nic_features_inner_rss:
@@ -288,7 +298,7 @@ Supports RSS hashing on RX.
 Inner RSS
 ---------
 
-Supports RX RSS hashing on Inner headers.
+Supports RX RSS hashing on Inner headers by rte_flow API.
 
 * **[uses]    rte_flow_action_rss**: ``level``.
 * **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
@@ -303,9 +313,10 @@ RSS key update
 Supports configuration of Receive Side Scaling (RSS) hash computation. Updating
 Receive Side Scaling (RSS) hash key.
 
-* **[implements] eth_dev_ops**: ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
+* **[uses]     user config**: ``rss_conf.rss_key``, ``rss_conf.rss_key_len``
 * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
-* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
+* **[related]    API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update()``,
   ``rte_eth_dev_rss_hash_conf_get()``.
 
 
-- 
2.33.0

[PATCH v5 2/3] doc: add link up/down feature

[Huisong Li]

Add link up/down feature for features.rst.

Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 doc/guides/nics/features.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 18b4c9637e..ced400beba 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -45,6 +45,10 @@ Supports getting the link speed, duplex mode and link state (up/down).
 * **[implements] rte_eth_dev_data**: ``dev_link``.
 * **[related]    API**: ``rte_eth_link_get()``, ``rte_eth_link_get_nowait()``.
 
+Set link up/down an Ethernet device.
+
+* **[implements] eth_dev_ops**: ``dev_set_link_up``, ``dev_set_link_down``.
+* **[related]    API**: ``rte_eth_dev_set_link_up()``, ``rte_eth_dev_set_link_down()``.
 
 .. _nic_features_link_status_event:
 
-- 
2.33.0

[PATCH v5 3/3] doc: add dump device private information ops

[Huisong Li]

Add dump device private information to dev ops.

Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
 doc/guides/nics/features.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index ced400beba..96a6f11ee3 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -982,3 +982,4 @@ Other dev ops not represented by a Feature
 * ``udp_tunnel_port_add``
 * ``udp_tunnel_port_del``
 * ``tx_pkt_prepare``
+* ``eth_dev_priv_dump``
-- 
2.33.0

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[Ferruh Yigit]

On 11/28/2023 1:21 AM, lihuisong (C) wrote:
> 
> 在 2023/11/27 23:43, Ferruh Yigit 写道:
>> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS
>>>>> features
>>>>> description.
>>>>>
>>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>>
>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>> ---
>>>>>    doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>>    1 file changed, 22 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/doc/guides/nics/features.rst
>>>>> b/doc/guides/nics/features.rst
>>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>>> --- a/doc/guides/nics/features.rst
>>>>> +++ b/doc/guides/nics/features.rst
>>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>>    Supports RSS hashing on RX.
>>>>>      * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>>
>>>> Feature title is "RSS hash", it can be two things,
>>>> 1. "Receive Side Scaling" support
>>>> 2. Provide RSS hash to application
>>>>
>>>> When this document first prepared RSS hash value was always provided to
>>>> the application when RSS enabled.
>>>> So intention with this feature was "Receive Side Scaling" support,
>>>> hence
>>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>>
>>>> Later providing RSS has to the application separated as optimization,
>>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
>>>> support.
>>> What should I do for above two comments？
>>> To tell application how to use it?
>>>
>> Just tried to give some context.
> got it.
>>  
>>
>>>> As the intention of this feature is "Receive Side Scaling" support, we
>>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>>
>>>> Instead perhaps can expand to:
>>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>>   I just pick their common part.😁
>>>
>>> ok, will fix it.
>>>
>>>>
>>>>>    * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>    * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>>    * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>>> ``mbuf.rss``.
>>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>>> ``rte_eth_dev_rss_hash_update``
>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>    
>>>> ack
>>>>
>>>>>      .. _nic_features_inner_rss:
>>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>>    Inner RSS
>>>>>    ---------
>>>>>    -Supports RX RSS hashing on Inner headers.
>>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>>    
>>>> This should be clarified with details below, not sure if it required to
>>>> limit description to rte_flow.
>>> But this block like rte_flow_action_rss is from rte_flow.
>>> And ethdev ops doesn't support inner RSS.
>>> So I think it is ok.
>>>
>> Yes it is supported by rte_flow, and '[uses]' information should already
>> clarify it.
> Should we remove the 'rte_flow API' wrods I added in above description?
>

I think it can be removed.


>>
>>>>
>>>> And I guess similar confusion exist with the providing hash to user.
>>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>>> and update below items accordingly.
>>> Do we need to tell user how to use it here?
>>> I feel this document is a little simple and main to list interface for
>>> user.
>>> In addition, it is better that the more detail about RSS should be
>>> presented  in rte_flow features.
>>>
>> No, I am not suggesting to add more detail.
>>
>> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
>> correct, ethdev APIs checks offload flags, but does rte_flow
>> implementation check it?
> As far as I know, It is possibly verified in PMD if have or required.
>>
>> My suggestion is double check that piece of information and fix it if
>> required.
>>
>>
>>>>
>>>>>    * **[uses]    rte_flow_action_rss**: ``level``.
>>>>>    * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>>    Supports configuration of Receive Side Scaling (RSS) hash
>>>>> computation. Updating
>>>>>    Receive Side Scaling (RSS) hash key.
>>>>>    -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>>> ``rss_hash_conf_get``.
>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>>> ``rss_conf.rss_key_len``
>>>>>    * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>> +
>>>> ack
>>>>
>>>> There is an inconsistency in the documentation but I think it is
>>>> good to
>>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>>> +1 will fix it.
>>>>
>>>>> +
>>>>> +.. _nic_features_rss_hash_algo_update:
>>>>> +
>>>>> +RSS hash algorithm update
>>>>> +-------------------------
>>>>> +
>>>>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm.
>>>>> Updating
>>>>> +RSS hash algorithm.
>>>>> +
>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>      ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>      
>>>> This document describes features listed in the 'default.ini', so we
>>>> shouldn't have above.
>>>>
>>>> And I don't think RSS hash algorithm update is a big enough feature to
>>>> list in the feature list, perhaps it can be embedded in the RSS support
>>>> block, what do you think?
>>> Yes it is not a bit feature.
>>> so put it to RSS hash, right?
>>>
>> Yes please.
>>
>> .

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[Ferruh Yigit]

On 11/28/2023 1:34 AM, lihuisong (C) wrote:
> 
> 在 2023/11/28 0:35, Ferruh Yigit 写道:
>> On 11/27/2023 3:43 PM, Ferruh Yigit wrote:
>>> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>>>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS
>>>>>> features
>>>>>> description.
>>>>>>
>>>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>>>
>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>>> ---
>>>>>>    doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>>>    1 file changed, 22 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/doc/guides/nics/features.rst
>>>>>> b/doc/guides/nics/features.rst
>>>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>>>> --- a/doc/guides/nics/features.rst
>>>>>> +++ b/doc/guides/nics/features.rst
>>>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>>>    Supports RSS hashing on RX.
>>>>>>      * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>>>
>>>>> Feature title is "RSS hash", it can be two things,
>>>>> 1. "Receive Side Scaling" support
>>>>> 2. Provide RSS hash to application
>>>>>
>>>>> When this document first prepared RSS hash value was always
>>>>> provided to
>>>>> the application when RSS enabled.
>>>>> So intention with this feature was "Receive Side Scaling" support,
>>>>> hence
>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>>>
>>>>> Later providing RSS has to the application separated as optimization,
>>>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for
>>>>> this
>>>>> support.
>>>> What should I do for above two comments？
>>>> To tell application how to use it?
>>>>
>>> Just tried to give some context.
>>>
>>>
>>>>> As the intention of this feature is "Receive Side Scaling" support, we
>>>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>>>
>>>>> Instead perhaps can expand to:
>>>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>>>   I just pick their common part.😁
>>>>
>>>> ok, will fix it.
>>>>
>>>>>
>>>>>>    * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>>    * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>>>    * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>>>> ``mbuf.rss``.
>>>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>>>> ``rte_eth_dev_rss_hash_update``
>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>    
>>>>> ack
>>>>>
>>>>>>      .. _nic_features_inner_rss:
>>>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>>>    Inner RSS
>>>>>>    ---------
>>>>>>    -Supports RX RSS hashing on Inner headers.
>>>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>>>    
>>>>> This should be clarified with details below, not sure if it
>>>>> required to
>>>>> limit description to rte_flow.
>>>> But this block like rte_flow_action_rss is from rte_flow.
>>>> And ethdev ops doesn't support inner RSS.
>>>> So I think it is ok.
>>>>
>>> Yes it is supported by rte_flow, and '[uses]' information should already
>>> clarify it.
>>>
>>>
>>>>>
>>>>> And I guess similar confusion exist with the providing hash to user.
>>>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>>>> and update below items accordingly.
>>>> Do we need to tell user how to use it here?
>>>> I feel this document is a little simple and main to list interface for
>>>> user.
>>>> In addition, it is better that the more detail about RSS should be
>>>> presented  in rte_flow features.
>>>>
>>> No, I am not suggesting to add more detail.
>>>
>>> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
>>> correct, ethdev APIs checks offload flags, but does rte_flow
>>> implementation check it?
>>>
>>> My suggestion is double check that piece of information and fix it if
>>> required.
>>>
>> Thinking twice, ethdev API or rte_flow or different ways to configure
>> RSS, but datapath that puts hash value to mbuf is same.
>> So same 'RTE_ETH_RX_OFFLOAD_RSS_HASH' check is used for both method, and
>> it is OK to have it documented.
> There was a check for  RTE_ETH_RX_OFFLOAD_RSS_HASH in
> rte_eth_dev_configure().
> This offload flag depends on the RTE_ETH_MQ_RX_RSS_FLAG mode.
> As far as I know, ethdev ops allows to enable RSS hash only when
> RTE_ETH_MQ_RX_RSS_FLAG mode is set.
> But rte flow API enable RSS hash doesn't depend on this MQ mode.
> So I guess that the mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH and mbuf.rss
> also be set to report application when create RSS hash flow, even if
> application don't set RTE_ETH_MQ_RX_RSS_FLAG mode by ethdev ops.
>

Even rte_flow is used for RSS, application may want to control exposing
hash value via 'RTE_ETH_RX_OFFLOAD_RSS_HASH' flag, so lets keep the
documentation as it is.

But in implementation it is possible that some drivers may not be using
the offload flag, as you explained above, this confused me but we still
should document what is expected from driver.


>>
>>
>>
>>>>>
>>>>>>    * **[uses]    rte_flow_action_rss**: ``level``.
>>>>>>    * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>>>    Supports configuration of Receive Side Scaling (RSS) hash
>>>>>> computation. Updating
>>>>>>    Receive Side Scaling (RSS) hash key.
>>>>>>    -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>>>> ``rss_hash_conf_get``.
>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>>>> ``rss_conf.rss_key_len``
>>>>>>    * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>> +
>>>>> ack
>>>>>
>>>>> There is an inconsistency in the documentation but I think it is
>>>>> good to
>>>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>>>> +1 will fix it.
>>>>>
>>>>>> +
>>>>>> +.. _nic_features_rss_hash_algo_update:
>>>>>> +
>>>>>> +RSS hash algorithm update
>>>>>> +-------------------------
>>>>>> +
>>>>>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm.
>>>>>> Updating
>>>>>> +RSS hash algorithm.
>>>>>> +
>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>>      ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>      
>>>>> This document describes features listed in the 'default.ini', so we
>>>>> shouldn't have above.
>>>>>
>>>>> And I don't think RSS hash algorithm update is a big enough feature to
>>>>> list in the feature list, perhaps it can be embedded in the RSS
>>>>> support
>>>>> block, what do you think?
>>>> Yes it is not a bit feature.
>>>> so put it to RSS hash, right?
>>>>
>>> Yes please.
>>>
>> .

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[lihuisong (C)]

在 2023/11/28 18:09, Ferruh Yigit 写道:
> On 11/28/2023 1:21 AM, lihuisong (C) wrote:
>> 在 2023/11/27 23:43, Ferruh Yigit 写道:
>>> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>>>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS
>>>>>> features
>>>>>> description.
>>>>>>
>>>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>>>
>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>>> ---
>>>>>>     doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>>>     1 file changed, 22 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/doc/guides/nics/features.rst
>>>>>> b/doc/guides/nics/features.rst
>>>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>>>> --- a/doc/guides/nics/features.rst
>>>>>> +++ b/doc/guides/nics/features.rst
>>>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>>>     Supports RSS hashing on RX.
>>>>>>       * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>>>
>>>>> Feature title is "RSS hash", it can be two things,
>>>>> 1. "Receive Side Scaling" support
>>>>> 2. Provide RSS hash to application
>>>>>
>>>>> When this document first prepared RSS hash value was always provided to
>>>>> the application when RSS enabled.
>>>>> So intention with this feature was "Receive Side Scaling" support,
>>>>> hence
>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>>>
>>>>> Later providing RSS has to the application separated as optimization,
>>>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for this
>>>>> support.
>>>> What should I do for above two comments？
>>>> To tell application how to use it?
>>>>
>>> Just tried to give some context.
>> got it.
>>>   
>>>
>>>>> As the intention of this feature is "Receive Side Scaling" support, we
>>>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>>>
>>>>> Instead perhaps can expand to:
>>>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>>>    I just pick their common part.😁
>>>>
>>>> ok, will fix it.
>>>>
>>>>>>     * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>>     * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>>>     * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>>>> ``mbuf.rss``.
>>>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>>>> ``rte_eth_dev_rss_hash_update``
>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>     
>>>>> ack
>>>>>
>>>>>>       .. _nic_features_inner_rss:
>>>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>>>     Inner RSS
>>>>>>     ---------
>>>>>>     -Supports RX RSS hashing on Inner headers.
>>>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>>>     
>>>>> This should be clarified with details below, not sure if it required to
>>>>> limit description to rte_flow.
>>>> But this block like rte_flow_action_rss is from rte_flow.
>>>> And ethdev ops doesn't support inner RSS.
>>>> So I think it is ok.
>>>>
>>> Yes it is supported by rte_flow, and '[uses]' information should already
>>> clarify it.
>> Should we remove the 'rte_flow API' wrods I added in above description?
>>
> I think it can be removed.
The latest version(V5, only three fix doc patch) has been sent out.
Do we I need to send V6 for this?😂
>
>
>>>>> And I guess similar confusion exist with the providing hash to user.
>>>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>>>> and update below items accordingly.
>>>> Do we need to tell user how to use it here?
>>>> I feel this document is a little simple and main to list interface for
>>>> user.
>>>> In addition, it is better that the more detail about RSS should be
>>>> presented  in rte_flow features.
>>>>
>>> No, I am not suggesting to add more detail.
>>>
>>> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
>>> correct, ethdev APIs checks offload flags, but does rte_flow
>>> implementation check it?
>> As far as I know, It is possibly verified in PMD if have or required.
>>> My suggestion is double check that piece of information and fix it if
>>> required.
>>>
>>>
>>>>>>     * **[uses]    rte_flow_action_rss**: ``level``.
>>>>>>     * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>>>     Supports configuration of Receive Side Scaling (RSS) hash
>>>>>> computation. Updating
>>>>>>     Receive Side Scaling (RSS) hash key.
>>>>>>     -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>>>> ``rss_hash_conf_get``.
>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>>>> ``rss_conf.rss_key_len``
>>>>>>     * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>> +
>>>>> ack
>>>>>
>>>>> There is an inconsistency in the documentation but I think it is
>>>>> good to
>>>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>>>> +1 will fix it.
>>>>>> +
>>>>>> +.. _nic_features_rss_hash_algo_update:
>>>>>> +
>>>>>> +RSS hash algorithm update
>>>>>> +-------------------------
>>>>>> +
>>>>>> +Supports configuration of Receive Side Scaling (RSS) hash algorithm.
>>>>>> Updating
>>>>>> +RSS hash algorithm.
>>>>>> +
>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>>       ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>       
>>>>> This document describes features listed in the 'default.ini', so we
>>>>> shouldn't have above.
>>>>>
>>>>> And I don't think RSS hash algorithm update is a big enough feature to
>>>>> list in the feature list, perhaps it can be embedded in the RSS support
>>>>> block, what do you think?
>>>> Yes it is not a bit feature.
>>>> so put it to RSS hash, right?
>>>>
>>> Yes please.
>>>
>>> .
> .

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[Ferruh Yigit]

On 11/28/2023 11:29 AM, lihuisong (C) wrote:
> 
> 在 2023/11/28 18:09, Ferruh Yigit 写道:
>> On 11/28/2023 1:21 AM, lihuisong (C) wrote:
>>> 在 2023/11/27 23:43, Ferruh Yigit 写道:
>>>> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>>>>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>>>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS
>>>>>>> features
>>>>>>> description.
>>>>>>>
>>>>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>>>>
>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>>>> ---
>>>>>>>     doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>>>>     1 file changed, 22 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/doc/guides/nics/features.rst
>>>>>>> b/doc/guides/nics/features.rst
>>>>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>>>>> --- a/doc/guides/nics/features.rst
>>>>>>> +++ b/doc/guides/nics/features.rst
>>>>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>>>>     Supports RSS hashing on RX.
>>>>>>>       * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>>>>
>>>>>> Feature title is "RSS hash", it can be two things,
>>>>>> 1. "Receive Side Scaling" support
>>>>>> 2. Provide RSS hash to application
>>>>>>
>>>>>> When this document first prepared RSS hash value was always
>>>>>> provided to
>>>>>> the application when RSS enabled.
>>>>>> So intention with this feature was "Receive Side Scaling" support,
>>>>>> hence
>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>>>>
>>>>>> Later providing RSS has to the application separated as optimization,
>>>>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for
>>>>>> this
>>>>>> support.
>>>>> What should I do for above two comments？
>>>>> To tell application how to use it?
>>>>>
>>>> Just tried to give some context.
>>> got it.
>>>>  
>>>>>> As the intention of this feature is "Receive Side Scaling"
>>>>>> support, we
>>>>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>>>>
>>>>>> Instead perhaps can expand to:
>>>>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>>>>    I just pick their common part.😁
>>>>>
>>>>> ok, will fix it.
>>>>>
>>>>>>>     * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>>>     * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>>>>     * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>>>>> ``mbuf.rss``.
>>>>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>>>>> ``rte_eth_dev_rss_hash_update``
>>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>>     
>>>>>> ack
>>>>>>
>>>>>>>       .. _nic_features_inner_rss:
>>>>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>>>>     Inner RSS
>>>>>>>     ---------
>>>>>>>     -Supports RX RSS hashing on Inner headers.
>>>>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>>>>     
>>>>>> This should be clarified with details below, not sure if it
>>>>>> required to
>>>>>> limit description to rte_flow.
>>>>> But this block like rte_flow_action_rss is from rte_flow.
>>>>> And ethdev ops doesn't support inner RSS.
>>>>> So I think it is ok.
>>>>>
>>>> Yes it is supported by rte_flow, and '[uses]' information should
>>>> already
>>>> clarify it.
>>> Should we remove the 'rte_flow API' wrods I added in above description?
>>>
>> I think it can be removed.
> The latest version(V5, only three fix doc patch) has been sent out.
> Do we I need to send V6 for this?😂
>

Nope, no new version needed for this, I already acked the one with it,
but if there will be a new version this can be updated.

Thanks Huisong for improving the documentation.

>>
>>
>>>>>> And I guess similar confusion exist with the providing hash to user.
>>>>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>>>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>>>>> and update below items accordingly.
>>>>> Do we need to tell user how to use it here?
>>>>> I feel this document is a little simple and main to list interface for
>>>>> user.
>>>>> In addition, it is better that the more detail about RSS should be
>>>>> presented  in rte_flow features.
>>>>>
>>>> No, I am not suggesting to add more detail.
>>>>
>>>> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
>>>> correct, ethdev APIs checks offload flags, but does rte_flow
>>>> implementation check it?
>>> As far as I know, It is possibly verified in PMD if have or required.
>>>> My suggestion is double check that piece of information and fix it if
>>>> required.
>>>>
>>>>
>>>>>>>     * **[uses]    rte_flow_action_rss**: ``level``.
>>>>>>>     * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>>>>     Supports configuration of Receive Side Scaling (RSS) hash
>>>>>>> computation. Updating
>>>>>>>     Receive Side Scaling (RSS) hash key.
>>>>>>>     -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>>>>> ``rss_hash_conf_get``.
>>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>>>>> ``rss_conf.rss_key_len``
>>>>>>>     * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>> +
>>>>>> ack
>>>>>>
>>>>>> There is an inconsistency in the documentation but I think it is
>>>>>> good to
>>>>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>>>>> +1 will fix it.
>>>>>>> +
>>>>>>> +.. _nic_features_rss_hash_algo_update:
>>>>>>> +
>>>>>>> +RSS hash algorithm update
>>>>>>> +-------------------------
>>>>>>> +
>>>>>>> +Supports configuration of Receive Side Scaling (RSS) hash
>>>>>>> algorithm.
>>>>>>> Updating
>>>>>>> +RSS hash algorithm.
>>>>>>> +
>>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>>>       ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>>       
>>>>>> This document describes features listed in the 'default.ini', so we
>>>>>> shouldn't have above.
>>>>>>
>>>>>> And I don't think RSS hash algorithm update is a big enough
>>>>>> feature to
>>>>>> list in the feature list, perhaps it can be embedded in the RSS
>>>>>> support
>>>>>> block, what do you think?
>>>>> Yes it is not a bit feature.
>>>>> so put it to RSS hash, right?
>>>>>
>>>> Yes please.
>>>>
>>>> .
>> .

Re: [PATCH v3 1/6] doc: add RSS hash algorithm feature

[lihuisong (C)]

在 2023/11/28 19:52, Ferruh Yigit 写道:
> On 11/28/2023 11:29 AM, lihuisong (C) wrote:
>> 在 2023/11/28 18:09, Ferruh Yigit 写道:
>>> On 11/28/2023 1:21 AM, lihuisong (C) wrote:
>>>> 在 2023/11/27 23:43, Ferruh Yigit 写道:
>>>>> On 11/27/2023 1:12 PM, lihuisong (C) wrote:
>>>>>> 在 2023/11/27 20:19, Ferruh Yigit 写道:
>>>>>>> On 11/25/2023 1:47 AM, Huisong Li wrote:
>>>>>>>> Add hash algorithm feature introduced by 23.11 and fix some RSS
>>>>>>>> features
>>>>>>>> description.
>>>>>>>>
>>>>>>>> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
>>>>>>>>
>>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>>>>> ---
>>>>>>>>      doc/guides/nics/features.rst | 26 ++++++++++++++++++++++----
>>>>>>>>      1 file changed, 22 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/doc/guides/nics/features.rst
>>>>>>>> b/doc/guides/nics/features.rst
>>>>>>>> index 1a1dc16c1e..0d38c5c525 100644
>>>>>>>> --- a/doc/guides/nics/features.rst
>>>>>>>> +++ b/doc/guides/nics/features.rst
>>>>>>>> @@ -277,10 +277,12 @@ RSS hash
>>>>>>>>      Supports RSS hashing on RX.
>>>>>>>>        * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>>>>>> ``RTE_ETH_MQ_RX_RSS_FLAG``.
>>>>>>>> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>>>>>> +* **[uses]     user config**: ``rss_conf.rss_hf``.
>>>>>>>>
>>>>>>> Feature title is "RSS hash", it can be two things,
>>>>>>> 1. "Receive Side Scaling" support
>>>>>>> 2. Provide RSS hash to application
>>>>>>>
>>>>>>> When this document first prepared RSS hash value was always
>>>>>>> provided to
>>>>>>> the application when RSS enabled.
>>>>>>> So intention with this feature was "Receive Side Scaling" support,
>>>>>>> hence
>>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG' added.
>>>>>>>
>>>>>>> Later providing RSS has to the application separated as optimization,
>>>>>>> 'RTE_ETH_RX_OFFLOAD_RSS_HASH' & 'RTE_MBUF_F_RX_RSS_HASH' added for
>>>>>>> this
>>>>>>> support.
>>>>>> What should I do for above two comments？
>>>>>> To tell application how to use it?
>>>>>>
>>>>> Just tried to give some context.
>>>> got it.
>>>>>   
>>>>>>> As the intention of this feature is "Receive Side Scaling"
>>>>>>> support, we
>>>>>>> shouldn't reduce configuration struct to 'rss_conf.rss_hf'.
>>>>>>>
>>>>>>> Instead perhaps can expand to:
>>>>>>> 'rte_eth_conf.rx_adv_conf.rss_conf', 'rte_eth_rss_conf'
>>>>>>     I just pick their common part.😁
>>>>>>
>>>>>> ok, will fix it.
>>>>>>
>>>>>>>>      * **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>>>>      * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>>>>>      * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``,
>>>>>>>> ``mbuf.rss``.
>>>>>>>> +* **[related]  API**: ``rte_eth_dev_configure``,
>>>>>>>> ``rte_eth_dev_rss_hash_update``
>>>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>>>      
>>>>>>> ack
>>>>>>>
>>>>>>>>        .. _nic_features_inner_rss:
>>>>>>>> @@ -288,7 +290,7 @@ Supports RSS hashing on RX.
>>>>>>>>      Inner RSS
>>>>>>>>      ---------
>>>>>>>>      -Supports RX RSS hashing on Inner headers.
>>>>>>>> +Supports RX RSS hashing on Inner headers by rte_flow API.
>>>>>>>>      
>>>>>>> This should be clarified with details below, not sure if it
>>>>>>> required to
>>>>>>> limit description to rte_flow.
>>>>>> But this block like rte_flow_action_rss is from rte_flow.
>>>>>> And ethdev ops doesn't support inner RSS.
>>>>>> So I think it is ok.
>>>>>>
>>>>> Yes it is supported by rte_flow, and '[uses]' information should
>>>>> already
>>>>> clarify it.
>>>> Should we remove the 'rte_flow API' wrods I added in above description?
>>>>
>>> I think it can be removed.
>> The latest version(V5, only three fix doc patch) has been sent out.
>> Do we I need to send V6 for this?😂
>>
> Nope, no new version needed for this, I already acked the one with it,
> but if there will be a new version this can be updated.
ok
> Thanks Huisong for improving the documentation.

Willing to contribute my strength to the community.😁

>
>>>
>>>>>>> And I guess similar confusion exist with the providing hash to user.
>>>>>>> Need to check if rte_flow implementation puts hash to mbuf along with
>>>>>>> doing the RSS, or if it checks 'RTE_ETH_RX_OFFLOAD_RSS_HASH' offload,
>>>>>>> and update below items accordingly.
>>>>>> Do we need to tell user how to use it here?
>>>>>> I feel this document is a little simple and main to list interface for
>>>>>> user.
>>>>>> In addition, it is better that the more detail about RSS should be
>>>>>> presented  in rte_flow features.
>>>>>>
>>>>> No, I am not suggesting to add more detail.
>>>>>
>>>>> My concern is 'RTE_ETH_RX_OFFLOAD_RSS_HASH' information may not be
>>>>> correct, ethdev APIs checks offload flags, but does rte_flow
>>>>> implementation check it?
>>>> As far as I know, It is possibly verified in PMD if have or required.
>>>>> My suggestion is double check that piece of information and fix it if
>>>>> required.
>>>>>
>>>>>
>>>>>>>>      * **[uses]    rte_flow_action_rss**: ``level``.
>>>>>>>>      * **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>>>>>> ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>>>>>>>> @@ -303,9 +305,25 @@ RSS key update
>>>>>>>>      Supports configuration of Receive Side Scaling (RSS) hash
>>>>>>>> computation. Updating
>>>>>>>>      Receive Side Scaling (RSS) hash key.
>>>>>>>>      -* **[implements] eth_dev_ops**: ``rss_hash_update``,
>>>>>>>> ``rss_hash_conf_get``.
>>>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>>>> +* **[uses]     user config**: ``rss_conf.rss_key``,
>>>>>>>> ``rss_conf.rss_key_len``
>>>>>>>>      * **[provides]   rte_eth_dev_info**: ``hash_key_size``.
>>>>>>>> -* **[related]    API**: ``rte_eth_dev_rss_hash_update()``,
>>>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>>>> +  ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>>> +
>>>>>>> ack
>>>>>>>
>>>>>>> There is an inconsistency in the documentation but I think it is
>>>>>>> good to
>>>>>>> use '()' when documenting API, like: 'rte_eth_dev_configure()'
>>>>>> +1 will fix it.
>>>>>>>> +
>>>>>>>> +.. _nic_features_rss_hash_algo_update:
>>>>>>>> +
>>>>>>>> +RSS hash algorithm update
>>>>>>>> +-------------------------
>>>>>>>> +
>>>>>>>> +Supports configuration of Receive Side Scaling (RSS) hash
>>>>>>>> algorithm.
>>>>>>>> Updating
>>>>>>>> +RSS hash algorithm.
>>>>>>>> +
>>>>>>>> +* **[implements] eth_dev_ops**: ``dev_configure``,
>>>>>>>> ``rss_hash_update``, ``rss_hash_conf_get``.
>>>>>>>> +* **[uses]     user config**: ``rss_conf.algorithm``
>>>>>>>> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
>>>>>>>> +* **[related]    API**: ``rte_eth_dev_configure``,
>>>>>>>> ``rte_eth_dev_rss_hash_update()``,
>>>>>>>>        ``rte_eth_dev_rss_hash_conf_get()``.
>>>>>>>>        
>>>>>>> This document describes features listed in the 'default.ini', so we
>>>>>>> shouldn't have above.
>>>>>>>
>>>>>>> And I don't think RSS hash algorithm update is a big enough
>>>>>>> feature to
>>>>>>> list in the feature list, perhaps it can be embedded in the RSS
>>>>>>> support
>>>>>>> block, what do you think?
>>>>>> Yes it is not a bit feature.
>>>>>> so put it to RSS hash, right?
>>>>>>
>>>>> Yes please.
>>>>>
>>>>> .
>>> .
> .

Re: [PATCH v5 1/3] doc: fix the description of RSS related feature

[Thomas Monjalon]

28/11/2023 06:59, Huisong Li:
> This patch fixes the description of RSS feature.
> And the settinf ot hash algorithm is introduced by 23.11, so add it.
> 
> Fixes: 34ff088cc241 ("ethdev: set and query RSS hash algorithm")
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ---
>  * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``RTE_ETH_MQ_RX_RSS_FLAG``.
> -* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
> +* **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf.rss_hf``. ``rss_conf.rss_hf``

No need to keep dev_conf.rx_adv_conf

>  * **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:RTE_ETH_RX_OFFLOAD_RSS_HASH``.
>  * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>  * **[provides] mbuf**: ``mbuf.ol_flags:RTE_MBUF_F_RX_RSS_HASH``, ``mbuf.rss``.
> +* **[related]  API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update``
> +  ``rte_eth_dev_rss_hash_conf_get()``.
> +
> +Support RSS hash algorithm on Rx.
> +
> +* **[implements] eth_dev_ops**: ``dev_configure``, ``rss_hash_update``, ``rss_hash_conf_get``.
> +* **[uses]     user config**: ``rss_conf.algorithm``
> +* **[provides]   rte_eth_dev_info**: ``rss_algo_capa``.
> +* **[related]    API**: ``rte_eth_dev_configure()``, ``rte_eth_dev_rss_hash_update()``,
> +  ``rte_eth_dev_rss_hash_conf_get()``.

Fixed typos, alignment and applied, thanks.

Note: the rest of the series is not applied in 23.11.
They are additions which may require reviews.