[PATCH v2 1/5] ethdev: support setting and querying RSS algorithm

[Jie Hai]

Currently, rte_eth_rss_conf supports configuring and querying
rss hash functions, rss key and it's length, but not rss hash
algorithm.

The structure ``rte_eth_rss_conf`` is extended by adding a new
field "func". This represents the RSS algorithms to apply. The
following API is affected:
- rte_eth_dev_configure
- rte_eth_dev_rss_hash_update
- rte_eth_dev_rss_hash_conf_get

If the value of "func" used for configuration is a gibberish
value, report the error and return. Do the same for
rte_eth_dev_rss_hash_update and rte_eth_dev_configure.

To check whether the drivers report the "func" field, it is set
to default value before querying.

Signed-off-by: Jie Hai 
Signed-off-by: Dongdong Liu 
---
 doc/guides/rel_notes/release_23_11.rst |  2 ++
 lib/ethdev/rte_ethdev.c| 17 +
 lib/ethdev/rte_ethdev.h|  6 ++
 3 files changed, 25 insertions(+)

diff --git a/doc/guides/rel_notes/release_23_11.rst 
b/doc/guides/rel_notes/release_23_11.rst
index 4411bb32c195..3746436e8bc9 100644
--- a/doc/guides/rel_notes/release_23_11.rst
+++ b/doc/guides/rel_notes/release_23_11.rst
@@ -123,6 +123,8 @@ ABI Changes
Also, make sure to start the actual text at the margin.
===
 
+   * ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
+ algorithm.
 
 Known Issues
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 0840d2b5942a..4cbcdb344cac 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1445,6 +1445,14 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t 
nb_rx_q, uint16_t nb_tx_q,
goto rollback;
}
 
+   if (dev_conf->rx_adv_conf.rss_conf.func >= RTE_ETH_HASH_FUNCTION_MAX) {
+   RTE_ETHDEV_LOG(ERR,
+   "Ethdev port_id=%u invalid rss hash function (%u)\n",
+   port_id, dev_conf->rx_adv_conf.rss_conf.func);
+   ret = -EINVAL;
+   goto rollback;
+   }
+
/* Check if Rx RSS distribution is disabled but RSS hash is enabled. */
if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
(dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
@@ -4630,6 +4638,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
return -ENOTSUP;
}
 
+   if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
+   RTE_ETHDEV_LOG(ERR,
+   "Ethdev port_id=%u invalid rss hash function (%u)\n",
+   port_id, rss_conf->func);
+   return -EINVAL;
+   }
+
if (*dev->dev_ops->rss_hash_update == NULL)
return -ENOTSUP;
ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
@@ -4657,6 +4672,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
return -EINVAL;
}
 
