Re: [PATCH v2 38/45] bus/vmbus: use rte stdatomic API

[Mattias Rönnblom]

On 2024-03-21 22:34, Long Li wrote:



   static inline void
   vmbus_set_monitor(const struct vmbus_channel *channel, uint32_t

monitor_id)

   {
-   uint32_t *monitor_addr, monitor_mask;
+   RTE_ATOMIC(uint32_t) *monitor_addr, monitor_mask;
unsigned int trigger_index;

trigger_index = monitor_id / HV_MON_TRIG_LEN;
monitor_mask = 1u << (monitor_id % HV_MON_TRIG_LEN);

-   monitor_addr = &channel->monitor_page->trigs[trigger_index].pending;
+   monitor_addr =
+   (uint32_t __rte_atomic
+*)&channel->monitor_page->trigs[trigger_index].pending;


Why is "pending" not RTE_ATOMIC()?


The usage is okay. The value is used to notify the VSP (Hyper-V). It's always 
set (no read) from DPDK.



OK, so my question was not "does it need to be atomic", but rather "why 
isn't it marked RTE_ATOMIC() when it's treated as atomic".


But what you are saying is that it need not be atomic? Just the 
equivalent of WRITE_ONCE()? Or a relaxed atomic store?



Linux kernel driver does the same thing.

Long

[PATCH v2 0/6] support setting lanes

[Dengdui Huang]

At the physical layer, multiple lanes are often used to work together
to achieve higher speeds. So a speeds can be achieved with different
number of lanes. For example, the following solutions can be used to
implement 100G:
1. Combines four 25G lanes
2. Combines two 50G lanes
3. A single 100G lane

It is assumed that two ports are interconnected and the two ports support
the above three solutions. But, we just configured the speed to 100G and
one port uses four 25G lanes by default and the other port uses two 50G lanes
by default, the port cannot be up. In this case, we need to configure the
ports to use the same solutions (for example, uses two 50G lanes) so that
the ports can be up.

This patch set add support setting lanes for ethdev. application can use
this feature to configure lanes to help select the same solutions.

In addition, modify the testpmd and hns3 driver to adapt to it.

change log:
v1->v2:
 - ethdev updata parse link mode info function name
 - hns3 driver modify report FEC capability
 - testpmd add a command to config speed with lanes
 - update UT and some code rectification

Dengdui Huang (6):
  ethdev: support setting lanes
  test: updated UT for setting lanes
  ethdev: add function to parse link mode info
  net/hns3: use parse link mode info function
  net/hns3: support setting lanes
  app/testpmd: support setting lanes

 app/test-pmd/cmdline.c  | 199 +-
 app/test-pmd/config.c   |  78 --
 app/test/test_ethdev_link.c |  18 +-
 doc/guides/rel_notes/release_24_03.rst  |  11 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   9 +
 drivers/net/bnxt/bnxt_ethdev.c  |   3 +-
 drivers/net/hns3/hns3_cmd.h |  15 +-
 drivers/net/hns3/hns3_common.c  |   2 +
 drivers/net/hns3/hns3_ethdev.c  | 206 --
 drivers/net/hns3/hns3_ethdev.h  |   2 +
 lib/ethdev/ethdev_linux_ethtool.c   | 208 +++---
 lib/ethdev/ethdev_private.h |   4 +
 lib/ethdev/ethdev_trace.h   |   4 +-
 lib/ethdev/meson.build  |   2 +
 lib/ethdev/rte_ethdev.c | 284 ++--
 lib/ethdev/rte_ethdev.h | 104 +--
 lib/ethdev/version.map  |   7 +
 17 files changed, 829 insertions(+), 327 deletions(-)

-- 
2.33.0

[PATCH v2 3/6] ethdev: add function to parse link mode info

[Dengdui Huang]

Added function rte_eth_link_mode_parse() to parse
the speed number, lanes and duplex parameters
from the Link speed apabilities bitmap flags.

Signed-off-by: Dengdui Huang 
---
 doc/guides/rel_notes/release_24_03.rst |   3 +
 lib/ethdev/rte_ethdev.c| 199 +
 lib/ethdev/rte_ethdev.h|  29 
 lib/ethdev/version.map |   1 +
 4 files changed, 232 insertions(+)

diff --git a/doc/guides/rel_notes/release_24_03.rst 
b/doc/guides/rel_notes/release_24_03.rst
index 4621689c68..b41b0028b2 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -78,6 +78,9 @@ New Features
 
 * **Support setting lanes for ethdev.**
   * Support setting lanes by extended ``RTE_ETH_LINK_SPEED_*``.
+  * Added function to parse the speed number, lanes and duplex parameters from
+  * the Link speed apabilities bitmap flags:
+  ``rte_eth_link_mode_parse()``
 
 * **Added hash calculation of an encapsulated packet as done by the HW.**
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 6571116fbf..bac7c652be 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1105,6 +1105,205 @@ rte_eth_dev_rx_offload_name(uint64_t offload)
return name;
 }
 
