On 07/07/2023 14:22, David Marchand wrote:
On Wed, Jun 21, 2023 at 11:26 AM Kevin Traynor <[email protected]> wrote:

Extend 'pmd-sleep-max' so that individual PMD thread cores
may have a specified max sleep request value.

Any PMD thread core without a value will use the datapath default
(no sleep request) or datapath global value set by the user.

To set PMD thread cores 8 and 9 to never request a load based sleep
and all other PMD thread cores to be able to request a max sleep of
50 usecs:

$ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=50,8:0,9:0

To set PMD thread cores 10 and 11 to request a max sleep of 100 usecs
and all other PMD thread cores to never request a sleep:

$ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=10:100,11:100

'pmd-sleep-show' can be used to dump the global and individual PMD thread
core max sleep request values.

Signed-off-by: Kevin Traynor <[email protected]>

Reviewed-by: David Marchand <[email protected]>

I have some nits below which can be fixed when applying.
But I am ok too if we go with the current patch.



diff --git a/Documentation/topics/dpdk/pmd.rst 
b/Documentation/topics/dpdk/pmd.rst
index 40e6b7843..eafcbc504 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -375,4 +375,27 @@ system configuration (e.g. enabling processor C-states) 
and workloads.
      rate.

+Max sleep request values can be set for individual PMDs using key:value pairs.
+Any PMD that has been assigned a specified value will use that. Any PMD that
+does not have a specified value will use the current global default.
+
+Specified values for individual PMDs can be added or removed at any time.
+
+For example, to set PMD thread cores 8 and 9 to never request a load based

Nit: in the dpif-netdev/pmd-sleep-show command output, "PMD thread
core X" is short, and understandable.

But for descriptions in the documentation, "PMD thread cores" is strange.
We didn't use such denomination so far in the doc.

I would go with "PMD on cores 8 and 9" (or PMD threads on cores 8 and 9).

+sleep and all others PMD cores to be able to request a max sleep of 50 usecs::

Nit: Idem, PMD (or PMD threads)



That makes sense - I agree, this is documentation so there is no need to abbreviate. I changed to your "PMD threads" style suggestions.


+
+    $ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=50,8:0,9:0
+
+The max sleep request for each PMD can be checked in the logs or with::
+
+    $ ovs-appctl dpif-netdev/pmd-sleep-show
+    PMD max sleep request is 50 usecs by default.
+    PMD load based sleeps are enabled by default.
+    PMD thread core   8 NUMA  0: Max sleep request set to    0 usecs.
+    PMD thread core   9 NUMA  1: Max sleep request set to    0 usecs.
+    PMD thread core  10 NUMA  0: Max sleep request set to   50 usecs.
+    PMD thread core  11 NUMA  1: Max sleep request set to   50 usecs.
+    PMD thread core  12 NUMA  0: Max sleep request set to   50 usecs.
+    PMD thread core  13 NUMA  1: Max sleep request set to   50 usecs.
+
  .. _ovs-vswitchd(8):
      http://openvswitch.org/support/dist-docs/ovs-vswitchd.8.html

[snip]

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 88d25f1da..d9ee53ff5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c

[snip]

@@ -5065,4 +5077,182 @@ parse_affinity_list(const char *affinity_list, unsigned 
*core_ids, int n_rxq)
  }

+static int
+parse_pmd_sleep_list(const char *max_sleep_list,
+                     struct pmd_sleep **pmd_sleeps)
+{
+    char *list, *copy, *key, *value;
+    int num_vals = 0;
+
+    if (!max_sleep_list) {
+        return num_vals;
+    }
+
+    list = copy = xstrdup(max_sleep_list);
+
+    while (ofputil_parse_key_value(&list, &key, &value)) {
+        char *error = NULL;
+        unsigned core;
+        uint64_t temp, pmd_max_sleep;
+        int i;
+
+        error = str_to_u64(key, &temp);
+        if (error) {
+            free(error);
+            continue;
+        }
+
+        error = str_to_u64(value, &pmd_max_sleep);
+        if (error) {
+            /* No value specified. key is dp default. */
+            core = UINT_MAX;
+            pmd_max_sleep = temp;
+            free(error);
+        } else {
+            /* Value specified. key is  pmd core id.*/

Nit: extra space between "is pmd".


Fixed in v3.

Thanks for your review and comments on both versions.


+            if (temp >= UINT_MAX) {
+                continue;
+            }
+            core = (unsigned) temp;
+        }
+

[snip]



_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to