+   rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
+
if (*dev->dev_ops->rss_hash_conf_get == NULL)
return -ENOTSUP;
ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 04a2564f222a..1bb5f23059ca 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -174,6 +174,7 @@ extern "C" {
 
 #include "rte_ethdev_trace_fp.h"
 #include "rte_dev_info.h"
+#include "rte_flow.h"
 
 extern int rte_eth_dev_logtype;
 
@@ -461,11 +462,16 @@ struct rte_vlan_filter_conf {
  * The *rss_hf* field of the *rss_conf* structure indicates the different
  * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
  * Supplying an *rss_hf* equal to zero disables the RSS feature.
+ *
+ * The *func* field of the *rss_conf* structure indicates the hash algorithm
+ * applied by the RSS hashing. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
+ * the PMD to use its best-effort algorithm rather than a specific one.
  */
 struct rte_eth_rss_conf {
uint8_t *rss_key;/**< If not NULL, 40-byte hash key. */
uint8_t rss_key_len; /**< hash key length in bytes. */
uint64_t rss_hf; /**< Hash functions to apply - see below. */
+   enum rte_eth_hash_function func;/**< Hash algorithm to apply. */
 };
 
 /*
-- 
2.33.0

[PATCH v2 0/5] support setting and querying RSS algorithms

[Jie Hai]

This patchset is to support setting and querying RSS algorithms.

--
v2:
1. return error if "func" is invalid.
2. modify the comments of the "func" field.
3. modify commit log of patch [3/5].
4. use malloc instead of rte_malloc.
5. adjust display format of RSS info.
6. remove the string display of rss_hf.
--

Huisong Li (1):
  net/hns3: support setting and querying RSS hash function

Jie Hai (4):
  ethdev: support setting and querying rss algorithm
  app/proc-info: fix never show RSS info
  app/proc-info: adjust the display format of RSS info
  app/proc-info: support querying RSS hash algorithm

 app/proc-info/main.c   | 45 +++-
 doc/guides/rel_notes/release_23_11.rst |  2 ++
 drivers/net/hns3/hns3_rss.c| 47 +++---
 lib/ethdev/rte_ethdev.c| 17 ++
 lib/ethdev/rte_ethdev.h|  6 
 5 files changed, 88 insertions(+), 29 deletions(-)

-- 
2.33.0

[PATCH v2 3/5] app/proc-info: fix never show RSS info

[Jie Hai]

Command show-port should show rss info (rss_key, len and rss_hf),
However, the information is showned only when rss_conf.rss_key is not
NULL. Since no memory is allocated for rss_conf.rss_key, rss_key
will always be NULL and the rss_info will never show. This patch
allocates memory for rss_conf.rss_key and makes it possible to show
rss info.

Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
Cc: sta...@dpdk.org

Signed-off-by: Jie Hai 
Signed-off-by: Dongdong Liu 
---
 app/proc-info/main.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 88cee0ca487b..f6b77a705dce 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1013,6 +1013,7 @@ show_port(void)
struct rte_eth_fc_conf fc_conf;
struct rte_ether_addr mac;
struct rte_eth_dev_owner owner;
+   uint8_t *rss_key;
 
/* Skip if port is not in mask */
if ((enabled_port_mask & (1ul << i)) == 0)
@@ -1171,19 +1172,25 @@ show_port(void)
printf("\n");
}
 
+   rss_key = malloc(dev_info.hash_key_size * sizeof(uint8_t));
+   if (rss_key == NULL)
+   return;
+
+   rss_conf.rss_key = rss_key;
+   rss_conf.rss_key_len = dev_info.hash_key_size;
ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
if (ret == 0) {
-   if (rss_conf.rss_key) {
-   printf("  - RSS\n");
-   printf("\t  -- RSS len %u key (hex):",
-   rss_conf.rss_key_len);
-   for (k = 0; k < rss_conf.rss_key_len; k++)
-   printf(" %x", rss_conf.rss_key[k]);
-   printf("\t  -- hf 0x%"PRIx64"\n",
-   rss_conf.rss_hf);
-   }
+   printf("  - RSS\n");
+   printf("\t  -- RSS len %u key (hex):",
+   rss_conf.rss_key_len);
+   for (k = 0; k < rss_conf.rss_key_len; k++)
+   printf(" %x", rss_conf.rss_key[k]);
+   printf("\t  -- hf 0x%"PRIx64"\n",
+   rss_conf.rss_hf);
}
 
+   free(rss_key);
+
 #ifdef RTE_LIB_SECURITY
show_security_context(i, true);
 #endif
-- 
2.33.0

[PATCH v2 4/5] app/proc-info: adjust the display format of RSS info

[Jie Hai]

This patch splits the length and value of RSS key into two parts,
removes spaces between RSS keys, and adds line breaks between RSS
key and RSS hf.

Before the adjustment, RSS info is shown as:
  - RSS
  -- RSS len 40 key (hex): 6d 5a 56 da 25 5b e c2 41 67 \
 25 3d 43 a3 8f b0 d0 ca 2b cb ae 7b 30 b4 77 cb 2d \
 a3 80 30 f2 c 6a 42 b7 3b be ac 1 fa -- hf 0x0
and after:
  - RSS info
  -- RSS key len : 40
  -- RSS key (hex) :
6d5a56da255b0ec24167253d43a38fb0d0ca2bcbae7b30b4 \
77cb2da38030f20c6a42b73bbeac01fa
  -- RSS hf : 0x0

Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
Cc: sta...@dpdk.org

Signed-off-by: Jie Hai 
---
 app/proc-info/main.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index f6b77a705dce..6d2d77fea6ba 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1180,12 +1180,13 @@ show_port(void)
rss_conf.rss_key_len = dev_info.hash_key_size;
ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
if (ret == 0) {
-   printf("  - RSS\n");
-   printf("\t  -- RSS len %u key (hex):",
+   printf("  - RSS info\n");
+   printf("\t  -- key len : %u\n",
rss_conf.rss_key_len);
+   printf("\t  -- key (hex) : ");
for (k = 0; k < rss_conf.rss_key_len; k++)
-   printf(" %x", rss_conf.rss_key[k]);
-   printf("\t  -- hf 0x%"PRIx64"\n",
+   printf("%02x", rss_conf.rss_key[k]);
+   printf("\n\t  -- hf : 0x%"PRIx64"\n",
rss_conf.rss_hf);
}
 
-- 
2.33.0

[PATCH v2 5/5] app/proc-info: support querying RSS hash algorithm

[Jie Hai]

Display RSS hash algorithm with command show-port as below.
  - RSS info
  -- hash algorithm : toeplitz

Signed-off-by: Jie Hai 
Signed-off-by: Dongdong Liu 
---
 app/proc-info/main.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 6d2d77fea6ba..02b418a4c661 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -996,6 +996,23 @@ show_offloads(uint64_t offloads,
}
 }
 
+static const char *
+rss_func_to_str(enum rte_eth_hash_function func)
+{
+   switch (func) {
+   case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
+   return "simple_xor";
+   case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
+   return "toeplitz";
+   case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
+   return "symmetric_toeplitz";
+   case RTE_ETH_HASH_FUNCTION_DEFAULT:
+   return "default";
+   default:
+   return "unknown";
+   }
+}
+
 static void
 show_port(void)
 {
@@ -1188,6 +1205,8 @@ show_port(void)
printf("%02x", rss_conf.rss_key[k]);
printf("\n\t  -- hf : 0x%"PRIx64"\n",
rss_conf.rss_hf);
+   printf("\t  -- hash algorithm : %s\n",
+   rss_func_to_str(rss_conf.func));
}
 
free(rss_key);
-- 
2.33.0

[PATCH v2 2/5] net/hns3: support setting and querying RSS hash function

[Jie Hai]

From: Huisong Li 

Support setting and querying RSS hash function by ethdev ops.

Signed-off-by: Huisong Li 
Signed-off-by: Dongdong Liu 
---
 drivers/net/hns3/hns3_rss.c | 47 +
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 6126512bd780..c8346d43d15c 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -646,14 +646,14 @@ hns3_dev_rss_hash_update(struct rte_eth_dev *dev,
if (ret)
goto set_tuple_fail;
 
-   if (key) {
-   ret = hns3_rss_set_algo_key(hw, hw->rss_info.hash_algo,
-   key, hw->rss_key_size);
-   if (ret)
-   goto set_algo_key_fail;
-   /* Update the shadow RSS key with user specified */
+   ret = hns3_update_rss_algo_key(hw, rss_conf->func, key, key_len);
+   if (ret != 0)
+   goto set_algo_key_fail;
+
+   if (rss_conf->func != RTE_ETH_HASH_FUNCTION_DEFAULT)
+   hw->rss_info.hash_algo = hns3_hash_func_map[rss_conf->func];
+   if (key != NULL)
memcpy(hw->rss_info.key, key, hw->rss_key_size);
-   }
hw->rss_info.rss_hf = rss_hf;
rte_spinlock_unlock(&hw->lock);
 
@@ -769,7 +769,13 @@ int
 hns3_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
   struct rte_eth_rss_conf *rss_conf)
 {
+   const uint8_t hash_func_map[] = {
+   [HNS3_RSS_HASH_ALGO_TOEPLITZ] = RTE_ETH_HASH_FUNCTION_TOEPLITZ,
+   [HNS3_RSS_HASH_ALGO_SIMPLE] = RTE_ETH_HASH_FUNCTION_SIMPLE_XOR,
+   [HNS3_RSS_HASH_ALGO_SYMMETRIC_TOEP] = 
RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
+   };
struct hns3_adapter *hns = dev->data->dev_private;
+   uint8_t rss_key[HNS3_RSS_KEY_SIZE_MAX] = {0};
struct hns3_hw *hw = &hns->hw;
uint8_t hash_algo;
int ret;
@@ -777,26 +783,27 @@ hns3_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
rte_spinlock_lock(&hw->lock);
ret = hns3_rss_hash_get_rss_hf(hw, &rss_conf->rss_hf);
if (ret != 0) {
+   rte_spinlock_unlock(&hw->lock);
hns3_err(hw, "obtain hash tuples failed, ret = %d", ret);
-   goto out;
+   return ret;
+   }
+
+   ret = hns3_rss_get_algo_key(hw, &hash_algo, rss_key, hw->rss_key_size);
+   if (ret != 0) {
+   rte_spinlock_unlock(&hw->lock);
+   hns3_err(hw, "obtain hash algo and key failed, ret = %d", ret);
+   return ret;
}
+   rte_spinlock_unlock(&hw->lock);
 
-   /* Get the RSS Key required by the user */
+   /* Get the RSS Key if user required. */
if (rss_conf->rss_key && rss_conf->rss_key_len >= hw->rss_key_size) {
-   ret = hns3_rss_get_algo_key(hw, &hash_algo, rss_conf->rss_key,
-   hw->rss_key_size);
-   if (ret != 0) {
-   hns3_err(hw, "obtain hash algo and key failed, ret = 
%d",
-ret);
-   goto out;
-   }
+   memcpy(rss_conf->rss_key, rss_key, hw->rss_key_size);
rss_conf->rss_key_len = hw->rss_key_size;
}
+   rss_conf->func = hash_func_map[hash_algo];
 
-out:
-   rte_spinlock_unlock(&hw->lock);
-
-   return ret;
+   return 0;
 }
 
 /*
-- 
2.33.0

RE: [PATCH v2] mbuf: add ESP packet type

[Morten Brørup]

> From: Alexander Kozyrev [mailto:akozy...@nvidia.com]
> Sent: Saturday, 26 August 2023 01.34
> 
> Support the IP Encapsulating Security Payload (ESP) in transport mode.
> 
> Signed-off-by: Alexander Kozyrev 
> ---
>  lib/mbuf/rte_mbuf_ptype.h | 31 +++
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/mbuf/rte_mbuf_ptype.h b/lib/mbuf/rte_mbuf_ptype.h
> index 17a2dd3576..d20199580c 100644
> --- a/lib/mbuf/rte_mbuf_ptype.h
> +++ b/lib/mbuf/rte_mbuf_ptype.h
> @@ -294,10 +294,10 @@ extern "C" {

Please also update the comment accordingly. E.g.:

 * It refers to those packets of any IP types, while cannot be recognized as
 * any of above L4 types (RTE_PTYPE_L4_ICMP, RTE_PTYPE_L4_TCP,
 * RTE_PTYPE_L4_UDP, RTE_PTYPE_L4_FRAG (for IPv6), RTE_PTYPE_L4_ESP,
 * RTE_PTYPE_L4_SCTP).

Please note that I took the liberty to reorder the protocols in numerical 
order, and add that RTE_PTYPE_L4_FRAG only applies to IPv6. At your preference, 
feel free to keep the original ordering and add RTE_PTYPE_L4_ESP at the end, or 
use my suggested changes.

>   *
>   * Packet format:
>   * <'ether type'=0x0800
> - * | 'version'=4, 'protocol'!=[6|17|132|1], 'MF'=0, 'frag_offset'=0>
> + * | 'version'=4, 'protocol'!=[6|17|132|1|50], 'MF'=0, 'frag_offset'=0>

I notice that ICMP(1) is already last in the list, but this list is more 
readable with the protocols enumerated in numerical order:

 * | 'version'=4, 'protocol'!=[1|6|17|50|132], 'MF'=0, 'frag_offset'=0>

>   * or,
>   * <'ether type'=0x86DD
> - * | 'version'=6, 'next header'!=[6|17|44|132|1]>
> + * | 'version'=6, 'next header'!=[6|17|44|132|1|50]>

Same comment about numerical order here:

 * | 'version'=6, 'next header'!=[1|6|17|44|50|132]>

>   */
>  #define RTE_PTYPE_L4_NONFRAG0x0600
>  /**
> @@ -308,6 +308,17 @@ extern "C" {
>   * | 'version'=4, 'protocol'=2, 'MF'=0, 'frag_offset'=0>
>   */
>  #define RTE_PTYPE_L4_IGMP   0x0700
> +/**
> + * ESP (IP Encapsulating Security Payload) transport packet type.
> + *
> + * Packet format:
> + * <'ether type'=0x0800
> + * | 'version'=4, 'protocol'=50, 'MF'=0, 'frag_offset'=0>
> + * or,
> + * <'ether type'=0x86DD
> + * | 'version'=6, 'next header'=50>
> + */
> +#define RTE_PTYPE_L4_ESP0x0800
>  /**
>   * Mask of layer 4 packet types.
>   * It is used for outer packet for tunneling cases.
> @@ -652,12 +663,24 @@ extern "C" {
>   *
>   * Packet format (inner only):
>   * <'ether type'=0x0800
> - * | 'version'=4, 'protocol'!=[6|17|132|1], 'MF'=0, 'frag_offset'=0>
> + * | 'version'=4, 'protocol'!=[6|17|132|1|50], 'MF'=0, 'frag_offset'=0>
>   * or,
>   * <'ether type'=0x86DD
> - * | 'version'=6, 'next header'!=[6|17|44|132|1]>
> + * | 'version'=6, 'next header'!=[6|17|44|132|1|50]>
>   */
>  #define RTE_PTYPE_INNER_L4_NONFRAG  0x0600
> +/**
> + * ESP (IP Encapsulating Security Payload) transport packet type.
> + * It is used for inner packet only.
> + *
> + * Packet format (inner only):
> + * <'ether type'=0x0800
> + * | 'version'=4, 'protocol'=50, 'MF'=0, 'frag_offset'=0>
> + * or,
> + * <'ether type'=0x86DD
> + * | 'version'=6, 'next header'=50>
> + */
> +#define RTE_PTYPE_INNER_L4_ESP  0x0800
>  /**
>   * Mask of inner layer 4 packet types.
>   */
> --
> 2.18.2

For v3, you can add:

Acked-by: Morten Brørup 

Re: [PATCH] remove wrappers for GCC < 4.8

[David Marchand]

On Fri, Aug 25, 2023 at 6:14 PM Tyler Retzlaff
 wrote:
> On Thu, Aug 24, 2023 at 10:52:04AM +0200, David Marchand wrote:
> > Hello Tyler,
> >
> > On Thu, Aug 24, 2023 at 10:30 AM David Marchand
> >  wrote:
> > > diff --git a/lib/eal/include/rte_debug.h b/lib/eal/include/rte_debug.h
> > > index 2c4b94a7c9..74593cd4d4 100644
> > > --- a/lib/eal/include/rte_debug.h
> > > +++ b/lib/eal/include/rte_debug.h
> > > @@ -60,11 +60,7 @@ void rte_dump_stack(void);
> > >   * documentation.
> > >   */
> > >  void __rte_panic(const char *funcname , const char *format, ...)
> > > -#ifdef __GNUC__
> > > -#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
> > > __rte_cold
> > > -#endif
> > > -#endif
> >
> > I don't see some wrapping around __rte_cold for MSVC in your series.
> > Would this patch break MSVC buidlds?
>
> If it gets expanded on MSVC probably. But don't let it stop you from
> applying this series. You can just expand it empty if you want.

That's what I did for safety when I applied the msvc series.

>
> Until the atomics series is merged msvc still won't build so I'll supply
> minor fixes as these things pop up.
>
> Once everything is staged I'll work with UNH to get a basic build CI
> going to reduce the overhead of worrying about this kind of change being
> breaking.

Yes, the sooner, the better.


-- 
David Marchand

Re: [PATCH] drivers: add dependencies for some classes

[David Marchand]

On Fri, Aug 25, 2023 at 8:18 PM Morten Brørup  
wrote:
>
> > From: David Marchand [mailto:david.march...@redhat.com]
> > Sent: Friday, 25 August 2023 19.03
> >
> > A few classes meson.build were not expressing dependencies to the
> > associated device library. Define std_deps for baseband, gpu and regex
> > drivers.
> >
> > Signed-off-by: David Marchand 
> > ---
>
> On the surface, it looks like you are also removing a lot of (superfluous) 
> dependencies. If not just a side effect of the added std_deps, perhaps the 
> patch description should mention this too.

The dependency to the device library in a driver become superfluous as
a result of adding it to std_deps:
- std_deps is passed to drivers as the default deps:
https://git.dpdk.org/dpdk/tree/drivers/meson.build#n125
- none of those updated drivers meson.build was overriding/resetting
deps (grep -w deps $patch).

I'll add a note in the commitlog to make this explicit.

>
> Anyway,
>
> Acked-by: Morten Brørup 
>

Thanks.


-- 
David Marchand

mbuf fast free optimization idea

[Morten Brørup]

When RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE is used, the usage pattern of some of 
the mbuf fields differs:

- The "pool" pointer becomes rarely used. (Only one packet per burst.)
- The "next" pointer becomes unused.
- The "tx_offload" field is still used. (Set by application, read by driver.)

This means that the 2nd cache line in the mbuf now only contains one hot field, 
"tx_offload".

So, if we swap the "tx_offload" field (in the 2nd cache line) with the "pool" 
field (in the 1st cache line), the 2nd cache line becomes rarely used.

This should provide a performance improvement for applications (using 
RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) when under cache pressure.

PS: Back in the days, the mbuf description said that the 1st cache line was for 
RX purposes, and the 2nd cache line was for TX purposes. This is not true 
anymore, so let's not limit out thinking by that (obsolete) design.


Med venlig hilsen / Kind regards,
-Morten Brørup

RE: [PATCH v11 01/16] eal: use rdtsc intrinsic

[Ali Alnubani]

> -Original Message-
> From: Tyler Retzlaff 
> Sent: Friday, August 11, 2023 10:21 PM
> To: dev@dpdk.org
> Cc: Bruce Richardson ; Konstantin Ananyev
> ; Ciara Power ;
> NBU-Contact-Thomas Monjalon (EXTERNAL) ;
> david.march...@redhat.com; m...@smartsharesystems.com; Tyler Retzlaff
> 
> Subject: [PATCH v11 01/16] eal: use rdtsc intrinsic
> 
> Inline assembly is not supported for MSVC x64. Convert code to use
> __rdtsc intrinsic.
> 
> Signed-off-by: Tyler Retzlaff 
> Acked-by: Konstantin Ananyev 
> Acked-by: Morten Brørup 
> ---

Hello,

This patch is causing a build failure in Windows with Clang 11:

"""
[72/803] Compiling C object 
drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj
FAILED: 
drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj
clang 
@drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj.rsp
In file included from ../drivers/common/idpf/idpf_common_rxtx_avx512.c:6:
In file included from ..\drivers\common\idpf/idpf_common_device.h:9:
In file included from ..\drivers\common\idpf/base/idpf_prototype.h:9:
In file included from ..\drivers\common\idpf/base/idpf_osdep.h:18:
In file included from ..\lib\eal\include\rte_malloc.h:16:
In file included from ..\lib\eal\include\rte_memory.h:25:
In file included from ..\lib\eal\include\rte_fbarray.h:39:
In file included from ..\lib\eal\x86\include\rte_rwlock.h:13:
In file included from ..\lib\eal\x86\include/rte_spinlock.h:18:
In file included from ..\lib\eal\x86\include/rte_cycles.h:12:
In file included from C:\Tools\LLVM\lib\clang\11.0.0\include\x86intrin.h:24:
C:\Tools\LLVM\lib\clang\11.0.0\include\prfchwintrin.h:50:1: error: conflicting 
types for '__m_prefetchw'
_m_prefetchw(void *__P)
^
..\lib\eal\windows\include\rte_windows.h:28:22: note: expanded from macro 
'_m_prefetchw'
#define _m_prefetchw __m_prefetchw
 ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\um\winnt.h:3324:1: 
note: previous declaration is here
_m_prefetchw (
^
..\lib\eal\windows\include\rte_windows.h:28:22: note: expanded from macro 
'_m_prefetchw'
#define _m_prefetchw __m_prefetchw
 ^
1 error generated.
[73/803] Generating rte_bus_auxiliary_def with a custom command
[74/803] Compiling C object 
drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_controlq.c.obj
[75/803] Compiling C object 
drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_idpf_common_rxtx.c.obj
[76/803] Compiling C object 
drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_controlq_setup.c.obj
[77/803] Generating rte_bus_vdev.pmd.c with a custom command
[78/803] Compiling C object 
drivers/a715181@@tmp_rte_bus_pci@sta/bus_pci_pci_params.c.obj
[79/803] Compiling C object 
drivers/a715181@@tmp_rte_bus_pci@sta/bus_pci_pci_common.c.obj
[80/803] Compiling C object 
drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_common.c.obj
[81/803] Compiling C object 
drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_idpf_common_virtchnl.c.obj
ninja: build stopped: subcommand failed.
"""

Cross build with x86_64-w64-mingw32-gcc 12.2.1 on Fedora Linux doesn't 
reproduce.

Regards,
Ali

Re: mbuf fast free optimization idea

[Asaf Penso]

+ @Slava Ovsiienko

Regards,
Asaf Penso

From: Morten Brørup 
Sent: Saturday, August 26, 2023 1:04:18 PM
To: olivier.m...@6wind.com ; NBU-Contact-Thomas 
Monjalon (EXTERNAL) ; Shahaf Shuler 
Cc: dev@dpdk.org 
Subject: mbuf fast free optimization idea

When RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE is used, the usage pattern of some of 
the mbuf fields differs:

- The "pool" pointer becomes rarely used. (Only one packet per burst.)
- The "next" pointer becomes unused.
- The "tx_offload" field is still used. (Set by application, read by driver.)

This means that the 2nd cache line in the mbuf now only contains one hot field, 
"tx_offload".

So, if we swap the "tx_offload" field (in the 2nd cache line) with the "pool" 
field (in the 1st cache line), the 2nd cache line becomes rarely used.

This should provide a performance improvement for applications (using 
RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) when under cache pressure.

PS: Back in the days, the mbuf description said that the 1st cache line was for 
RX purposes, and the 2nd cache line was for TX purposes. This is not true 
anymore, so let's not limit out thinking by that (obsolete) design.


Med venlig hilsen / Kind regards,
-Morten Brørup

RE: [RFC] lib/st_ring: add single thread ring

[Honnappa Nagarahalli]

> 
> > From: Mattias Rönnblom [mailto:hof...@lysator.liu.se]
> > Sent: Thursday, 24 August 2023 12.53
> >
> > On 2023-08-24 10:05, Morten Brørup wrote:
> > >> From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com]
> > >> Sent: Tuesday, 22 August 2023 07.47
> > >>
> > >>> From: Morten Brørup 
> > >>> Sent: Monday, August 21, 2023 2:37 AM
> > >>>
> >  From: Honnappa Nagarahalli
> [mailto:honnappa.nagaraha...@arm.com]
> >  Sent: Monday, 21 August 2023 08.04
> > 
> >  Add a single thread safe and multi-thread unsafe ring data structure.
> >  This library provides an simple and efficient alternative to
> >  multi- thread safe ring when multi-thread safety is not required.
> > 
> >  Signed-off-by: Honnappa Nagarahalli
> >  
> >  ---
> > >>>
> > >>> Good idea.
> > >>>
> > >>> However, I prefer it to be implemented in the ring lib as one more
> > >>> ring
> > >> type.
> > >>> That would also give us a lot of the infrastructure (management
> > >>> functions, documentation and tests) for free.
> > >> IMO, the current code for rte_ring seems complex with C11 and
> > >> generic implementations, APIs for pointer objects vs APIs for
> > >> flexible element size etc. I did not want to introduce one more flavor 
> > >> and
> make it more complex.
> > >
> > >  From the user perspective, I think one more ring flavor is less
> > > complex
> > than an entirely separate (very similar) library with its own set of
> > (very
> > similar) APIs.
> > >
> > > I agree that the ring lib has grown somewhat over-engineered, but
> > > please
> > don't use that as an argument for making the same-thread ring a separate
> lib.
> > >
> >
> > What's being proposed is a double-ended queue, not a ring (in the DPDK
> > sense).
> >
> > If you want to Swiss army knifify the rte_ring further and make it a
> > deque, then rte_stack should scrapped as well, since it's will become
> > just a subset of the new rte_ring_now_really_a_deque.
> 
> OK. I accept that argument for not hacking it into the ring lib.
> 
> Then I will suggest that the new "deque" library should be designed with
> multi-threading in mind, like its two sibling libs (ring and stack). This 
> makes it
> easier to use, and leaves it open for expansion to other flavors in the 
> future.
> 
> It is perfectly acceptable that the first version only supports the 
> same-thread
> deque flavor, and only the same-thread specialized APIs are exposed. I don't
> require any APIs or implementations supporting single-threaded (individual
> producer/consumer threads) or multi-threaded flavors, I only request that the
> design and API resemble those of its two sibling libraries. (And if there are 
> no
> use cases for multi-threading flavors, they might never be added to this lib.)
+1, will aim for this