+int
+rte_eth_link_mode_parse(uint32_t link_speed,
+   struct rte_eth_link_mode_info *mode_onfo)
+{
+   const struct {
+   uint32_t speed_capa;
+   struct rte_eth_link_mode_info mode_onfo;
+   } speed_mode_onfo_map[] = {
+   {
+   RTE_ETH_LINK_SPEED_10M_HD,
+   {
+   RTE_ETH_SPEED_NUM_10M,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_HALF_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_10M,
+   {
+   RTE_ETH_SPEED_NUM_10M,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_100M_HD,
+   {
+   RTE_ETH_SPEED_NUM_100M,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_HALF_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_100M,
+   {
+   RTE_ETH_SPEED_NUM_100M,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_1G,
+   {
+   RTE_ETH_SPEED_NUM_1G,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_2_5G,
+   {
+   RTE_ETH_SPEED_NUM_2_5G,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_5G,
+   {
+   RTE_ETH_SPEED_NUM_5G,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_10G,
+   {
+   RTE_ETH_SPEED_NUM_10G,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_20G_2LANES,
+   {
+   RTE_ETH_SPEED_NUM_20G,
+   RTE_ETH_LANES_2,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_ETH_LINK_SPEED_25G,
+   {
+   RTE_ETH_SPEED_NUM_25G,
+   RTE_ETH_LANES_1,
+   RTE_ETH_LINK_FULL_DUPLEX
+   }
+   },
+   {
+   RTE_

[PATCH v2 2/6] test: updated UT for setting lanes

[Dengdui Huang]

The lanes number is added to the ethdev link test case.

Signed-off-by: Dengdui Huang 
---
 app/test/test_ethdev_link.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/app/test/test_ethdev_link.c b/app/test/test_ethdev_link.c
index f063a5fe26..1c31bc0448 100644
--- a/app/test/test_ethdev_link.c
+++ b/app/test/test_ethdev_link.c
@@ -17,14 +17,15 @@ test_link_status_up_default(void)
.link_speed = RTE_ETH_SPEED_NUM_2_5G,
.link_status = RTE_ETH_LINK_UP,
.link_autoneg = RTE_ETH_LINK_AUTONEG,
-   .link_duplex = RTE_ETH_LINK_FULL_DUPLEX
+   .link_duplex = RTE_ETH_LINK_FULL_DUPLEX,
+   .link_lanes = RTE_ETH_LANES_1
};
char text[RTE_ETH_LINK_MAX_STR_LEN];
 
ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
printf("Default link up #1: %s\n", text);
-   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 2.5 Gbps FDX Autoneg",
+   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 2.5 Gbps 1 lanes FDX Autoneg",
text, strlen(text), "Invalid default link status string");
 
link_status.link_duplex = RTE_ETH_LINK_HALF_DUPLEX;
@@ -33,7 +34,7 @@ test_link_status_up_default(void)
ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
printf("Default link up #2: %s\n", text);
RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
-   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 10 Mbps HDX Fixed",
+   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 10 Mbps 1 lanes HDX Fixed",
text, strlen(text), "Invalid default link status "
"string with HDX");
 
@@ -41,7 +42,7 @@ test_link_status_up_default(void)
ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
printf("Default link up #3: %s\n", text);
RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
-   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at Unknown HDX Fixed",
+   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at Unknown 1 lanes HDX Fixed",
text, strlen(text), "Invalid default link status "
"string with HDX");
 
@@ -49,7 +50,7 @@ test_link_status_up_default(void)
ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
printf("Default link up #3: %s\n", text);
RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
-   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at None HDX Fixed",
+   TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at None 1 lanes HDX Fixed",
text, strlen(text), "Invalid default link status "
"string with HDX");
 
@@ -57,6 +58,7 @@ test_link_status_up_default(void)
link_status.link_speed = RTE_ETH_SPEED_NUM_400G;
link_status.link_duplex = RTE_ETH_LINK_HALF_DUPLEX;
link_status.link_autoneg = RTE_ETH_LINK_AUTONEG;
+   link_status.link_lanes = RTE_ETH_LANES_4;
ret = rte_eth_link_to_str(text, sizeof(text), &link_status);
printf("Default link up #4:len = %d, %s\n", ret, text);
RTE_TEST_ASSERT(ret < RTE_ETH_LINK_MAX_STR_LEN,
@@ -72,7 +74,8 @@ test_link_status_down_default(void)
.link_speed = RTE_ETH_SPEED_NUM_2_5G,
.link_status = RTE_ETH_LINK_DOWN,
.link_autoneg = RTE_ETH_LINK_AUTONEG,
-   .link_duplex = RTE_ETH_LINK_FULL_DUPLEX
+   .link_duplex = RTE_ETH_LINK_FULL_DUPLEX,
+   .link_lanes = RTE_ETH_LANES_1
};
char text[RTE_ETH_LINK_MAX_STR_LEN];
 
@@ -92,7 +95,8 @@ test_link_status_invalid(void)
.link_speed = 5,
.link_status = RTE_ETH_LINK_UP,
.link_autoneg = RTE_ETH_LINK_AUTONEG,
-   .link_duplex = RTE_ETH_LINK_FULL_DUPLEX
+   .link_duplex = RTE_ETH_LINK_FULL_DUPLEX,
+   .link_lanes = RTE_ETH_LANES_UNKNOWN
};
char text[RTE_ETH_LINK_MAX_STR_LEN];
 
-- 
2.33.0

[PATCH v2 4/6] net/hns3: use parse link mode info function

[Dengdui Huang]

This patch use the framework public function to
replace the driver's private function.

Signed-off-by: Dengdui Huang 
---
 drivers/net/hns3/hns3_ethdev.c | 51 +++---
 1 file changed, 10 insertions(+), 41 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index ecd3b2ef64..1b380ac75f 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -4810,45 +4810,6 @@ hns3_check_port_speed(struct hns3_hw *hw, uint32_t 
link_speeds)
return 0;
 }
 
-static uint32_t
-hns3_get_link_speed(uint32_t link_speeds)
-{
-   uint32_t speed = RTE_ETH_SPEED_NUM_NONE;
-
-   if (link_speeds & RTE_ETH_LINK_SPEED_10M ||
-   link_speeds & RTE_ETH_LINK_SPEED_10M_HD)
-   speed = RTE_ETH_SPEED_NUM_10M;
-   if (link_speeds & RTE_ETH_LINK_SPEED_100M ||
-   link_speeds & RTE_ETH_LINK_SPEED_100M_HD)
-   speed = RTE_ETH_SPEED_NUM_100M;
-   if (link_speeds & RTE_ETH_LINK_SPEED_1G)
-   speed = RTE_ETH_SPEED_NUM_1G;
-   if (link_speeds & RTE_ETH_LINK_SPEED_10G)
-   speed = RTE_ETH_SPEED_NUM_10G;
-   if (link_speeds & RTE_ETH_LINK_SPEED_25G)
-   speed = RTE_ETH_SPEED_NUM_25G;
-   if (link_speeds & RTE_ETH_LINK_SPEED_40G)
-   speed = RTE_ETH_SPEED_NUM_40G;
-   if (link_speeds & RTE_ETH_LINK_SPEED_50G)
-   speed = RTE_ETH_SPEED_NUM_50G;
-   if (link_speeds & RTE_ETH_LINK_SPEED_100G)
-   speed = RTE_ETH_SPEED_NUM_100G;
-   if (link_speeds & RTE_ETH_LINK_SPEED_200G)
-   speed = RTE_ETH_SPEED_NUM_200G;
-
-   return speed;
-}
-
-static uint8_t
-hns3_get_link_duplex(uint32_t link_speeds)
-{
-   if ((link_speeds & RTE_ETH_LINK_SPEED_10M_HD) ||
-   (link_speeds & RTE_ETH_LINK_SPEED_100M_HD))
-   return RTE_ETH_LINK_HALF_DUPLEX;
-   else
-   return RTE_ETH_LINK_FULL_DUPLEX;
-}
-
 static int
 hns3_set_copper_port_link_speed(struct hns3_hw *hw,
struct hns3_set_link_speed_cfg *cfg)
@@ -4980,14 +4941,22 @@ static int
 hns3_apply_link_speed(struct hns3_hw *hw)
 {
struct rte_eth_conf *conf = &hw->data->dev_conf;
+   struct rte_eth_link_mode_info mode_info = {0};
struct hns3_set_link_speed_cfg cfg;
+   int ret;
 
memset(&cfg, 0, sizeof(struct hns3_set_link_speed_cfg));
cfg.autoneg = (conf->link_speeds == RTE_ETH_LINK_SPEED_AUTONEG) ?
RTE_ETH_LINK_AUTONEG : RTE_ETH_LINK_FIXED;
if (cfg.autoneg != RTE_ETH_LINK_AUTONEG) {
-   cfg.speed = hns3_get_link_speed(conf->link_speeds);
-   cfg.duplex = hns3_get_link_duplex(conf->link_speeds);
+   ret = rte_eth_link_mode_parse(conf->link_speeds, &mode_info);
+   if (ret) {
+   hns3_err(hw, "failed to parse link mode, ret = %d", 
ret);
+   return ret;
+   }
+   cfg.speed = mode_onfo.speed_num;
+   cfg.speed = mode_info.speed_num;
+   cfg.duplex = mode_info.duplex;
}
 
return hns3_set_port_link_speed(hw, &cfg);
-- 
2.33.0

[PATCH v2 1/6] ethdev: support setting lanes

[Dengdui Huang]

Some speeds can be achieved with different number of lanes. For example,
100Gbps can be achieved using two lanes of 50Gbps or four lanes of 25Gbps.
When use different lanes, the port cannot be up. This patch add support
setting lanes and report lanes.

In addition, add a device capability RTE_ETH_DEV_CAPA_SETTING_LANES
When the device does not support it, if a speed supports different
numbers of lanes, the application does not knowe which the lane number
are used by the device.

Signed-off-by: Dengdui Huang 
---
 doc/guides/rel_notes/release_24_03.rst |   6 +
 drivers/net/bnxt/bnxt_ethdev.c |   3 +-
 drivers/net/hns3/hns3_ethdev.c |   1 +
 lib/ethdev/ethdev_linux_ethtool.c  | 208 -
 lib/ethdev/ethdev_private.h|   4 +
 lib/ethdev/ethdev_trace.h  |   4 +-
 lib/ethdev/meson.build |   2 +
 lib/ethdev/rte_ethdev.c|  85 +++---
 lib/ethdev/rte_ethdev.h|  75 ++---
 lib/ethdev/version.map |   6 +
 10 files changed, 250 insertions(+), 144 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_03.rst 
b/doc/guides/rel_notes/release_24_03.rst
index 7bd9ceab27..4621689c68 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -76,6 +76,9 @@ New Features
   * Added a fath path function ``rte_eth_tx_queue_count``
 to get the number of used descriptors of a Tx queue.
 
+* **Support setting lanes for ethdev.**
+  * Support setting lanes by extended ``RTE_ETH_LINK_SPEED_*``.
+
 * **Added hash calculation of an encapsulated packet as done by the HW.**
 
   Added function to calculate hash when doing tunnel encapsulation:
@@ -254,6 +257,9 @@ ABI Changes
 
 * No ABI change that would break compatibility with 23.11.
 
+* ethdev: Convert a numerical speed to a bitmap flag with lanes:
+  The function ``rte_eth_speed_bitflag`` add lanes parameters.
+
 
 Known Issues
 
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ba31ae9286..e881a7f3cc 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -711,7 +711,8 @@ static int bnxt_update_phy_setting(struct bnxt *bp)
}
 
/* convert to speedbit flag */
-   curr_speed_bit = rte_eth_speed_bitflag((uint32_t)link->link_speed, 1);
+   curr_speed_bit = rte_eth_speed_bitflag((uint32_t)link->link_speed,
+  RTE_ETH_LANES_UNKNOWN, 1);
 
/*
 * Device is not obliged link down in certain scenarios, even
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index b10d1216d2..ecd3b2ef64 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5969,6 +5969,7 @@ hns3_get_speed_fec_capa(struct rte_eth_fec_capa 
*speed_fec_capa,
for (i = 0; i < RTE_DIM(speed_fec_capa_tbl); i++) {
speed_bit =
rte_eth_speed_bitflag(speed_fec_capa_tbl[i].speed,
+ RTE_ETH_LANES_UNKNOWN,
  RTE_ETH_LINK_FULL_DUPLEX);
if ((speed_capa & speed_bit) == 0)
continue;
diff --git a/lib/ethdev/ethdev_linux_ethtool.c 
b/lib/ethdev/ethdev_linux_ethtool.c
index e792204b01..6412845161 100644
--- a/lib/ethdev/ethdev_linux_ethtool.c
+++ b/lib/ethdev/ethdev_linux_ethtool.c
@@ -7,6 +7,10 @@
 #include "rte_ethdev.h"
 #include "ethdev_linux_ethtool.h"
 
+#define RTE_ETH_LINK_MODES_INDEX_SPEED 0
+#define RTE_ETH_LINK_MODES_INDEX_DUPLEX1
+#define RTE_ETH_LINK_MODES_INDEX_LANES 2
+
 /* Link modes sorted with index as defined in ethtool.
  * Values are speed in Mbps with LSB indicating duplex.
  *
@@ -15,123 +19,119 @@
  * and allows to compile with new bits included even on an old kernel.
  *
  * The array below is built from bit definitions with this shell command:
- *   sed -rn 's;.*(ETHTOOL_LINK_MODE_)([0-9]+)([0-9a-zA-Z_]*).*= *([0-9]*).*;'\
- *   '[\4] = \2, /\* \1\2\3 *\/;p' /usr/include/linux/ethtool.h |
- *   awk '/_Half_/{$3=$3+1","}1'
+ *   sed -rn 
's;.*(ETHTOOL_LINK_MODE_)([0-9]+)([a-zA-Z]+)([0-9_]+)([0-9a-zA-Z_]*)
+ *   .*= *([0-9]*).*;'\ '[\6] = {\2, 1, \4}, /\* \1\2\3\4\5 *\/;p'
+ */usr/include/linux/ethtool.h | awk '/_Half_/{$4=0","}1' |
+ *awk '/, _}/{$5=1"},"}1' | awk '{sub(/_}/,"\}");}1'
  */
-static const uint32_t link_modes[] = {
- [0] =  11, /* ETHTOOL_LINK_MODE_10baseT_Half_BIT */
- [1] =  10, /* ETHTOOL_LINK_MODE_10baseT_Full_BIT */
- [2] = 101, /* ETHTOOL_LINK_MODE_100baseT_Half_BIT */
- [3] = 100, /* ETHTOOL_LINK_MODE_100baseT_Full_BIT */
- [4] =1001, /* ETHTOOL_LINK_MODE_1000baseT_Half_BIT */
- [5] =1000, /* ETHTOOL_LINK_MODE_1000baseT_Full_BIT */
-[12] =   1, /* ETHTOOL_LINK_MODE_1baseT_Full_BIT */
-[15] =2500, /* ETHTOOL_LINK_MODE_25

[PATCH v2 6/6] app/testpmd: support setting lanes

[Dengdui Huang]

Add a command to config speed with lanes and
added print the lane number for show info command.

Signed-off-by: Dengdui Huang 
---
 app/test-pmd/cmdline.c  | 199 +---
 app/test-pmd/config.c   |  78 +---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   9 +
 3 files changed, 194 insertions(+), 92 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f521a1fe9e..413bf735a2 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -696,6 +696,11 @@ static void cmd_help_long_parsed(void *parsed_result,
" duplex (half|full|auto)\n"
"Set speed and duplex for all ports or port_id\n\n"
 
+   "port config (port_id|all)"
+   " speed 
(10|100|1000|2500|5000|1|25000|4|5|10|20|40|auto)"
+   " lanes (lane_num) duplex (half|full|auto)\n"
+   "Set speed and duplex for all ports or port_id\n\n"
+
"port config (port_id|all) loopback (mode)\n"
"Set loopback mode for all ports or port_id\n\n"
 
@@ -1357,14 +1362,19 @@ struct cmd_config_speed_all {
cmdline_fixed_string_t all;
cmdline_fixed_string_t item1;
cmdline_fixed_string_t item2;
+   cmdline_fixed_string_t item3;
cmdline_fixed_string_t value1;
-   cmdline_fixed_string_t value2;
+   uint8_t value2;
+   cmdline_fixed_string_t value3;
 };
 
 static int
-parse_and_check_speed_duplex(char *speedstr, char *duplexstr, uint32_t *speed)
+parse_and_check_speed_duplex(char *speedstr, uint8_t lanes, char *duplexstr,
+uint32_t *speed)
 {
 
+   uint32_t speed_num;
+   char *endptr;
int duplex;
 
if (!strcmp(duplexstr, "half")) {
@@ -1378,47 +1388,22 @@ parse_and_check_speed_duplex(char *speedstr, char 
*duplexstr, uint32_t *speed)
return -1;
}
 
-   if (!strcmp(speedstr, "10")) {
-   *speed = (duplex == RTE_ETH_LINK_HALF_DUPLEX) ?
-   RTE_ETH_LINK_SPEED_10M_HD : 
RTE_ETH_LINK_SPEED_10M;
-   } else if (!strcmp(speedstr, "100")) {
-   *speed = (duplex == RTE_ETH_LINK_HALF_DUPLEX) ?
-   RTE_ETH_LINK_SPEED_100M_HD : 
RTE_ETH_LINK_SPEED_100M;
-   } else {
-   if (duplex != RTE_ETH_LINK_FULL_DUPLEX) {
-   fprintf(stderr, "Invalid speed/duplex parameters\n");
-   return -1;
-   }
-   if (!strcmp(speedstr, "1000")) {
-   *speed = RTE_ETH_LINK_SPEED_1G;
-   } else if (!strcmp(speedstr, "2500")) {
-   *speed = RTE_ETH_LINK_SPEED_2_5G;
-   } else if (!strcmp(speedstr, "5000")) {
-   *speed = RTE_ETH_LINK_SPEED_5G;
-   } else if (!strcmp(speedstr, "1")) {
-   *speed = RTE_ETH_LINK_SPEED_10G;
-   } else if (!strcmp(speedstr, "25000")) {
-   *speed = RTE_ETH_LINK_SPEED_25G;
-   } else if (!strcmp(speedstr, "4")) {
-   *speed = RTE_ETH_LINK_SPEED_40G;
-   } else if (!strcmp(speedstr, "5")) {
-   *speed = RTE_ETH_LINK_SPEED_50G;
-   } else if (!strcmp(speedstr, "10")) {
-   *speed = RTE_ETH_LINK_SPEED_100G;
-   } else if (!strcmp(speedstr, "20")) {
-   *speed = RTE_ETH_LINK_SPEED_200G;
-   } else if (!strcmp(speedstr, "40")) {
-   *speed = RTE_ETH_LINK_SPEED_400G;
-   } else if (!strcmp(speedstr, "auto")) {
-   *speed = RTE_ETH_LINK_SPEED_AUTONEG;
-   } else {
-   fprintf(stderr, "Unknown speed parameter\n");
-   return -1;
-   }
+   if (!strcmp(speedstr, "auto")) {
+   *speed = RTE_ETH_LINK_SPEED_AUTONEG;
+   return 0;
}
 
-   if (*speed != RTE_ETH_LINK_SPEED_AUTONEG)
-   *speed |= RTE_ETH_LINK_SPEED_FIXED;
+   speed_num = strtol(speedstr, &endptr, 10);
+   if (*endptr != '\0') {
+   fprintf(stderr, "Unknown speed parameter\n");
+   return -1;
+   }
+
+   *speed = rte_eth_speed_bitflag(speed_num, lanes, duplex);
+   if (*speed == 0) {
+   fprintf(stderr, "param error\n");
+   return -1;
+   }
 
return 0;
 }
@@ -1426,22 +1411,37 @@ parse_and_check_speed_duplex(char *speedstr, char 
*duplexstr, uint32_t *speed)
 static void
 cmd_config_speed_all_parsed(void *parsed_result,
__rte_unused struct cmdline *cl,
-   __rte_unused void *data)
+   void *data)
 {
struct cmd_config_speed_all *res = parsed_

[PATCH v2 5/6] net/hns3: support setting lanes

[Dengdui Huang]

Some speeds can be achieved with different number of lanes. For example,
100Gbps can be achieved using two lanes of 50Gbps or four lanes of 25Gbps.
When use different lanes, the port cannot be up. This patch add support
for setting lanes and report lanes.

In addition, when reporting FEC capability, it is incorrect to calculate
speed_num from the speed function when one speed supports a different
number of lanes. This patch modifies it together.

Signed-off-by: Dengdui Huang 
---
 doc/guides/rel_notes/release_24_03.rst |   2 +
 drivers/net/hns3/hns3_cmd.h|  15 ++-
 drivers/net/hns3/hns3_common.c |   2 +
 drivers/net/hns3/hns3_ethdev.c | 158 ++---
 drivers/net/hns3/hns3_ethdev.h |   2 +
 5 files changed, 134 insertions(+), 45 deletions(-)

diff --git a/doc/guides/rel_notes/release_24_03.rst 
b/doc/guides/rel_notes/release_24_03.rst
index b41b0028b2..c9b8740323 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -193,6 +193,8 @@ New Features
   * Added power-saving during polling within the ``rte_event_dequeue_burst()`` 
API.
   * Added support for DMA adapter.
 
+* **Added setting lanes for hns3 PF driver.**
+  * This feature add support for setting lanes and report lanes.
 
 Removed Items
 -
diff --git a/drivers/net/hns3/hns3_cmd.h b/drivers/net/hns3/hns3_cmd.h
index 79a8c1edad..31ff7b35d8 100644
--- a/drivers/net/hns3/hns3_cmd.h
+++ b/drivers/net/hns3/hns3_cmd.h
@@ -753,7 +753,9 @@ struct hns3_config_mac_mode_cmd {
 struct hns3_config_mac_speed_dup_cmd {
uint8_t speed_dup;
uint8_t mac_change_fec_en;
-   uint8_t rsv[22];
+   uint8_t rsv[4];
+   uint8_t lanes;
+   uint8_t rsv1[17];
 };
 
 #define HNS3_TQP_ENABLE_B  0
@@ -789,12 +791,15 @@ struct hns3_sfp_type {
 #define HNS3_FIBER_LINK_SPEED_1G_BIT   BIT(0)
 #define HNS3_FIBER_LINK_SPEED_10G_BIT  BIT(1)
 #define HNS3_FIBER_LINK_SPEED_25G_BIT  BIT(2)
-#define HNS3_FIBER_LINK_SPEED_50G_BIT  BIT(3)
-#define HNS3_FIBER_LINK_SPEED_100G_BIT BIT(4)
+#define HNS3_FIBER_LINK_SPEED_50G_R2_BIT   BIT(3)
+#define HNS3_FIBER_LINK_SPEED_100G_R4_BIT  BIT(4)
 #define HNS3_FIBER_LINK_SPEED_40G_BIT  BIT(5)
 #define HNS3_FIBER_LINK_SPEED_100M_BIT BIT(6)
 #define HNS3_FIBER_LINK_SPEED_10M_BIT  BIT(7)
-#define HNS3_FIBER_LINK_SPEED_200G_BIT BIT(8)
+#define HNS3_FIBER_LINK_SPEED_200G_EXT_BIT BIT(8)
+#define HNS3_FIBER_LINK_SPEED_50G_R1_BIT   BIT(9)
+#define HNS3_FIBER_LINK_SPEED_100G_R2_BIT  BIT(10)
+#define HNS3_FIBER_LINK_SPEED_200G_R4_BIT  BIT(11)
 
 #define HNS3_FIBER_FEC_AUTO_BITBIT(0)
 #define HNS3_FIBER_FEC_BASER_BIT   BIT(1)
@@ -823,7 +828,7 @@ struct hns3_sfp_info_cmd {
uint32_t supported_speed; /* speed supported by current media */
uint32_t module_type;
uint8_t fec_ability; /* supported fec modes, see HNS3_FIBER_FEC_XXX_BIT 
*/
-   uint8_t rsv0;
+   uint8_t lanes;
uint8_t pause_status;
uint8_t rsv1[5];
 };
diff --git a/drivers/net/hns3/hns3_common.c b/drivers/net/hns3/hns3_common.c
index 28c26b049c..b6db012993 100644
--- a/drivers/net/hns3/hns3_common.c
+++ b/drivers/net/hns3/hns3_common.c
@@ -93,6 +93,8 @@ hns3_dev_infos_get(struct rte_eth_dev *eth_dev, struct 
rte_eth_dev_info *info)
 
info->dev_capa = RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP |
 RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP;
+   if (!hns->is_vf)
+   info->dev_capa |= RTE_ETH_DEV_CAPA_SETTING_LANES;
if (hns3_dev_get_support(hw, INDEP_TXRX))
info->dev_capa |= RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP |
  RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP;
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 1b380ac75f..7f36c193a6 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -63,12 +63,12 @@ struct hns3_intr_state {
uint32_t hw_err_state;
 };
 
-#define HNS3_SPEEDS_SUPP_FEC (RTE_ETH_LINK_SPEED_10G | \
- RTE_ETH_LINK_SPEED_25G | \
- RTE_ETH_LINK_SPEED_40G | \
- RTE_ETH_LINK_SPEED_50G | \
- RTE_ETH_LINK_SPEED_100G | \
- RTE_ETH_LINK_SPEED_200G)
+#define HNS3_SPEED_NUM_10G_BIT RTE_BIT32(1)
+#define HNS3_SPEED_NUM_25G_BIT RTE_BIT32(2)
+#define HNS3_SPEED_NUM_40G_BIT RTE_BIT32(3)
+#define HNS3_SPEED_NUM_50G_BIT RTE_BIT32(4)
+#define HNS3_SPEED_NUM_100G_BITRTE_BIT32(5)
+#define HNS3_SPEED_NUM_200G_BITRTE_BIT32(6)
 
 static const struct rte_eth_fec_capa speed_fec_capa_tbl[] = {
{ RTE_ETH_SPEED_NUM_10G, RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC) |
@@ -2234,13 +2234,17 @@ hns3_get_firber_port_speed_capa(uint32_t 
supported_speed)
if (supported_speed & HNS3_FIBER_LINK_SPEED_25G_BIT)
 

[PATCH v2] graph: fix head move when graph walk in mcore dispatch

[Jingjing Wu]

Head move happens before the core id check, which will cause the last
source node be executed even core id is not correct. This patch changes
head check to less than 1 instead of 0 to fix this issue.

Fixes: 35dfd9b9fd85 ("graph: introduce graph walk by cross-core dispatch")

Signed-off-by: Jingjing Wu 
---
 lib/graph/rte_graph_model_mcore_dispatch.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/graph/rte_graph_model_mcore_dispatch.h 
b/lib/graph/rte_graph_model_mcore_dispatch.h
index 75ec388cad..1cc75b7ac4 100644
--- a/lib/graph/rte_graph_model_mcore_dispatch.h
+++ b/lib/graph/rte_graph_model_mcore_dispatch.h
@@ -100,9 +100,8 @@ rte_graph_walk_mcore_dispatch(struct rte_graph *graph)
node = (struct rte_node *)RTE_PTR_ADD(graph, 
cir_start[(int32_t)head++]);
 
/* skip the src nodes which not bind with current worker */
-   if ((int32_t)head < 0 && node->dispatch.lcore_id != 
graph->dispatch.lcore_id)
+   if ((int32_t)head < 1 && node->dispatch.lcore_id != 
graph->dispatch.lcore_id)
continue;
-
/* Schedule the node until all task/objs are done */
if (node->dispatch.lcore_id != RTE_MAX_LCORE &&
graph->dispatch.lcore_id != node->dispatch.lcore_id &&
-- 
2.34.1

RE: [EXTERNAL] [PATCH] graph: enhance export to graphviz

[Kiran Kumar Kokkilagadda]

> -Original Message-
> From: Robin Jarry 
> Sent: Wednesday, March 20, 2024 10:41 PM
> To: dev@dpdk.org; Jerin Jacob ; Kiran Kumar
> Kokkilagadda ; Nithin Kumar Dabilpuram
> ; Zhirun Yan 
> Subject: [EXTERNAL] [PATCH] graph: enhance export to graphviz
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> --
> * Quote graph name to avoid parsing errors when it contains a dash.
> * Use fixed margin and smaller font for a more compact layout.
> * Use sans-serif font, the default is times new roman which is not the
>   best choice for a packet processing generated graph.
> * Force bold blue border and arrows for source nodes.
> * Force dark orange borders for sink nodes (nodes without next nodes).
> 
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__f.jarry.cc_rte-
> 2Dgraph-
> 2Ddot_before.svg&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=owEKckYY4FT
> mil1Z6oBURwkTThyuRbLAY9LdfiaT6HA&m=pyYDQxlqMplh30E36wiXH6LLReml
> Q19SXfdBaWFn9w76vzq1CZdH-
> MC9xnFF1F73&s=j08NkMjMiHpSAVfucSwMN7c769_JbCqmTlEv-O0LyeQ&e=
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__f.jarry.cc_rte-
> 2Dgraph-
> 2Ddot_after.svg&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=owEKckYY4FTmi
> l1Z6oBURwkTThyuRbLAY9LdfiaT6HA&m=pyYDQxlqMplh30E36wiXH6LLRemlQ1
> 9SXfdBaWFn9w76vzq1CZdH-MC9xnFF1F73&s=dQTT3aUgxo-
> oZVMsw0KJR1UT2Gn3oi9r50TAJdIgJLs&e=
> Signed-off-by: Robin Jarry 

Acked-by: Kiran Kumar Kokkilagadda 

> ---
>  lib/graph/graph.c | 32 +---
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/graph/graph.c b/lib/graph/graph.c index
> 26f0968a978d..147bc6c685c5 100644
> --- a/lib/graph/graph.c
> +++ b/lib/graph/graph.c
> @@ -674,25 +674,43 @@ __rte_node_stream_alloc_size(struct rte_graph
> *graph, struct rte_node *node,  static int  graph_to_dot(FILE *f, struct graph
> *graph)  {
> - const char *src_edge_color = " [color=blue]\n";
> - const char *edge_color = "\n";
>   struct graph_node *graph_node;
>   char *node_name;
>   rte_edge_t i;
>   int rc;
> 
> - rc = fprintf(f, "Digraph %s {\n\trankdir=LR;\n", graph->name);
> + rc = fprintf(f, "digraph \"%s\" {\n\trankdir=LR;\n", graph->name);
> + if (rc < 0)
> + goto end;
> +
> + rc = fprintf(f, "\tnode [margin=0.02 fontsize=11 fontname=sans];\n");
>   if (rc < 0)
>   goto end;
> 
>   STAILQ_FOREACH(graph_node, &graph->node_list, next) {
> + const char *attrs = "";
>   node_name = graph_node->node->name;
> +
> + rc = fprintf(f, "\t\"%s\"", node_name);
> + if (rc < 0)
> + goto end;
> + if (graph_node->node->flags & RTE_NODE_SOURCE_F) {
> + attrs = " [color=blue style=bold]";
> + rc = fprintf(f, "%s", attrs);
> + if (rc < 0)
> + goto end;
> + } else if (graph_node->node->nb_edges == 0) {
> + rc = fprintf(f, " [color=darkorange]");
> + if (rc < 0)
> + goto end;
> + }
> + rc = fprintf(f, ";\n");
> + if (rc < 0)
> + goto end;
>   for (i = 0; i < graph_node->node->nb_edges; i++) {
> - rc = fprintf(f, "\t\"%s\"->\"%s\"%s", node_name,
> + rc = fprintf(f, "\t\"%s\" -> \"%s\"%s;\n", node_name,
>graph_node->adjacency_list[i]->node-
> >name,
> -  graph_node->node->flags &
> RTE_NODE_SOURCE_F
> -  ? src_edge_color
> -  : edge_color);
> +  attrs);
>   if (rc < 0)
>   goto end;
>   }
> --
> 2.44.0

Intel x553 hot swap of SFP 10G/1G causes link down and unknown speed

[Prasanna Panchamukhi]

Hot swap of  SFPs 10G/1G current is not support on Intel x553 controller
with dpdk 21.11.3


CPU  Intel(R) Atom(TM) CPU C3558R @ 2.40GHz

x553 ports bond to dpdk inserted with 10G and 1G SFPs

# lspci


07:00.0 *Ether*net controller: Intel Corporation *Ether*net Connection X553
10 GbE SFP+ (rev 11)

07:00.1 *Ether*net controller: Intel Corporation *Ether*net Connection X553
10 GbE SFP+ (rev 11)


# /usr/share/dpdk/tools/dpdk-devbind.py --status


Network devices using DPDK-compatible driver



0

:07:00.0 'Ethernet Connection X553 10 GbE SFP+ 15c4' drv=vfio-pci
unused=

:07:00.1 'Ethernet Connection X553 10 GbE SFP+ 15c4' drv=vfio-pci

unused=



 et7   Driver PMDPort  HWaddr cc:1a:a3:ff:ea:e9 MTU 9236

Speed 1,000Mbps Link UP  Duplex FULL Autoneg ON

RX Queues: 1   RX Queue size: 4096

TX Queues: 1   TX Queue Size: 1024

Inc/RX packets: 22,327   bytes: 23,925,237

dropped: 0

Out/TX packets: 22,352   bytes: 23,924,291

dropped: 0


et6   Driver PMDPort  HWaddr cc:1a:a3:ff:ea:e9 MTU 9236

Speed 10,000Mbps Link UP  Duplex FULL Autoneg ON

RX Queues: 1   RX Queue size: 4096

TX Queues: 1   TX Queue Size: 1024

Inc/RX packets: 2,798,567,431   bytes: 4,186,656,722,403

dropped: 0

Out/TX packets: 523,462,096bytes: 783,087,633,300

dropped: 28,382



Swap the SFPs with the cable between port6 and port 7.



et7   Driver PMDPort  HWaddr cc:1a:a3:ff:ea:e9 MTU 9236

Speed UNKNOWN  Link DOWN Duplex HALF Autoneg ON

RX Queues: 1   RX Queue size: 4096

TX Queues: 1   TX Queue Size: 1024

Inc/RX packets: 22,327   bytes: 23,925,237

dropped: 0

Out/TX packets: 22,362   bytes: 23,924,891

dropped: 0

et7   Driver PMDPort  HWaddr cc:1a:a3:ff:ea:e9 MTU 9236

Speed UNKNOWN  Link DOWN Duplex HALF Autoneg ON

RX Queues: 1   RX Queue size: 4096

TX Queues: 1   TX Queue Size: 1024

Inc/RX packets: 22,327   bytes: 23,925,237

dropped: 0

Out/TX packets: 22,362   bytes: 23,924,891

dropped: 0



Same experiment with Linux ixgbe kernel driver and on hotswap on SFPs both
link and speed were correctly reported. Logs below:




I saw some patches in the archives posted in 2021 which did not make it to
the upstream release that support these features by Stephen.


https://mails.dpdk.org/archives/dev/2021-December/230965.html


Is there a plan to support this feature dpdk like linux kernel ixgbe driver
?


Thanks,

Prasanna

Re: [PATCH 0/2] introduce PM QoS interface

[lihuisong (C)]

+Tyler, +Alan, +Wei, +Long for asking this similar feature on Windows.

在 2024/3/21 21:30, Morten Brørup 写道:

From: lihuisong (C) [mailto:lihuis...@huawei.com]
Sent: Thursday, 21 March 2024 04.04

Hi Moren,

Thanks for your revew.

在 2024/3/20 22:05, Morten Brørup 写道:

From: Huisong Li [mailto:lihuis...@huawei.com]
Sent: Wednesday, 20 March 2024 11.55

The system-wide CPU latency QoS limit has a positive impact on the idle
state selection in cpuidle governor.

Linux creates a cpu_dma_latency device under '/dev' directory to obtain the
CPU latency QoS limit on system and send the QoS request for userspace.
Please see the PM QoS framework in the following link:
https://docs.kernel.org/power/pm_qos_interface.html?highlight=qos
This feature is supported by kernel-v2.6.25.

The deeper the idle state, the lower the power consumption, but the longer
the resume time. Some service are delay sensitive and very except the low
resume time, like interrupt packet receiving mode.

So this series introduce PM QoS interface.

This looks like a 1:1 wrapper for a Linux kernel feature.

right

Does Windows or BSD offer something similar?

How do we know Windows or BSD support this similar feature?

Ask Windows experts or research using Google.

I download freebsd source code, I didn't find this similar feature.
They don't even support cpuidle feature(this QoS feature affects cpuilde.).
I don't find any useful about this on Windows from google.


@Tyler, @Alan, @Wei and @Long

Do you know windows support that userspace read and send CPU latency 
which has an impact on deep level of CPU idle?



The DPDK power lib just work on Linux according to the meson.build under
lib/power.
If they support this features, they can open it.

The DPDK power lib currently only works on Linux, yes.
But its API should still be designed to be platform agnostic, so the functions 
can be implemented on other platforms in the future.

DPDK is on track to work across multiple platforms, including Windows.
We must always consider other platforms, and not design DPDK APIs as if they 
are for Linux/BSD only.

totally understand you.



Furthermore, any high-res timing should use nanoseconds, not microseconds or

milliseconds.

I realize that the Linux kernel only uses microseconds for these APIs, but

the DPDK API should use nanoseconds.
Nanoseconds is more precise, it's good.
But DPDK API how use nanoseconds as you said the the Linux kernel only
uses microseconds for these APIs.
Kernel interface just know an integer value with microseconds unit.

One solution is to expose nanoseconds in the DPDK API, and in the Linux 
specific implementation convert from/to microseconds.
If so, we have to modify the implementation interface on Linux. This 
change the input/output unit about the interface.

And DPDK also has to do this based on kernel version. It is not good.
The cpuidle governor select which idle state based on the worst-case 
latency of idle state.
These the worst-case latency of Cstate reported by ACPI table is in 
microseconds as the section 8.4.1.1. _CST (C States) and 8.4.3.3. _LPI 
(Low Power Idle States) in ACPI spec [1].

So it is probably not meaning to change this interface implementation.

For the case need PM QoS in DPDK, I think, it is better to set cpu 
latency to zero to prevent service thread from the deeper the idle state.

You might also want to add a note to the in-line documentation of the relevant 
functions that the Linux implementation only uses microsecond resolution.

[1] 
https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html

[PATCH] net/mlx5/hws: fix invalid memory access in decapl3

[Gregory Etelson]

From: Alex Vesker 

In case decapL3 action is created we would access header
data even in case the SHARED flag is not set, this would
lead to an invalid memory access.

Fixes: 3a6c50215c07 ("net/mlx5/hws: support multi-pattern")

Cc: sta...@dpdk.org

Signed-off-by: Alex Vesker 
---
 drivers/net/mlx5/hws/mlx5dr_action.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/hws/mlx5dr_action.c 
b/drivers/net/mlx5/hws/mlx5dr_action.c
index 084d4d606e..562fb5cbb4 100644
--- a/drivers/net/mlx5/hws/mlx5dr_action.c
+++ b/drivers/net/mlx5/hws/mlx5dr_action.c
@@ -1775,7 +1775,9 @@ mlx5dr_action_handle_tunnel_l3_to_l2(struct mlx5dr_action 
*action,
 
/* Create a full modify header action list in case shared */
mlx5dr_action_prepare_decap_l3_actions(hdrs->sz, mh_data, 
&num_of_actions);
-   mlx5dr_action_prepare_decap_l3_data(hdrs->data, mh_data, 
num_of_actions);
+
+   if (action->flags & MLX5DR_ACTION_FLAG_SHARED)
+   mlx5dr_action_prepare_decap_l3_data(hdrs->data, mh_data, 
num_of_actions);
 
/* All DecapL3 cases require the same max arg size */
arg_obj = mlx5dr_arg_create_modify_header_arg(ctx,
-- 
2.39.2

[DPDK/ethdev Bug 1406] "net/ice" and "net/i40e" have not implemented outer UDP checksum offload, but the capability flag has been set.

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=1406

Bug ID: 1406
   Summary: "net/ice" and "net/i40e" have not implemented outer
UDP checksum offload, but the capability flag has been
set.
   Product: DPDK
   Version: 23.11
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: ethdev
  Assignee: dev@dpdk.org
  Reporter: junwan...@cestc.cn
  Target Milestone: ---

As shown in [1],a geneve packet with TSO request is transmitted, and resulting
packets mismatch because of the wrong checksum in the outer UDP header (it is
the same as the one in the original superframe). However, since the test sees
capability bit TX_OFFLOAD_OUTER_UDP_CKSUM in device info, the checksum is
expected to be offloaded.

Both X710 and E810 encounter the same issue. Upon reviewing the unloading
portion of the code, it's evident that only adding capabilities without
unloading part of the code was implemented.

[1] https://github.com/openvswitch/ovs-issues/issues/321

-- 
You are receiving this mail because:
You are the assignee for the bug.

RE: [PATCH 0/2] introduce PM QoS interface

[Morten Brørup]

> From: lihuisong (C) [mailto:lihuis...@huawei.com]
> Sent: Friday, 22 March 2024 09.54
> 
> +Tyler, +Alan, +Wei, +Long for asking this similar feature on Windows.
> 
> 在 2024/3/21 21:30, Morten Brørup 写道:
> >> From: lihuisong (C) [mailto:lihuis...@huawei.com]
> >> Sent: Thursday, 21 March 2024 04.04
> >>
> >> Hi Moren,
> >>
> >> Thanks for your revew.
> >>
> >> 在 2024/3/20 22:05, Morten Brørup 写道:
>  From: Huisong Li [mailto:lihuis...@huawei.com]
>  Sent: Wednesday, 20 March 2024 11.55
> 
>  The system-wide CPU latency QoS limit has a positive impact on the idle
>  state selection in cpuidle governor.
> 
>  Linux creates a cpu_dma_latency device under '/dev' directory to obtain
> the
>  CPU latency QoS limit on system and send the QoS request for userspace.
>  Please see the PM QoS framework in the following link:
>  https://docs.kernel.org/power/pm_qos_interface.html?highlight=qos
>  This feature is supported by kernel-v2.6.25.
> 
>  The deeper the idle state, the lower the power consumption, but the
> longer
>  the resume time. Some service are delay sensitive and very except the low
>  resume time, like interrupt packet receiving mode.
> 
>  So this series introduce PM QoS interface.
> >>> This looks like a 1:1 wrapper for a Linux kernel feature.
> >> right
> >>> Does Windows or BSD offer something similar?
> >> How do we know Windows or BSD support this similar feature?
> > Ask Windows experts or research using Google.
> I download freebsd source code, I didn't find this similar feature.
> They don't even support cpuidle feature(this QoS feature affects cpuilde.).
> I don't find any useful about this on Windows from google.
> 
> 
> @Tyler, @Alan, @Wei and @Long
> 
> Do you know windows support that userspace read and send CPU latency
> which has an impact on deep level of CPU idle?
> 
> >> The DPDK power lib just work on Linux according to the meson.build under
> >> lib/power.
> >> If they support this features, they can open it.
> > The DPDK power lib currently only works on Linux, yes.
> > But its API should still be designed to be platform agnostic, so the
> functions can be implemented on other platforms in the future.
> >
> > DPDK is on track to work across multiple platforms, including Windows.
> > We must always consider other platforms, and not design DPDK APIs as if they
> are for Linux/BSD only.
> totally understand you.
> >
> >>> Furthermore, any high-res timing should use nanoseconds, not microseconds
> or
> >> milliseconds.
> >>> I realize that the Linux kernel only uses microseconds for these APIs, but
> >> the DPDK API should use nanoseconds.
> >> Nanoseconds is more precise, it's good.
> >> But DPDK API how use nanoseconds as you said the the Linux kernel only
> >> uses microseconds for these APIs.
> >> Kernel interface just know an integer value with microseconds unit.
> > One solution is to expose nanoseconds in the DPDK API, and in the Linux
> specific implementation convert from/to microseconds.
> If so, we have to modify the implementation interface on Linux. This
> change the input/output unit about the interface.
> And DPDK also has to do this based on kernel version. It is not good.
> The cpuidle governor select which idle state based on the worst-case
> latency of idle state.
> These the worst-case latency of Cstate reported by ACPI table is in
> microseconds as the section 8.4.1.1. _CST (C States) and 8.4.3.3. _LPI
> (Low Power Idle States) in ACPI spec [1].
> So it is probably not meaning to change this interface implementation.

OK... Since microsecond resolution is good enough for ACPI and Linux, you have 
me convinced that it's also good enough for DPDK (for this specific topic).

Thank you for the detailed reply!

> 
> For the case need PM QoS in DPDK, I think, it is better to set cpu
> latency to zero to prevent service thread from the deeper the idle state.

It would defeat the purpose (i.e. not saving sufficient amounts of power) if 
the CPU cannot enter a deeper idle state.

Personally, I would think a wake-up latency of up to 10 microseconds should be 
fine for must purposes.
Default Linux timerslack is 50 microseconds, so you could also use that value.

> > You might also want to add a note to the in-line documentation of the
> relevant functions that the Linux implementation only uses microsecond
> resolution.
> >
> [1]
> https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html

[PATCH] Spelling: Fix spelling mistake.

[masoumeh . farhadinia]

From: Masi 

Caught by codespell.

Signed-off-by: Masi 
---
 examples/ipsec-secgw/parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ipsec-secgw/parser.c b/examples/ipsec-secgw/parser.c
index 98f8176651..2bd6df335b 100644
--- a/examples/ipsec-secgw/parser.c
+++ b/examples/ipsec-secgw/parser.c
@@ -388,7 +388,7 @@ cfg_parse_neigh(void *parsed_result, __rte_unused struct 
cmdline *cl,
rc = parse_mac(res->mac, &mac);
APP_CHECK(rc == 0, st, "invalid ether addr:%s", res->mac);
rc = add_dst_ethaddr(res->port, &mac);
-   APP_CHECK(rc == 0, st, "invalid port numer:%hu", res->port);
+   APP_CHECK(rc == 0, st, "invalid port number:%hu", res->port);
if (st->status < 0)
return;
 }
-- 
2.44.0.windows.1

[PATCH] Spelling : Fix spelling mistake.

[Fidel Castro]

Caught by codespell.

Signed-off-by: Fidel Castro 
---
 app/test/test_power.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_power.c b/app/test/test_power.c
index f1e80299d3..403adc22d6 100644
--- a/app/test/test_power.c
+++ b/app/test/test_power.c
@@ -143,7 +143,7 @@ test_power(void)
/* Test setting a valid environment */
ret = rte_power_set_env(envs[i]);
if (ret != 0) {
-   printf("Unexpectedly unsucceeded on setting a valid 
environment\n");
+   printf("Unexpectedly unsuccessful on setting a valid 
environment\n");
return -1;
}
 
-- 
2.44.0.windows.1

[PATCH] Spelling: Fixed a spelling.

[EmiAoki]

Caught by codespell.

Signed-off-by: emi 
---
 doc/guides/prog_guide/profile_app.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/profile_app.rst 
b/doc/guides/prog_guide/profile_app.rst
index 14292d4c25..a6b5fb4d5e 100644
--- a/doc/guides/prog_guide/profile_app.rst
+++ b/doc/guides/prog_guide/profile_app.rst
@@ -59,7 +59,7 @@ addition to the standard events, ``perf`` can be used to 
profile arm64
 specific PMU (Performance Monitor Unit) events through raw events (``-e``
 ``-rXX``).
 
-For more derails refer to the
+For more details refer to the
 `ARM64 specific PMU events enumeration 
`_.
 
 
-- 
2.39.2

[PATCH] Spelling: Fixed a spelling mistake.

[Flore]

Caught by codespell

Signed-off-by: Flore Norceide 
---
 doc/guides/prog_guide/packet_framework.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/packet_framework.rst 
b/doc/guides/prog_guide/packet_framework.rst
index ebc69d8c3e..9987ead6c5 100644
--- a/doc/guides/prog_guide/packet_framework.rst
+++ b/doc/guides/prog_guide/packet_framework.rst
@@ -509,7 +509,7 @@ the number of L2 or L3 cache memory misses is greatly 
reduced, hence one of the
 This is because the cost of L2/L3 cache memory miss on memory read accesses is 
high, as usually due to data dependency between instructions,
 the CPU execution units have to stall until the read operation is completed 
from L3 cache memory or external DRAM memory.
 By using prefetch instructions, the latency of memory read accesses is hidden,
-provided that it is preformed early enough before the respective data 
structure is actually used.
+provided that it is performed early enough before the respective data 
structure is actually used.
 
 By splitting the processing into several stages that are executed on different 
packets (the packets from the input burst are interlaced),
 enough work is created to allow the prefetch instructions to complete 
successfully (before the prefetched data structures are actually accessed) and
-- 
2.42.0.windows.2

[PATCH] Spelling: Fix Spelling mistake.

[vinh . t . tran10]

From: vtran0314 

Caught by codespell.

Signed-off-by: Vinh Tran 
---
 app/test/test_cfgfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_cfgfile.c b/app/test/test_cfgfile.c
index 2f596affee..adc6732cd1 100644
--- a/app/test/test_cfgfile.c
+++ b/app/test/test_cfgfile.c
@@ -168,7 +168,7 @@ test_cfgfile_invalid_section_header(void)
struct rte_cfgfile *cfgfile;
 
cfgfile = rte_cfgfile_load(CFG_FILES_ETC "/invalid_section.ini", 0);
-   TEST_ASSERT_NULL(cfgfile, "Expected failured did not occur");
+   TEST_ASSERT_NULL(cfgfile, "Expected failed did not occur");
 
return 0;
 }
-- 
2.44.0.windows.1

[PATCH] Spelling: Fixed a spelling mistake.

[hollynichols04]

From: Holly Nichols 

Caught by codespell.

Signed-off-by: Holly Nichols 
---
 app/test/test_cfgfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_cfgfile.c b/app/test/test_cfgfile.c
index 2f596affee..21bf34a718 100644
--- a/app/test/test_cfgfile.c
+++ b/app/test/test_cfgfile.c
@@ -196,7 +196,7 @@ test_cfgfile_invalid_key_value_pair(void)
struct rte_cfgfile *cfgfile;
 
cfgfile = rte_cfgfile_load(CFG_FILES_ETC "/empty_key_value.ini", 0);
-   TEST_ASSERT_NULL(cfgfile, "Expected failured did not occur");
+   TEST_ASSERT_NULL(cfgfile, "Expected failure did not occur");
 
return 0;
 }
-- 
2.44.0.windows.1

Re: [PATCH 0/3] support setting lanes

[Thomas Monjalon]

22/03/2024 06:51, Jerin Jacob:
> On Fri, Mar 22, 2024 at 10:56 AM Ajit Khaparde
>  wrote:
> >
> > On Thu, Mar 21, 2024 at 9:39 PM Jerin Jacob  wrote:
> > >
> > > On Fri, Mar 22, 2024 at 7:58 AM huangdengdui  
> > > wrote:
> > > >
> > > >
> > > >
> > > > On 2024/3/21 16:28, Thomas Monjalon wrote:
> > > > > 21/03/2024 03:02, huangdengdui:
> > > > >>
> > > > >> On 2024/3/20 20:31, Ferruh Yigit wrote:
> > > > >>> On 3/18/2024 9:26 PM, Damodharam Ammepalli wrote:
> > > >  On Mon, Mar 18, 2024 at 7:56 AM Thomas Monjalon 
> > > >   wrote:
> > > > >
> > > > > 12/03/2024 08:52, Dengdui Huang:
> > > > >> Some speeds can be achieved with different number of lanes. For 
> > > > >> example,
> > > > >> 100Gbps can be achieved using two lanes of 50Gbps or four lanes 
> > > > >> of 25Gbps.
> > > > >> When use different lanes, the port cannot be up.
> > > > >
> > > > > I'm not sure what you are referring to.
> > > > > I suppose it is not PCI lanes.
> > > > > Please could you link to an explanation of how a port is split in 
> > > > > lanes?
> > > > > Which hardware does this?
> > > > >
> > > >  This is a snapshot of 100Gb that the latest BCM576xx supports.
> > > >  100Gb (NRZ: 25G per lane, 4 lanes) link speed
> > > >  100Gb (PAM4-56: 50G per lane, 2 lanes) link speed
> > > >  100Gb (PAM4-112: 100G per lane, 1 lane) link speed
> > > > 
> > > >  Let the user feed in lanes=< integer value> and the NIC driver 
> > > >  decides
> > > >  the matching combination speed x lanes that works. In future if a 
> > > >  new speed
> > > >  is implemented with more than 8 lanes, there wouldn't be a need
> > > >  to touch this speed command. Using separate lane command would
> > > >  be a better alternative to support already shipped products and 
> > > >  only new
> > > >  drivers would consider this lanes configuration, if applicable.
> > > > 
> > > > >>>
> > > > >>> As far as I understand, lane is related to the physical layer of the
> > > > >>> NIC, there are multiple copies of transmitter, receiver, modulator 
> > > > >>> HW
> > > > >>> block and each set called as a 'lane' and multiple lanes work 
> > > > >>> together
> > > > >>> to achieve desired speed. (please correct me if this is wrong).
> > > > >>>
> > > > >>> Why not just configuring the speed is not enough? Why user needs to 
> > > > >>> know
> > > > >>> the detail and configuration of the lanes?
> > > > >>> Will it work if driver/device configure the "speed x lane" 
> > > > >>> internally
> > > > >>> for the requested speed?
> > > > >>>
> > > > >>> Is there a benefit to force specific lane count for a specific speed
> > > > >>> (like power optimization, just a wild guess)?
> > > > >>>
> > > > >>>
> > > > >>> And +1 for auto-negotiation if possible.
> > > > >>
> > > > >> As you said above,，multiple lanes work together to achieve desired 
> > > > >> speed.
> > > > >> For example, the following solutions can be used to implement 100G:
> > > > >> 1、Combines four 25G lanes
> > > > >> 2、Combines two 50G lanes
> > > > >> 3、A single 100G lane
> > > > >>
> > > > >> It is assumed that two ports are interconnected and the two ports 
> > > > >> support
> > > > >> the foregoing three solutions. But, we just configured the speed to 
> > > > >> 100G and
> > > > >> one port uses four 25G lanes by default and the other port uses two 
> > > > >> 50G lanes
> > > > >> by default, the port cannot be up. In this case, we need to 
> > > > >> configure the
> > > > >> two ports to use the same solutions (for example, uses two 50G lanes)
> > > > >> so that the ports can be up.
> > > > >
> > > > > Why this config is not OK? How do we know?
> > > > > Really I have a very bad feeling about this feature.
> > > > >
> > > > >
> > > > Sorry, I don't quite understand your question.
> > > > Are you asking why cannot be up when one port uses four 25G lanes and 
> > > > the other port uses two 50G lanes?
> > > >
> > > > 100GBASE-SR2 (two 50G lanes) and 100GBASE-SR4 (four 25G lanes) have 
> > > > different standards at the physical layer.[1]
> > > > So it's not possible to communicate. Configuring lanes can help the 
> > > > driver choose the same standard.
> > >
> > > Typically, low-level drivers like FW configure this.
> > >
> > > For example, If FW configures, 100G port as 100GBASE-SR2 then two
> > > ethdev(port 0 and port1) will show up.
> > > Now, assume if we expose this API and Can end user configure port 1 as
> > > 25G lines if so,
> > > a) What happens to port0 and it states?
> > There should be no impact to port0.
> >
> > > b) Will port2, port3 will show up after issuing this API(As end user
> > > configured 25Gx4 for 100G)? Will application needs to hotplug to get
> > > use ports.
> > No. The port count does not change. Nor does the number of PCI
> > functions seen by the host. Unless designed otherwise.
> >
> > Changing the lane count does not change anything in physical terms.

[PATCH] dma/cnxk: fix driver header

[David Marchand]

Add missing 'extern "C"' to file.
Using __rte_internal requires including rte_compat.h.

Fixes: 1693345b6a33 ("dma/cnxk: support DMA event enqueue/dequeue")

Signed-off-by: David Marchand 
---
 drivers/dma/cnxk/cnxk_dma_event_dp.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/dma/cnxk/cnxk_dma_event_dp.h 
b/drivers/dma/cnxk/cnxk_dma_event_dp.h
index 85374792a6..06b5ca8279 100644
--- a/drivers/dma/cnxk/cnxk_dma_event_dp.h
+++ b/drivers/dma/cnxk/cnxk_dma_event_dp.h
@@ -5,9 +5,14 @@
 #ifndef _CNXK_DMA_EVENT_DP_H_
 #define _CNXK_DMA_EVENT_DP_H_
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 #include 
 
 #include 
+#include 
 #include 
 
 __rte_internal
@@ -21,4 +26,9 @@ uint16_t cn9k_dma_adapter_dual_enqueue(void *ws, struct 
rte_event ev[], uint16_t
 
 __rte_internal
 uintptr_t cnxk_dma_adapter_dequeue(uintptr_t get_work1);
+
+#ifdef __cplusplus
+}
+#endif
+
 #endif /* _CNXK_DMA_EVENT_DP_H_ */
-- 
2.44.0

[PATCH] table: remove experimental CRC API for some arches

[David Marchand]

x86 and ARM architectures provide a non-experimental implementation for
rte_crc32_u64().
Experimental API rte_crc32_u64_generic() was only exported for other
arches.

Leaving this API exposed could result in portability issues: an
application using rte_crc32_u64_generic() would not compile on x86 or
ARM.

Move this symbol code in the only caller of the table library, and
remove this symbol.

Signed-off-by: David Marchand 
---
 lib/table/rte_table_hash_func.h | 29 +++--
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/lib/table/rte_table_hash_func.h b/lib/table/rte_table_hash_func.h
index a962ec2f68..aa779c2182 100644
--- a/lib/table/rte_table_hash_func.h
+++ b/lib/table/rte_table_hash_func.h
@@ -14,23 +14,6 @@ extern "C" {
 #include 
 #include 
 
-__rte_experimental
-static inline uint64_t
-rte_crc32_u64_generic(uint64_t crc, uint64_t value)
-{
-   int i;
-
-   crc = (crc & 0xLLU) ^ value;
-   for (i = 63; i >= 0; i--) {
-   uint64_t mask;
-
-   mask = -(crc & 1LLU);
-   crc = (crc >> 1LLU) ^ (0x82F63B78LLU & mask);
-   }
-
-   return crc;
-}
-
 #if defined(RTE_ARCH_X86_64)
 
 #include 
@@ -48,7 +31,17 @@ rte_crc32_u64(uint64_t crc, uint64_t v)
 static inline uint64_t
 rte_crc32_u64(uint64_t crc, uint64_t v)
 {
-   return rte_crc32_u64_generic(crc, v);
+   int i;
+
+   crc = (crc & 0xLLU) ^ v;
+   for (i = 63; i >= 0; i--) {
+   uint64_t mask;
+
+   mask = -(crc & 1LLU);
+   crc = (crc >> 1LLU) ^ (0x82F63B78LLU & mask);
+   }
+
+   return crc;
 }
 
 #endif
-- 
2.44.0

Re: [PATCH v2 1/6] ethdev: support setting lanes

[Thomas Monjalon]

22/03/2024 08:09, Dengdui Huang:
> -#define RTE_ETH_LINK_SPEED_10G RTE_BIT32(8)  /**< 10 Gbps */
> -#define RTE_ETH_LINK_SPEED_20G RTE_BIT32(9)  /**< 20 Gbps */
> -#define RTE_ETH_LINK_SPEED_25G RTE_BIT32(10) /**< 25 Gbps */
> -#define RTE_ETH_LINK_SPEED_40G RTE_BIT32(11) /**< 40 Gbps */
> -#define RTE_ETH_LINK_SPEED_50G RTE_BIT32(12) /**< 50 Gbps */
> -#define RTE_ETH_LINK_SPEED_56G RTE_BIT32(13) /**< 56 Gbps */
> -#define RTE_ETH_LINK_SPEED_100GRTE_BIT32(14) /**< 100 Gbps */
> -#define RTE_ETH_LINK_SPEED_200GRTE_BIT32(15) /**< 200 Gbps */
> -#define RTE_ETH_LINK_SPEED_400GRTE_BIT32(16) /**< 400 Gbps */
> +#define RTE_ETH_LINK_SPEED_10GRTE_BIT32(8)  /**< 10 Gbps */
> +#define RTE_ETH_LINK_SPEED_20GRTE_BIT32(9)  /**< 20 Gbps 2lanes 
> */
> +#define RTE_ETH_LINK_SPEED_25GRTE_BIT32(10) /**< 25 Gbps */
> +#define RTE_ETH_LINK_SPEED_40GRTE_BIT32(11) /**< 40 Gbps 4lanes 
> */
> +#define RTE_ETH_LINK_SPEED_50GRTE_BIT32(12) /**< 50 Gbps */
> +#define RTE_ETH_LINK_SPEED_56GRTE_BIT32(13) /**< 56 Gbps 4lanes 
> */
> +#define RTE_ETH_LINK_SPEED_100G   RTE_BIT32(14) /**< 100 Gbps */
> +#define RTE_ETH_LINK_SPEED_200G   RTE_BIT32(15) /**< 200 Gbps 4lanes 
> */
> +#define RTE_ETH_LINK_SPEED_400G   RTE_BIT32(16) /**< 400 Gbps 4lanes 
> */
> +#define RTE_ETH_LINK_SPEED_10G_4LANES RTE_BIT32(17)  /**< 10 Gbps 4lanes 
> */
> +#define RTE_ETH_LINK_SPEED_50G_2LANES RTE_BIT32(18) /**< 50 Gbps 2 lanes 
> */
> +#define RTE_ETH_LINK_SPEED_100G_2LANESRTE_BIT32(19) /**< 100 Gbps 2 
> lanes */
> +#define RTE_ETH_LINK_SPEED_100G_4LANESRTE_BIT32(20) /**< 100 Gbps 4lanes 
> */
> +#define RTE_ETH_LINK_SPEED_200G_2LANESRTE_BIT32(21) /**< 200 Gbps 2lanes 
> */
> +#define RTE_ETH_LINK_SPEED_400G_8LANESRTE_BIT32(22) /**< 400 Gbps 8lanes 
> */

I don't think it is a good idea to make this more complex.
It brings nothing as far as I can see, compared to having speed and lanes 
separated.
Can we have lanes information a separate value? no need for bitmask.

Re: [PATCH v2 1/6] ethdev: support setting lanes

[Ajit Khaparde]

On Fri, Mar 22, 2024 at 6:58 AM Thomas Monjalon  wrote:
>
> 22/03/2024 08:09, Dengdui Huang:
> > -#define RTE_ETH_LINK_SPEED_10G RTE_BIT32(8)  /**< 10 Gbps */
> > -#define RTE_ETH_LINK_SPEED_20G RTE_BIT32(9)  /**< 20 Gbps */
> > -#define RTE_ETH_LINK_SPEED_25G RTE_BIT32(10) /**< 25 Gbps */
> > -#define RTE_ETH_LINK_SPEED_40G RTE_BIT32(11) /**< 40 Gbps */
> > -#define RTE_ETH_LINK_SPEED_50G RTE_BIT32(12) /**< 50 Gbps */
> > -#define RTE_ETH_LINK_SPEED_56G RTE_BIT32(13) /**< 56 Gbps */
> > -#define RTE_ETH_LINK_SPEED_100GRTE_BIT32(14) /**< 100 Gbps */
> > -#define RTE_ETH_LINK_SPEED_200GRTE_BIT32(15) /**< 200 Gbps */
> > -#define RTE_ETH_LINK_SPEED_400GRTE_BIT32(16) /**< 400 Gbps */
> > +#define RTE_ETH_LINK_SPEED_10GRTE_BIT32(8)  /**< 10 Gbps */
> > +#define RTE_ETH_LINK_SPEED_20GRTE_BIT32(9)  /**< 20 Gbps 
> > 2lanes */
> > +#define RTE_ETH_LINK_SPEED_25GRTE_BIT32(10) /**< 25 Gbps */
> > +#define RTE_ETH_LINK_SPEED_40GRTE_BIT32(11) /**< 40 Gbps 
> > 4lanes */
> > +#define RTE_ETH_LINK_SPEED_50GRTE_BIT32(12) /**< 50 Gbps */
> > +#define RTE_ETH_LINK_SPEED_56GRTE_BIT32(13) /**< 56 Gbps 
> > 4lanes */
> > +#define RTE_ETH_LINK_SPEED_100G   RTE_BIT32(14) /**< 100 Gbps */
> > +#define RTE_ETH_LINK_SPEED_200G   RTE_BIT32(15) /**< 200 Gbps 
> > 4lanes */
> > +#define RTE_ETH_LINK_SPEED_400G   RTE_BIT32(16) /**< 400 Gbps 
> > 4lanes */
> > +#define RTE_ETH_LINK_SPEED_10G_4LANES RTE_BIT32(17)  /**< 10 Gbps 
> > 4lanes */
> > +#define RTE_ETH_LINK_SPEED_50G_2LANES RTE_BIT32(18) /**< 50 Gbps 2 
> > lanes */
> > +#define RTE_ETH_LINK_SPEED_100G_2LANESRTE_BIT32(19) /**< 100 Gbps 2 
> > lanes */
> > +#define RTE_ETH_LINK_SPEED_100G_4LANESRTE_BIT32(20) /**< 100 Gbps 
> > 4lanes */
> > +#define RTE_ETH_LINK_SPEED_200G_2LANESRTE_BIT32(21) /**< 200 Gbps 
> > 2lanes */
> > +#define RTE_ETH_LINK_SPEED_400G_8LANESRTE_BIT32(22) /**< 400 Gbps 
> > 8lanes */
>
> I don't think it is a good idea to make this more complex.
> It brings nothing as far as I can see, compared to having speed and lanes 
> separated.
> Can we have lanes information a separate value? no need for bitmask.
I agree.

>
>


smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v6 01/14] examples/l3fwd: fix queue ID restriction

[David Marchand]

Hello,

On Thu, Mar 21, 2024 at 7:48 PM Sivaprasad Tummala
 wrote:
>
> Currently application supports queue IDs up to 255

I think it only relates to Rx queue IDs.

Before this patch, the Tx queue count is already stored as a uint32_t
or uint16_t and checked against RTE_MAX_LCORE.
So no limit on the Tx queue count side.

Can you just adjust the commitlog accordingly?


(One may argue that the Tx queue count should be also checked against
RTE_MAX_QUEUES_PER_PORT, but it is a separate issue to this patch and
in practice, we probably always have RTE_MAX_QUEUES_PER_PORT >
RTE_MAX_LCORE).


> and max queues of 256 irrespective of device support.
> This limits the number of active lcores to 256.
>
> The patch fixes these constraints by increasing
> the queue IDs to support up to 65535.

[snip]

> diff --git a/examples/l3fwd/l3fwd_acl.c b/examples/l3fwd/l3fwd_acl.c
> index 401692bcec..2bd63181bc 100644
> --- a/examples/l3fwd/l3fwd_acl.c
> +++ b/examples/l3fwd/l3fwd_acl.c
> @@ -997,7 +997,7 @@ acl_main_loop(__rte_unused void *dummy)
> uint64_t prev_tsc, diff_tsc, cur_tsc;
> int i, nb_rx;
> uint16_t portid;
> -   uint8_t queueid;
> +   uint16_t queueid;
> struct lcore_conf *qconf;
> int socketid;
> const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1)
> @@ -1020,7 +1020,7 @@ acl_main_loop(__rte_unused void *dummy)
> portid = qconf->rx_queue_list[i].port_id;
> queueid = qconf->rx_queue_list[i].queue_id;
> RTE_LOG(INFO, L3FWD,
> -   " -- lcoreid=%u portid=%u rxqueueid=%hhu\n",
> +   " -- lcoreid=%u portid=%u rxqueueid=%hu\n",

Nit: should be %PRIu16 (idem in other hunks formatting a queue).


> lcore_id, portid, queueid);
> }
>

[snip]


> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 8d32ae1dd5..4d4738b92b 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c

[snip]


> @@ -366,7 +366,7 @@ init_lcore_rx_queues(void)
> nb_rx_queue = lcore_conf[lcore].n_rx_queue;
> if (nb_rx_queue >= MAX_RX_QUEUE_PER_LCORE) {
> printf("error: too many queues (%u) for lcore: %u\n",
> -   (unsigned)nb_rx_queue + 1, (unsigned)lcore);
> +   (unsigned int)nb_rx_queue + 1, (unsigned 
> int)lcore);

Nit: this does not seem related to the patch (probably a split issue,
as a later patch touches this part of the code too).


> return -1;
> } else {
> lcore_conf[lcore].rx_queue_list[nb_rx_queue].port_id =
> @@ -500,6 +500,8 @@ parse_config(const char *q_arg)
> char *str_fld[_NUM_FLD];
> int i;
> unsigned size;
> +   uint16_t max_fld[_NUM_FLD] = {USHRT_MAX,
> +   USHRT_MAX, UCHAR_MAX};

Nit: no newline.

This part validates user input for the rx queue used by a lcore.
Some later check in the example (or in ethdev) may raise an error if
requesting too many queues, but I think the limit here should be
RTE_MAX_QUEUES_PER_PORT.

Besides, this hunk also changes the check on max port and max lcore.
This is something that should be left untouched at this point of the series.

I would expect something like:
uint16_t max_fld[_NUM_FLD] = {255, RTE_MAX_QUEUES_PER_PORT, 255};


>
> nb_lcore_params = 0;
>
> @@ -518,7 +520,8 @@ parse_config(const char *q_arg)
> for (i = 0; i < _NUM_FLD; i++){
> errno = 0;
> int_fld[i] = strtoul(str_fld[i], &end, 0);
> -   if (errno != 0 || end == str_fld[i] || int_fld[i] > 
> 255)
> +   if (errno != 0 || end == str_fld[i] || int_fld[i] >
> +   
> max_fld[i])

Nit: no newline.

> return -1;
> }
> if (nb_lcore_params >= MAX_LCORE_PARAMS) {

[snip]


The other changes on the l3fwd example code in this series look good to me.


Thanks.

-- 
David Marchand

Re: [PATCH] dma/cnxk: fix driver header

[Tyler Retzlaff]

On Fri, Mar 22, 2024 at 02:52:55PM +0100, David Marchand wrote:
> Add missing 'extern "C"' to file.
> Using __rte_internal requires including rte_compat.h.
> 
> Fixes: 1693345b6a33 ("dma/cnxk: support DMA event enqueue/dequeue")
> 
> Signed-off-by: David Marchand 
> ---

Acked-by: Tyler Retzlaff 

[PATCH 1/2] mailmap: fix outdated entry for Zhirun

[David Marchand]

Fixes: aafd14f43eea ("maintainers: update for graph library")

Signed-off-by: David Marchand 
---
 .mailmap | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 50726e1232..83470af712 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1683,7 +1683,7 @@ Zhihong Wang  

 Zhike Wang  
 Zhimin Huang 
 Zhipeng Lu 
-Zhirun Yan 
+Zhirun Yan  
 Zhiwei He 
 Zhiyong Yang 
 Zhuobin Huang 
-- 
2.44.0

[PATCH 2/2] devtools: check that maintainers are listed in mailmap

[David Marchand]

When a maintainer changes its mail address, check that the associated
entry in .mailmap knows this address, otherwise there is a chance
.mailmap points at an obsolete address.

Signed-off-by: David Marchand 
---
 devtools/check-maintainers.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/devtools/check-maintainers.sh b/devtools/check-maintainers.sh
index 71697bb352..8a786e14a9 100755
--- a/devtools/check-maintainers.sh
+++ b/devtools/check-maintainers.sh
@@ -85,6 +85,18 @@ check_fx () # 
done
 }
 
+# Check that every maintainer mail is known of .mailmap:
+check_mailmap () #  
+{
+   sed -n -e 's/^M: \(.*<.*\)$/\1/p' $1 | sort -u | while read line; do
+   name=${line%% <*}
+   mail='<'${line##* <}
+   if ! grep -q "^$name <" $2 || ! grep -iq "^$name.*$mail" $2; 
then
+   echo $name mail address $mail is not in $2
+   fi
+   done
+}
+
 # Add a line to a set of lines if it begins with right pattern
 add_line_to_if () #   
 {
@@ -129,4 +141,10 @@ echo '# wrong patterns'
 echo '##'
 check_fx MAINTAINERS
 
+echo
+echo '##'
+echo '# wrong mailmap'
+echo '##'
+check_mailmap MAINTAINERS .mailmap
+
 # TODO: check overlaps
-- 
2.44.0

[PATCH v2] graph: expose node context as pointers

[Robin Jarry]

In some cases, the node context data is used to store two pointers
because the data is larger than the reserved 16 bytes. Having to define
intermediate structures just to be able to cast is tedious. Add two
pointers that take the same space than ctx.

Signed-off-by: Robin Jarry 
---

Notes:
v2:

* Added __extension__ (not sure where it is needed, I don't have access to 
windows).
* It still fails the header check for C++. It seems not possible to align 
an unnamed union...
  Tyler, do you have an idea about how to fix that?
* Added static_assert to ensure the anonymous union is not larger than 
RTE_NODE_CTX_SZ.

 lib/graph/rte_graph_worker_common.h | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/graph/rte_graph_worker_common.h 
b/lib/graph/rte_graph_worker_common.h
index 36d864e2c14e..a60c2bc3f0c3 100644
--- a/lib/graph/rte_graph_worker_common.h
+++ b/lib/graph/rte_graph_worker_common.h
@@ -112,7 +112,14 @@ struct __rte_cache_aligned rte_node {
};
/* Fast path area  */
 #define RTE_NODE_CTX_SZ 16
-   alignas(RTE_CACHE_LINE_SIZE) uint8_t ctx[RTE_NODE_CTX_SZ]; /**< Node 
Context. */
+   __extension__ alignas(RTE_CACHE_LINE_SIZE) union {
+   uint8_t ctx[RTE_NODE_CTX_SZ];
+   /* Convenience aliases to store pointers without complex 
casting. */
+   __extension__ struct {
+   void *ctx_ptr;
+   void *ctx_ptr2;
+   };
+   }; /**< Node Context. */
uint16_t size;  /**< Total number of objects available. */
uint16_t idx;   /**< Number of objects used. */
rte_graph_off_t off;/**< Offset of node in the graph reel. */
@@ -130,6 +137,9 @@ struct __rte_cache_aligned rte_node {
alignas(RTE_CACHE_LINE_MIN_SIZE) struct rte_node *nodes[]; /**< Next 
nodes. */
 };
 
+static_assert(offsetof(struct rte_node, size) - offsetof(struct rte_node, ctx) 
== RTE_NODE_CTX_SZ,
+   "The node context anonymous union cannot be larger than 
RTE_NODE_CTX_SZ");
+
 /**
  * @internal
  *
-- 
2.44.0

Re: [PATCH v2] graph: expose node context as pointers

[Tyler Retzlaff]

On Fri, Mar 22, 2024 at 05:31:31PM +0100, Robin Jarry wrote:
> In some cases, the node context data is used to store two pointers
> because the data is larger than the reserved 16 bytes. Having to define
> intermediate structures just to be able to cast is tedious. Add two
> pointers that take the same space than ctx.
> 
> Signed-off-by: Robin Jarry 
> ---
> 
> Notes:
> v2:
> 
> * Added __extension__ (not sure where it is needed, I don't have access 
> to windows).

i can answer this!

windows toolchains will only require __extension__ qualification on use
of statement expressions, so msvc won't require any use of __extension__
in this patch.

as a general rule of thumb __extension__ is something you may choose to
use for any gcc compiled code that is an extension to standard C and you
intend to use the -pedantic flag (i.e. -std=c11 && -pedantic used together)

i've commented inline below.

> * It still fails the header check for C++. It seems not possible to align 
> an unnamed union...
>   Tyler, do you have an idea about how to fix that?

yes, see below what i suspect you want.

> * Added static_assert to ensure the anonymous union is not larger than 
> RTE_NODE_CTX_SZ.
> 
>  lib/graph/rte_graph_worker_common.h | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/graph/rte_graph_worker_common.h 
> b/lib/graph/rte_graph_worker_common.h
> index 36d864e2c14e..a60c2bc3f0c3 100644
> --- a/lib/graph/rte_graph_worker_common.h
> +++ b/lib/graph/rte_graph_worker_common.h
> @@ -112,7 +112,14 @@ struct __rte_cache_aligned rte_node {
>   };
>   /* Fast path area  */
>  #define RTE_NODE_CTX_SZ 16
> - alignas(RTE_CACHE_LINE_SIZE) uint8_t ctx[RTE_NODE_CTX_SZ]; /**< Node 
> Context. */
> + __extension__ alignas(RTE_CACHE_LINE_SIZE) union {

__extension__ should not be on the anonymous union (since they are standard 
C11).

anonymous union declaration is actually a type with no name and then a data
field of that type so __rte_aligned is most likely what you want, since
you're using RTE_CACHE_LINE_SIZE we can use __rte_cache_aligned.

union __rte_cache_aligned {
   ... your union fields ...
};

and i think checkpatches still gives a warning unrelated to alignment
for this but it can be safely ignored. it's the warning about alignment
that we care about and should be fixed.

> + uint8_t ctx[RTE_NODE_CTX_SZ];
> + /* Convenience aliases to store pointers without complex 
> casting. */
> + __extension__ struct {

this is correct/recommended since anonymous structs aren't standard,
with the __extension__ -pedantic won't emit a warning (our intention).

> + void *ctx_ptr;
> + void *ctx_ptr2;
> + };
> + }; /**< Node Context. */
>   uint16_t size;  /**< Total number of objects available. */
>   uint16_t idx;   /**< Number of objects used. */
>   rte_graph_off_t off;/**< Offset of node in the graph reel. */
> @@ -130,6 +137,9 @@ struct __rte_cache_aligned rte_node {
>   alignas(RTE_CACHE_LINE_MIN_SIZE) struct rte_node *nodes[]; /**< Next 
> nodes. */
>  };
>  
> +static_assert(offsetof(struct rte_node, size) - offsetof(struct rte_node, 
> ctx) == RTE_NODE_CTX_SZ,
> + "The node context anonymous union cannot be larger than 
> RTE_NODE_CTX_SZ");
> +

you should include directly include  in this file for use of offsetof.
you should include directly include  in this file for use of the 
static_assert.

hope this helps!

ty

>  /**
>   * @internal
>   *
> -- 
> 2.44.0

Re: pcapng_autotest unit test false positive

[Patrick Robb]

Hi David,

Yes I'm seeing this pcapng_autotest fail intermittently on debian 11
recently. It also got flagged on Slack, where Stephen indicated it is
likely a lab infra failure.

Anyways, I guess based on the logs above it is a timestamp error from
TSC (as you can see HPET is not used). Indeed, the write_packets test
that is failing does call rte_get_tsc_cycles().

So, some steps I think we should take.

1. Refresh the debian11 test container image we are using in CI testing.
2. Reset VM which containers are running on (which does also reset tsc
cycle counter of course).
3. Re-image our VMs which the test containers run on, bringing it to
Ubuntu 22.04 and a newer kernel version.

If that fails, I guess we can also look at substituting HPET for TSC.
It looks like (provided you have set the right bootloader option) you
use -Duse_hpet=true with meson for this. But, it looks like the unit
test is written to use TSC instead of HPET anyways, so I don't think
this is relevant.

Does this sound reasonable? If so we will proceed.

@Cody Cheng Since I know you are refreshing the lab container images
anyways, let's bring debian 11 to the front of the queue. Thanks.

On Wed, Mar 20, 2024 at 2:02 PM David Marchand
 wrote:
>
> Hello Stephen,
>
> I noticed a (time based?) failure of the pcapng unit test in some UNH
> Debian 11 container.
> Please have a look.
>
> https://lab.dpdk.org/results/dashboard/patchsets/29604/
>
> --- stdout ---
> RTE>>pcapng_autotest
>  + --- +
>  + Test Suite : Test Pcapng Unit Test Suite
>  + --- +
> pcapng: output file /tmp/pcapng_test_oIueHb.pcapng
>  + TestCase [ 0] : test_add_interface succeeded
> pcapng: output file /tmp/pcapng_test_4hbuWV.pcapng
> 16:51:22.955616600: EE:47:6C:93:DE:F0 -> FF:FF:FF:FF:FF:FF type 800 length 200
>  + TestCase [ 1] : test_write_packets failed
>  + --- +
>  + Test Suite Summary : Test Pcapng Unit Test Suite
>  + --- +
>  + Tests Total :2
>  + Tests Skipped :  0
>  + Tests Executed : 2
>  + Tests Unsupported:   0
>  + Tests Passed :   1
>  + Tests Failed :   1
>  + --- +
> Test Failed
> RTE>>
> --- stderr ---
> EAL: Detected CPU lcores: 16
> EAL: Detected NUMA nodes: 2
> EAL: Detected static linkage of DPDK
> EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'VA'
> EAL: VFIO support initialized
> EAL: Device :03:00.0 is not NUMA-aware
> APP: HPET is not enabled, using TSC as default timer
> Timestamp out of range [16:51:16.481074161 .. 16:51:22.953203736]
> pcap_dispatch: failed:
>
>
> --
> David Marchand
>

Re: [PATCH v2 1/6] ethdev: support setting lanes

[Tyler Retzlaff]

On Fri, Mar 22, 2024 at 08:15:00AM -0700, Ajit Khaparde wrote:
> On Fri, Mar 22, 2024 at 6:58 AM Thomas Monjalon  wrote:
> >
> > 22/03/2024 08:09, Dengdui Huang:
> > > -#define RTE_ETH_LINK_SPEED_10G RTE_BIT32(8)  /**< 10 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_20G RTE_BIT32(9)  /**< 20 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_25G RTE_BIT32(10) /**< 25 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_40G RTE_BIT32(11) /**< 40 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_50G RTE_BIT32(12) /**< 50 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_56G RTE_BIT32(13) /**< 56 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_100GRTE_BIT32(14) /**< 100 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_200GRTE_BIT32(15) /**< 200 Gbps */
> > > -#define RTE_ETH_LINK_SPEED_400GRTE_BIT32(16) /**< 400 Gbps */
> > > +#define RTE_ETH_LINK_SPEED_10GRTE_BIT32(8)  /**< 10 Gbps */
> > > +#define RTE_ETH_LINK_SPEED_20GRTE_BIT32(9)  /**< 20 Gbps 
> > > 2lanes */
> > > +#define RTE_ETH_LINK_SPEED_25GRTE_BIT32(10) /**< 25 Gbps */
> > > +#define RTE_ETH_LINK_SPEED_40GRTE_BIT32(11) /**< 40 Gbps 
> > > 4lanes */
> > > +#define RTE_ETH_LINK_SPEED_50GRTE_BIT32(12) /**< 50 Gbps */
> > > +#define RTE_ETH_LINK_SPEED_56GRTE_BIT32(13) /**< 56 Gbps 
> > > 4lanes */
> > > +#define RTE_ETH_LINK_SPEED_100G   RTE_BIT32(14) /**< 100 Gbps */
> > > +#define RTE_ETH_LINK_SPEED_200G   RTE_BIT32(15) /**< 200 Gbps 
> > > 4lanes */
> > > +#define RTE_ETH_LINK_SPEED_400G   RTE_BIT32(16) /**< 400 Gbps 
> > > 4lanes */
> > > +#define RTE_ETH_LINK_SPEED_10G_4LANES RTE_BIT32(17)  /**< 10 Gbps 
> > > 4lanes */
> > > +#define RTE_ETH_LINK_SPEED_50G_2LANES RTE_BIT32(18) /**< 50 Gbps 2 
> > > lanes */
> > > +#define RTE_ETH_LINK_SPEED_100G_2LANESRTE_BIT32(19) /**< 100 Gbps 2 
> > > lanes */
> > > +#define RTE_ETH_LINK_SPEED_100G_4LANESRTE_BIT32(20) /**< 100 Gbps 
> > > 4lanes */
> > > +#define RTE_ETH_LINK_SPEED_200G_2LANESRTE_BIT32(21) /**< 200 Gbps 
> > > 2lanes */
> > > +#define RTE_ETH_LINK_SPEED_400G_8LANESRTE_BIT32(22) /**< 400 Gbps 
> > > 8lanes */
> >
> > I don't think it is a good idea to make this more complex.
> > It brings nothing as far as I can see, compared to having speed and lanes 
> > separated.
> > Can we have lanes information a separate value? no need for bitmask.
> I agree.

+1 api design coupling the two together is definitely undesirable it
seems like half the time you end up with redundant RTE_ETH_LANES_UNKNOWN.

> 
> >
> >

Re: [PATCH 0/2] introduce PM QoS interface

[Tyler Retzlaff]

On Fri, Mar 22, 2024 at 04:54:01PM +0800, lihuisong (C) wrote:
> +Tyler, +Alan, +Wei, +Long for asking this similar feature on Windows.
> 
> 在 2024/3/21 21:30, Morten Brørup 写道:
> >>From: lihuisong (C) [mailto:lihuis...@huawei.com]
> >>Sent: Thursday, 21 March 2024 04.04
> >>
> >>Hi Moren,
> >>
> >>Thanks for your revew.
> >>
> >>在 2024/3/20 22:05, Morten Brørup 写道:
> From: Huisong Li [mailto:lihuis...@huawei.com]
> Sent: Wednesday, 20 March 2024 11.55
> 
> The system-wide CPU latency QoS limit has a positive impact on the idle
> state selection in cpuidle governor.
> 
> Linux creates a cpu_dma_latency device under '/dev' directory to obtain 
> the
> CPU latency QoS limit on system and send the QoS request for userspace.
> Please see the PM QoS framework in the following link:
> https://docs.kernel.org/power/pm_qos_interface.html?highlight=qos
> This feature is supported by kernel-v2.6.25.
> 
> The deeper the idle state, the lower the power consumption, but the longer
> the resume time. Some service are delay sensitive and very except the low
> resume time, like interrupt packet receiving mode.
> 
> So this series introduce PM QoS interface.
> >>>This looks like a 1:1 wrapper for a Linux kernel feature.
> >>right
> >>>Does Windows or BSD offer something similar?
> >>How do we know Windows or BSD support this similar feature?
> >Ask Windows experts or research using Google.
> I download freebsd source code, I didn't find this similar feature.
> They don't even support cpuidle feature(this QoS feature affects cpuilde.).
> I don't find any useful about this on Windows from google.
> 
> 
> @Tyler, @Alan, @Wei and @Long
> 
> Do you know windows support that userspace read and send CPU latency
> which has an impact on deep level of CPU idle?

it is unlikely you'll find an api that let's you manage things in terms
of raw latency values as the linux knobs here do. windows more often employs
policy centric schemes to permit the system to abstract implementation detail.

powercfg is probably the closest thing you can use to tune the same
things on windows. where you select e.g. the 'performance' scheme but it
won't allow you to pick specific latency numbers.

https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/powercfg-command-line-options

> 
> >>The DPDK power lib just work on Linux according to the meson.build under
> >>lib/power.
> >>If they support this features, they can open it.
> >The DPDK power lib currently only works on Linux, yes.
> >But its API should still be designed to be platform agnostic, so the 
> >functions can be implemented on other platforms in the future.
> >
> >DPDK is on track to work across multiple platforms, including Windows.
> >We must always consider other platforms, and not design DPDK APIs as if they 
> >are for Linux/BSD only.
> totally understand you.

since lib/power isn't built for windows at this time i don't think it's
appropriate to constrain your innovation. i do appreciate the engagement
though and would just offer general guidance that if you can design your
api with some kind of abstraction in mind that would be great and by all
means if you can figure out how to wrangle powercfg /Qh into satisfying the
api in a policy centric way it might be kind of nice.

i'll let other windows experts chime in here if they choose.

thanks!

> >
> >>>Furthermore, any high-res timing should use nanoseconds, not microseconds 
> >>>or
> >>milliseconds.
> >>>I realize that the Linux kernel only uses microseconds for these APIs, but
> >>the DPDK API should use nanoseconds.
> >>Nanoseconds is more precise, it's good.
> >>But DPDK API how use nanoseconds as you said the the Linux kernel only
> >>uses microseconds for these APIs.
> >>Kernel interface just know an integer value with microseconds unit.
> >One solution is to expose nanoseconds in the DPDK API, and in the Linux 
> >specific implementation convert from/to microseconds.
> If so, we have to modify the implementation interface on Linux. This
> change the input/output unit about the interface.
> And DPDK also has to do this based on kernel version. It is not good.
> The cpuidle governor select which idle state based on the worst-case
> latency of idle state.
> These the worst-case latency of Cstate reported by ACPI table is in
> microseconds as the section 8.4.1.1. _CST (C States) and 8.4.3.3.
> _LPI (Low Power Idle States) in ACPI spec [1].
> So it is probably not meaning to change this interface implementation.
> 
> For the case need PM QoS in DPDK, I think, it is better to set cpu
> latency to zero to prevent service thread from the deeper the idle
> state.
> >You might also want to add a note to the in-line documentation of the 
> >relevant functions that the Linux implementation only uses microsecond 
> >resolution.
> >
> [1] 
> https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html

Re: release candidate 24.03-rc3

[Thinh Tran]

IBM - Power Systems
DPDK v24.03-rc3-2-gc96e9ca312


* Build CI on Fedora 30,37,38,39,40 container images for ppc64le
* Basic PF on Mellanox: No issue found
* Performance: not tested.
* OS:- RHEL 9.3  kernel: 5.14.0-362.2.1.el9_3.ppc64le
with gcc version 11.4.1 20230605 (Red Hat 11.4.1-2)
 - RHEL 9.2  kernel: 5.14.0-284.52.1.el9_2.ppc64le
with gcc version 11.3.1 20221121 (Red Hat 11.3.1-4) (GCC)

 - SLES 15-SP5 ppc64le kernel: 5.14.21-150500.55.44-default
with gcc version 12.3.0 (SUSE Linux)

Systems tested:
 - LPARs on IBM Power10 CHRP IBM,9105-22A
NICs:
- Mellanox Mellanox Technologies MT2894 Family [ConnectX-6 Lx]
- firmware version: 26.40.1000
- MLNX_OFED_LINUX-24.01-0.3.3.5


Regards,
Thinh Tran


On 3/17/2024 10:45 PM, Thomas Monjalon wrote:

A new DPDK release candidate is ready for testing:
https://git.dpdk.org/dpdk/tag/?id=v24.03-rc3

There are 153 new patches in this snapshot.

Release notes:
https://doc.dpdk.org/guides/rel_notes/release_24_03.html

As usual, you can report any issue on https://bugs.dpdk.org

Only documentation and bug fixes should be accepted at this stage.

DPDK 24.03-rc4 should be the last release candidate.
The final release should be done on 27th if no surprise.

Thank you everyone

RE: [PATCH v2 38/45] bus/vmbus: use rte stdatomic API

[Long Li]

> > The usage is okay. The value is used to notify the VSP (Hyper-V). It's 
> > always set
> (no read) from DPDK.
> >
> 
> OK, so my question was not "does it need to be atomic", but rather "why isn't 
> it
> marked RTE_ATOMIC() when it's treated as atomic".
> 
> But what you are saying is that it need not be atomic? Just the equivalent of
> WRITE_ONCE()? Or a relaxed atomic store?

Sorry I misunderstood your question. Yes, it will be a good idea to make 
"pending" as RTE_ATOMIC.

This value needs to be atomic. However, the existing code is still correct in 
that updating is done in atomic.

RE: [EXTERNAL] [PATCH v7 1/2] doc: remove outdated version details

[Akhil Goyal]

> SW PMDs documentation is updated to remove details of unsupported IPsec
> Multi-buffer versions. DPDK older than 20.11 is end of life. So, older
> DPDK versions are removed from the Crypto library version table.
> 
> Signed-off-by: Sivaramakrishnan Venkat 
> Signed-off-by: Brian Dooley 
> Acked-by: Pablo de Lara 
> Acked-by: Wathsala Vithanage 
> ---
Series applied to dpdk-next-crypto

RE: [PATCH v2 38/45] bus/vmbus: use rte stdatomic API

[Long Li]

>  static inline void
>  vmbus_set_monitor(const struct vmbus_channel *channel, uint32_t monitor_id)
> {
> - uint32_t *monitor_addr, monitor_mask;
> + RTE_ATOMIC(uint32_t) *monitor_addr, monitor_mask;

Does this mean monitor_mask will also change to RTE_ATOMIC(uint32_t)? 

Seems not necessary.

>   unsigned int trigger_index;
> 
>   trigger_index = monitor_id / HV_MON_TRIG_LEN;
>   monitor_mask = 1u << (monitor_id % HV_MON_TRIG_LEN);
> 
> - monitor_addr = &channel->monitor_page->trigs[trigger_index].pending;
> + monitor_addr =
> + (uint32_t __rte_atomic
> +*)&channel->monitor_page->trigs[trigger_index].pending;
>   vmbus_sync_set_bit(monitor_addr, monitor_mask);  }
> 
> --
> 1.8.3.1

RE: [EXTERNAL] [PATCH v3] doc/ipsec_mb: update Arm IPsec-MB library tag

[Akhil Goyal]

> > > Subject: Re: [EXTERNAL] [PATCH v3] doc/ipsec_mb: update Arm IPsec-MB
> library
> > > tag
> > >
> > > We are rebuilding from this tag right now. I know the IPSEC update is
> > > now postponed until after 24.03, but once this install is done, do you
> > > want me to issue retests for all the recent patches which had failed
> > > for ARM on this library previously? Then I guess it will be verified
> > > as you say.
> > >
> > Yes, IPsec-mb update is postponed.
> > But, I remember Bryan highlighted an issue in existing library.
> > Just wanted to check if this tag can be used in the current release or
> > Are we good without any changes.
> 
> Okay, triggered a DPDK main "periodic run" on the CI now that we are
> running from SECLIB-IPSEC-2024.03.12 and I see it passed. So it looks
> like this tag can be used with the current release.
> 
> Otherwise, I will take the opportunity to just trigger retests for all
> series which failed the crypto tests on ARM in recent weeks, just so
> we know whether the new library makes a difference (it should), even
> though the new requirement won't go through.

Thanks Patrick for testing this out.

Applied to dpdk-next-crypto

[RFC 0/2] Add support for link speed lanes

[Damodharam Ammepalli]

BRCM576xx NIC modules support speeds with different lanes configuration.
This is an alternate proposal to
https://patchwork.dpdk.org/project/dpdk/list/?series=31593

Please provide your review. Broadcom driver patches will follow.

Damodharam Ammepalli (2):
  lib/ethdev: Add link_speed lanes support into rte lib
  testpmd: Add speed lanes to testpmd config and show command

 app/test-pmd/cmdline.c | 142 +
 app/test-pmd/config.c  |  13 ++--
 lib/ethdev/ethdev_driver.h |  49 +
 lib/ethdev/rte_ethdev.c|  26 +++
 lib/ethdev/rte_ethdev.h|  52 ++
 lib/ethdev/version.map |   2 +
 6 files changed, 280 insertions(+), 4 deletions(-)

-- 
2.39.3


-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.


smime.p7s
Description: S/MIME Cryptographic Signature

[RFC 1/2] lib/ethdev: Add link_speed lanes support into rte lib

[Damodharam Ammepalli]

Update the eth_dev_ops structure with new function vectors
to get and set number of speed lanes. This will help user to
configure  the same fixed speed with different number of lanes
based on the physical carrier type.

Signed-off-by: Damodharam Ammepalli 
Reviewed-by: Kalesh AP 
Reviewed-by: Ajit Khaparde 
---
 lib/ethdev/ethdev_driver.h | 49 +++
 lib/ethdev/rte_ethdev.c| 26 +++
 lib/ethdev/rte_ethdev.h| 52 ++
 lib/ethdev/version.map |  2 ++
 4 files changed, 129 insertions(+)

diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 0dbf2dd6a2..b1f473e4de 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1179,6 +1179,51 @@ typedef int (*eth_rx_descriptor_dump_t)(const struct 
rte_eth_dev *dev,
uint16_t queue_id, uint16_t offset,
uint16_t num, FILE *file);
 
+/**
+ * @internal
+ * Get number of current active lanes and max supported lanes
+ *
+ * @param dev
+ *   ethdev handle of port.
+ * @param speed_lanes_capa
+ *   Number of active lanes that the link is trained up.
+ *   Max number of lanes supported by HW
+ * @return
+ *   Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ *   Success, get speed_lanes data success.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EIO
+ *   Device is removed.
+ */
+typedef int (*eth_speed_lanes_get_t)(struct rte_eth_dev *dev,
+struct rte_eth_speed_lanes_capa 
*speed_lanes_capa);
+
+/**
+ * @internal
+ * Set speed lanes
+ *
+ * @param dev
+ *   ethdev handle of port.
+ * @param speed_lanes_capa
+ *   Non-negative number of lanes
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ *   Success, set lanes success.
+ * @retval -ENOTSUP
+ *   Operation is not supported.
+ * @retval -EINVAL
+ *   Unsupported mode requested.
+ * @retval -EIO
+ *   Device is removed.
+ */
+typedef int (*eth_speed_lanes_set_t)(struct rte_eth_dev *dev, uint32_t 
speed_lanes_capa);
+
 /**
  * @internal
  * Dump Tx descriptor info to a file.
@@ -1474,6 +1519,10 @@ struct eth_dev_ops {
eth_count_aggr_ports_t count_aggr_ports;
/** Map a Tx queue with an aggregated port of the DPDK port */
eth_map_aggr_tx_affinity_t map_aggr_tx_affinity;
+   /** Get number of speed lanes supported and active lanes */
+   eth_speed_lanes_get_t speed_lanes_get;
+   /** Set number of speed lanes */
+   eth_speed_lanes_set_t speed_lanes_set;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..45e2f7645b 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -7008,4 +7008,30 @@ int rte_eth_dev_map_aggr_tx_affinity(uint16_t port_id, 
uint16_t tx_queue_id,
return ret;
 }
 
+int
+rte_eth_speed_lanes_get(uint16_t port_id, struct rte_eth_speed_lanes_capa 
*capa)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+   dev = &rte_eth_devices[port_id];
+
+   if (*dev->dev_ops->speed_lanes_get == NULL)
+   return -ENOTSUP;
+   return eth_err(port_id, (*dev->dev_ops->speed_lanes_get)(dev, capa));
+}
+
+int
+rte_eth_speed_lanes_set(uint16_t port_id, uint32_t speed_lanes_capa)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+   dev = &rte_eth_devices[port_id];
+
+   if (*dev->dev_ops->speed_lanes_set == NULL)
+   return -ENOTSUP;
+   return eth_err(port_id, (*dev->dev_ops->speed_lanes_set)(dev, 
speed_lanes_capa));
+}
+
 RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 147257d6a2..caae1f27c6 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1997,6 +1997,12 @@ struct rte_eth_fec_capa {
uint32_t capa;  /**< FEC capabilities bitmask */
 };
 
+/* A structure used to get and set lanes capabilities per link speed */
+struct rte_eth_speed_lanes_capa {
+   uint32_t active_lanes;
+   uint32_t max_lanes_cap;
+};
+
 #define RTE_ETH_ALL RTE_MAX_ETHPORTS
 
 /* Macros to check for valid port */
@@ -6917,6 +6923,52 @@ rte_eth_tx_queue_count(uint16_t port_id, uint16_t 
queue_id)
return rc;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get maximum speed lanes supported by the NIC.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param speed_lanes_capa
+ *   speed_lanes_capa is out only with max speed lanes capabilities.
+ *   If set to NULL, then its assumed zero or not supported.
+ *
+ * @return
+ *   - A non-negative value of active lanes that currently link is up with.
+ *   - A non-negative value that this HW scales up to for all speeds.
+ *   - (-ENOTSUP) if underlying hardware O

[RFC 2/2] testpmd: Add speed lanes to testpmd config and show command

[Damodharam Ammepalli]

Add speed lanes configuration and display commands support
to testpmd. Also provide display the lanes info show device info.

testpmd>
testpmd> port stop 0
testpmd> port config 0 speed_lanes 4
testpmd> port config 0 speed 20 duplex full
testpmd> port start 0
testpmd> show port summary 0
Number of available ports: 2
Port MAC Address   Name Driver Status   Link Lanes
014:23:F2:C3:BA:D2 :b1:00.0 net_bnxt   up   200 Gbps 4
testpmd>

testpmd> show port info 0

* Infos for port 0  *
MAC address: 14:23:F2:C3:BA:D2
Device name: :b1:00.0
Driver name: net_bnxt
Firmware-version: 228.9.115.0
Connect to socket: 2
memory allocation on the socket: 2
Link status: up
Link speed: 200 Gbps
Lanes: 4
Link duplex: full-duplex
Autoneg status: Off

Signed-off-by: Damodharam Ammepalli 
Reviewed-by: Kalesh AP 
Reviewed-by: Ajit Khaparde 
---
 app/test-pmd/cmdline.c | 142 +
 app/test-pmd/config.c  |  13 ++--
 2 files changed, 151 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b7759e38a8..785e5dd4de 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1361,6 +1361,27 @@ struct cmd_config_speed_all {
cmdline_fixed_string_t value2;
 };
 
+static int
+cmd_validate_lanes(portid_t pid, uint32_t *lanes)
+{
+   struct rte_eth_speed_lanes_capa spd_lanes = {0};
+   int ret;
+
+   ret = rte_eth_speed_lanes_get(pid, &spd_lanes);
+   /* if not supported default lanes to 0 */
+   if (ret == -ENOTSUP) {
+   *lanes = 0;
+   return 0;
+   }
+
+   if (*lanes > spd_lanes.max_lanes_cap) {
+   fprintf(stderr, "Invalid lanes %d configuration\n", *lanes);
+   return -1;
+   }
+
+   return 0;
+}
+
 static int
 parse_and_check_speed_duplex(char *speedstr, char *duplexstr, uint32_t *speed)
 {
@@ -1676,6 +1697,125 @@ static cmdline_parse_inst_t 
cmd_config_loopback_specific = {
},
 };
 
+/* *** configure speed_lanes for all ports *** */
+struct cmd_config_speed_lanes_all {
+   cmdline_fixed_string_t port;
+   cmdline_fixed_string_t keyword;
+   cmdline_fixed_string_t all;
+   cmdline_fixed_string_t item;
+   uint32_t lanes;
+};
+
+static void
+cmd_config_speed_lanes_all_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+   struct cmd_config_speed_lanes_all *res = parsed_result;
+   portid_t pid;
+
+   if (!all_ports_stopped()) {
+   fprintf(stderr, "Please stop all ports first\n");
+   return;
+   }
+
+   RTE_ETH_FOREACH_DEV(pid) {
+   if (cmd_validate_lanes(pid, &res->lanes))
+   return;
+   rte_eth_speed_lanes_set(pid, res->lanes);
+   }
+
+   cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
+}
+
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_port =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, port, 
"port");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_keyword =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, keyword,
+"config");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_all =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, all, "all");
+static cmdline_parse_token_string_t cmd_config_speed_lanes_all_item =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_speed_lanes_all, item,
+"speed_lanes");
+static cmdline_parse_token_num_t cmd_config_speed_lanes_all_lanes =
+   TOKEN_NUM_INITIALIZER(struct cmd_config_speed_lanes_all, lanes, 
RTE_UINT32);
+
+static cmdline_parse_inst_t cmd_config_speed_lanes_all = {
+   .f = cmd_config_speed_lanes_all_parsed,
+   .data = NULL,
+   .help_str = "port config all speed_lanes ",
+   .tokens = {
+   (void *)&cmd_config_speed_lanes_all_port,
+   (void *)&cmd_config_speed_lanes_all_keyword,
+   (void *)&cmd_config_speed_lanes_all_all,
+   (void *)&cmd_config_speed_lanes_all_item,
+   (void *)&cmd_config_speed_lanes_all_lanes,
+   NULL,
+   },
+};
+
+/* *** configure speed_lanes for specific port *** */
+struct cmd_config_speed_lanes_specific {
+   cmdline_fixed_string_t port;
+   cmdline_fixed_string_t keyword;
+   uint16_t port_id;
+   cmdline_fixed_string_t item;
+   uint32_t lanes;
+};
+
+static void
+cmd_config_speed_lanes_specific_parsed(void *parsed_result,
+  __rte_unused struct cmdline *cl,
+  __rte_unused void *data)
+{
+   struct cmd_config_speed_lanes_specific *res = parsed_result;
+
+   if (port_id_is_invalid(res->port_id, ENABLED_WAR

Re: [PATCH v2 1/6] ethdev: support setting lanes

[Damodharam Ammepalli]

On Fri, Mar 22, 2024 at 10:32 AM Tyler Retzlaff
 wrote:
>
> On Fri, Mar 22, 2024 at 08:15:00AM -0700, Ajit Khaparde wrote:
> > On Fri, Mar 22, 2024 at 6:58 AM Thomas Monjalon  wrote:
> > >
> > > 22/03/2024 08:09, Dengdui Huang:
> > > > -#define RTE_ETH_LINK_SPEED_10G RTE_BIT32(8)  /**< 10 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_20G RTE_BIT32(9)  /**< 20 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_25G RTE_BIT32(10) /**< 25 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_40G RTE_BIT32(11) /**< 40 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_50G RTE_BIT32(12) /**< 50 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_56G RTE_BIT32(13) /**< 56 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_100GRTE_BIT32(14) /**< 100 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_200GRTE_BIT32(15) /**< 200 Gbps */
> > > > -#define RTE_ETH_LINK_SPEED_400GRTE_BIT32(16) /**< 400 Gbps */
> > > > +#define RTE_ETH_LINK_SPEED_10GRTE_BIT32(8)  /**< 10 Gbps */
> > > > +#define RTE_ETH_LINK_SPEED_20GRTE_BIT32(9)  /**< 20 Gbps 
> > > > 2lanes */
> > > > +#define RTE_ETH_LINK_SPEED_25GRTE_BIT32(10) /**< 25 Gbps */
> > > > +#define RTE_ETH_LINK_SPEED_40GRTE_BIT32(11) /**< 40 Gbps 
> > > > 4lanes */
> > > > +#define RTE_ETH_LINK_SPEED_50GRTE_BIT32(12) /**< 50 Gbps */
> > > > +#define RTE_ETH_LINK_SPEED_56GRTE_BIT32(13) /**< 56 Gbps 
> > > > 4lanes */
> > > > +#define RTE_ETH_LINK_SPEED_100G   RTE_BIT32(14) /**< 100 Gbps 
> > > > */
> > > > +#define RTE_ETH_LINK_SPEED_200G   RTE_BIT32(15) /**< 200 Gbps 
> > > > 4lanes */
> > > > +#define RTE_ETH_LINK_SPEED_400G   RTE_BIT32(16) /**< 400 Gbps 
> > > > 4lanes */
> > > > +#define RTE_ETH_LINK_SPEED_10G_4LANES RTE_BIT32(17)  /**< 10 Gbps 
> > > > 4lanes */
> > > > +#define RTE_ETH_LINK_SPEED_50G_2LANES RTE_BIT32(18) /**< 50 Gbps 2 
> > > > lanes */
> > > > +#define RTE_ETH_LINK_SPEED_100G_2LANESRTE_BIT32(19) /**< 100 Gbps 
> > > > 2 lanes */
> > > > +#define RTE_ETH_LINK_SPEED_100G_4LANESRTE_BIT32(20) /**< 100 Gbps 
> > > > 4lanes */
> > > > +#define RTE_ETH_LINK_SPEED_200G_2LANESRTE_BIT32(21) /**< 200 Gbps 
> > > > 2lanes */
> > > > +#define RTE_ETH_LINK_SPEED_400G_8LANESRTE_BIT32(22) /**< 400 Gbps 
> > > > 8lanes */
> > >
> > > I don't think it is a good idea to make this more complex.
> > > It brings nothing as far as I can see, compared to having speed and lanes 
> > > separated.
> > > Can we have lanes information a separate value? no need for bitmask.
> > I agree.
>
> +1 api design coupling the two together is definitely undesirable it
> seems like half the time you end up with redundant RTE_ETH_LANES_UNKNOWN.

https://patchwork.dpdk.org/project/dpdk/list/?series=31606
This is how we have implemented internally and we could use this as a reference.
Ethtool 6.x allows lanes configuration this way.
ethtool -s ens6f0np0 speed  duplex full autoneg off lanes < int >
>
> >
> > >
> > >
>
>

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.


smime.p7s
Description: S/MIME Cryptographic Signature

Re: pcapng_autotest unit test false positive

[Stephen Hemminger]

On Fri, 22 Mar 2024 13:12:24 -0400
Patrick Robb  wrote:

> Hi David,
> 
> Yes I'm seeing this pcapng_autotest fail intermittently on debian 11
> recently. It also got flagged on Slack, where Stephen indicated it is
> likely a lab infra failure.
> 
> Anyways, I guess based on the logs above it is a timestamp error from
> TSC (as you can see HPET is not used). Indeed, the write_packets test
> that is failing does call rte_get_tsc_cycles().
> 
> So, some steps I think we should take.
> 
> 1. Refresh the debian11 test container image we are using in CI testing.
> 2. Reset VM which containers are running on (which does also reset tsc
> cycle counter of course).
> 3. Re-image our VMs which the test containers run on, bringing it to
> Ubuntu 22.04 and a newer kernel version.
> 
> If that fails, I guess we can also look at substituting HPET for TSC.
> It looks like (provided you have set the right bootloader option) you
> use -Duse_hpet=true with meson for this. But, it looks like the unit
> test is written to use TSC instead of HPET anyways, so I don't think
> this is relevant.
> 
> Does this sound reasonable? If so we will proceed.
> 
> @Cody Cheng Since I know you are refreshing the lab container images
> anyways, let's bring debian 11 to the front of the queue. Thanks.
> 
> On Wed, Mar 20, 2024 at 2:02 PM David Marchand
>  wrote:
> >
> > Hello Stephen,
> >
> > I noticed a (time based?) failure of the pcapng unit test in some UNH
> > Debian 11 container.
> > Please have a look.
> >
> > https://lab.dpdk.org/results/dashboard/patchsets/29604/
> >
> > --- stdout 
> > ---  
> > RTE>>pcapng_autotest  
> >  + --- +
> >  + Test Suite : Test Pcapng Unit Test Suite
> >  + --- +
> > pcapng: output file /tmp/pcapng_test_oIueHb.pcapng
> >  + TestCase [ 0] : test_add_interface succeeded
> > pcapng: output file /tmp/pcapng_test_4hbuWV.pcapng
> > 16:51:22.955616600: EE:47:6C:93:DE:F0 -> FF:FF:FF:FF:FF:FF type 800 length 
> > 200
> >  + TestCase [ 1] : test_write_packets failed
> >  + --- +
> >  + Test Suite Summary : Test Pcapng Unit Test Suite
> >  + --- +
> >  + Tests Total :2
> >  + Tests Skipped :  0
> >  + Tests Executed : 2
> >  + Tests Unsupported:   0
> >  + Tests Passed :   1
> >  + Tests Failed :   1
> >  + --- +
> > Test Failed  
> > RTE>>  
> > --- stderr 
> > ---
> > EAL: Detected CPU lcores: 16
> > EAL: Detected NUMA nodes: 2
> > EAL: Detected static linkage of DPDK
> > EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> > EAL: Selected IOVA mode 'VA'
> > EAL: VFIO support initialized
> > EAL: Device :03:00.0 is not NUMA-aware
> > APP: HPET is not enabled, using TSC as default timer
> > Timestamp out of range [16:51:16.481074161 .. 16:51:22.953203736]
> > pcap_dispatch: failed:
> >
> >
> > --
> > David Marchand
> >  

Could you build a simple test to see if TSC every runs backwards on
this machine. Or there could be yet another math error.
Or maybe container TSC is huge an wrapping around?

The point of the test is to make sure that there wasn't wraparound errors.

Re: [PATCH v2] graph: expose node context as pointers

[Robin Jarry]

Hi Tyler,

Tyler Retzlaff, Mar 22, 2024 at 17:56:

i can answer this!

windows toolchains will only require __extension__ qualification on use
of statement expressions, so msvc won't require any use of __extension__
in this patch.

as a general rule of thumb __extension__ is something you may choose to
use for any gcc compiled code that is an extension to standard C and you
intend to use the -pedantic flag (i.e. -std=c11 && -pedantic used together)


Got it, thanks!


>/* Fast path area  */
>  #define RTE_NODE_CTX_SZ 16
> -  alignas(RTE_CACHE_LINE_SIZE) uint8_t ctx[RTE_NODE_CTX_SZ]; /**< Node 
Context. */
> +  __extension__ alignas(RTE_CACHE_LINE_SIZE) union {

__extension__ should not be on the anonymous union (since they are standard 
C11).

anonymous union declaration is actually a type with no name and then a data
field of that type so __rte_aligned is most likely what you want, since
you're using RTE_CACHE_LINE_SIZE we can use __rte_cache_aligned.

union __rte_cache_aligned {
   ... your union fields ...
};

and i think checkpatches still gives a warning unrelated to alignment
for this but it can be safely ignored. it's the warning about alignment
that we care about and should be fixed.


This passes the C++ header check but it breaks the static_assert I just 
added. I believe the alignment is somehow transferred to all union 
fields. And since ctx is an array, it makes the whole union .


So before my patch:

 /* --- cacheline 3 boundary (192 bytes) --- */
 uint8_t  ctx[16] __attribute__((__aligned__(64))); /*   19216 */
 uint16_t size; /*   208 2 */

With the anonymous union aligned:

 /* --- cacheline 3 boundary (192 bytes) --- */
 union {
 uint8_t  ctx[16];  /*   19216 */
 struct {
 void *   ctx_ptr;  /*   192 8 */
 void *   ctx_ptr2; /*   200 8 */
 }; /*   19216 */
 } __attribute__((__aligned__(64)));/*   19264 */
 /* --- cacheline 4 boundary (256 bytes) --- */
 uint16_t size; /*   256 2 */

However, if I remove the explicit align, I get what I expect:

 /* --- cacheline 3 boundary (192 bytes) --- */
 union {
 uint8_t  ctx[16];  /*   19216 */
 struct {
 void *   ctx_ptr;  /*   192 8 */
 void *   ctx_ptr2; /*   200 8 */
 }; /*   19216 */
 }; /*   19216 */
 uint16_t size; /*   208 2 */

Is it OK to drop the explicit alignment? This is beyond my C skills :)


> +  uint8_t ctx[RTE_NODE_CTX_SZ];
> +  /* Convenience aliases to store pointers without complex casting. 
*/
> +  __extension__ struct {

this is correct/recommended since anonymous structs aren't standard,
with the __extension__ -pedantic won't emit a warning (our intention).


Ack.


> +static_assert(offsetof(struct rte_node, size) - offsetof(struct rte_node, 
ctx) == RTE_NODE_CTX_SZ,
> +  "The node context anonymous union cannot be larger than RTE_NODE_CTX_SZ");
> +

you should include directly include  in this file for use of offsetof.
you should include directly include  in this file for use of the 
static_assert.


Will do for v3.

Thanks!