> 
> >
> > > On the other hand: If the addition of an optimized same-thread ring
> > > flavor
> > would require too many invasive modifications of the existing ring
> > lib, I would accept that as an argument for not adding it as another
> > ring flavor to the existing ring lib.
> > >
> > >> The requirements are different as well. For ex: single thread ring
> > >> needs
> > APIs
> > >> for dequeuing and enqueuing at both ends of the ring which is not
> > applicable
> > >> to existing RTE ring.
> > >
> > > Yes, I will address this topic at the end of this mail.
> > >
> > >>
> > >> But, I see how the existing infra can be reused easily.
> > >
> > > This also goes for future infrastructure. I doubt that new
> > > infrastructure
> > added to the ring lib will also be added to the same-thread ring
> > lib... for reference, consider the PMDs containing copy-pasted code
> > from the mempool lib... none of the later improvements of the mempool
> > lib were implemented in those PMDs.
> > >
> > > In essence, I think this lib overlaps the existing ring lib too much
> > > to
> > justify making it a separate lib.
> > >
> > >>
> > >>>
> > >>> The ring lib already has performance-optimized APIs for
> > >>> single-consumer
> > and
> > >>> single-producer use, rte_ring_sc_dequeue_bulk() and
> > >>> rte_ring_sp_enqueue_burst(). Similar performance-optimized APIs
> > >>> for
> > single-
> > >>> thread use could be added: rte_ring_st_dequeue_bulk() and
> > >>> rte_ring_st_enqueue_burst().
> > >> Yes, the names look fine.
> > >> Looking through the code. We have the sync type enum:
> > >>
> > >> /** prod/cons sync types */
> > >> enum rte_ring_sync_type {
> > >>  RTE_RING_SYNC_MT, /**< multi-thread safe (default mode) */
> > >>  RTE_RING_SYNC_ST, /**< single thread only */
> > >>  RTE_RING_SYNC_MT_RTS, /**< multi-thread relaxed tail sync */
> > >>  RTE_RING_SYNC_MT_HTS, /**< multi-thread head/tail sync */
> > >> };
> > >>
> > >> The type RTE_RING_SYNC_ST needs better explanation (not a problem).
> > >> But,
> > this
> > >> name would have been ideal to use for single thread ring.
> > >> This enum does not need to be exposed to the