Re: [ovs-dev] [PATCH v3 5/6] dpif-netdev: Add per pmd sleep config.
On 13/07/2023 23:45, Ilya Maximets wrote: On 7/10/23 16:54, Kevin Traynor 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 Reviewed-by: David Marchand --- Documentation/topics/dpdk/pmd.rst | 23 +++ lib/dpif-netdev-private-thread.h | 3 + lib/dpif-netdev.c | 226 +++--- tests/pmd.at | 32 ++--- 4 files changed, 252 insertions(+), 32 deletions(-) Notes on this patch are a bit chaotic as they are mostly about the looks and perception. Hope they make some sense. :) No problem, thanks for reviewing it. I was able to understand them all. diff --git a/Documentation/topics/dpdk/pmd.rst b/Documentation/topics/dpdk/pmd.rst index 40e6b7843..7eb4eb7b3 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. 'max sleep request' doesn't sound like a human term. Maybe 'Maximum sleep values' or 'maximum sleep intervals' ? The reason I used it was because the request time is not a guarantee of the actual sleep time. So I was trying to be specific that it was the request in case someone wondered why it was taking a little longer in practice. Though as we've reduced it to typically just a few uS with the slack timer changes, perhaps it's not worth adding that detail and just keep it simple. will change to "Maximum sleep values". +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. What do you think about: s/PMD/PMD thread/ s/PMDs/PMD threads/ averywhere in the text above? Damn - I try to not just write "PMD" in general, but I can't help myself sometimes :-) + +For example, to set PMD threads on cores 8 and 9 to never request a load based +sleep and all others PMD threads to be able to request a max sleep of 50 usecs:: s/sleep of 50 usecs/maximum sleep of 50 microseconds (us)/ Ack + +$ 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:: Should we remove the previous mentioning of that command? No real need to have both so can remove the mention above. + +$ 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 to0 usecs. +PMD thread core 9 NUMA 1: Max sleep request set to0 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. A few note about the output: - Too much 'PMD', IMO. :) - The word 'request' has an unclear meaning and seems redundant. I agree it's maybe too low level. My fear was that a user misinterprets as the max sleep that has occurred. - 'by default' is a bit confusing. I see what you mean in context of this being a command - We do not use 'usecs' in other places. We use 'us' in pmd stats. oops, I should have been consistent with that What do you think about something like this: $ ovs-appctl dpif-netdev/pmd-sleep-show Default max sleep: 50 us pmd thread numa_id 0 core_id 8: max sleep:0 us pmd thread numa_id 1 core_id 9: max sleep:0 us pmd thread numa_id 0 core_id 10: max sleep: 50 us pmd thread numa_id 1 core_id 11: max sleep: 50 us pmd thread numa_id 0 core_id 12: max sleep: 50 us pmd thread numa_id 1 core_id 13: max sleep: 50 us --- ? Less words, the same header for numa/core as in other pmd-*-show commands. This format also allows extension in the future, if needed. Alternatively, we may degrade the output to a table: $ ovs-appctl dpif-netdev/pmd-sleep-show Default max sleep: 50 us core_id numa_id max
Re: [ovs-dev] [PATCH v3 5/6] dpif-netdev: Add per pmd sleep config.
On 7/10/23 16:54, Kevin Traynor 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
> Reviewed-by: David Marchand
> ---
> Documentation/topics/dpdk/pmd.rst | 23 +++
> lib/dpif-netdev-private-thread.h | 3 +
> lib/dpif-netdev.c | 226 +++---
> tests/pmd.at | 32 ++---
> 4 files changed, 252 insertions(+), 32 deletions(-)
Notes on this patch are a bit chaotic as they are mostly about the looks
and perception. Hope they make some sense. :)
>
> diff --git a/Documentation/topics/dpdk/pmd.rst
> b/Documentation/topics/dpdk/pmd.rst
> index 40e6b7843..7eb4eb7b3 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.
'max sleep request' doesn't sound like a human term.
Maybe 'Maximum sleep values' or 'maximum sleep intervals' ?
> +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.
What do you think about:
s/PMD/PMD thread/
s/PMDs/PMD threads/
averywhere in the text above?
> +
> +For example, to set PMD threads on cores 8 and 9 to never request a load
> based
> +sleep and all others PMD threads to be able to request a max sleep of 50
> usecs::
s/sleep of 50 usecs/maximum sleep of 50 microseconds (us)/
> +
> +$ 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::
Should we remove the previous mentioning of that command?
> +
> +$ 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 to0 usecs.
> +PMD thread core 9 NUMA 1: Max sleep request set to0 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.
A few note about the output:
- Too much 'PMD', IMO. :)
- The word 'request' has an unclear meaning and seems redundant.
- 'by default' is a bit confusing.
- We do not use 'usecs' in other places. We use 'us' in pmd stats.
What do you think about something like this:
$ ovs-appctl dpif-netdev/pmd-sleep-show
Default max sleep: 50 us
pmd thread numa_id 0 core_id 8:
max sleep:0 us
pmd thread numa_id 1 core_id 9:
max sleep:0 us
pmd thread numa_id 0 core_id 10:
max sleep: 50 us
pmd thread numa_id 1 core_id 11:
max sleep: 50 us
pmd thread numa_id 0 core_id 12:
max sleep: 50 us
pmd thread numa_id 1 core_id 13:
max sleep: 50 us
---
?
Less words, the same header for numa/core as in other pmd-*-show
commands. This format also allows extension in the future, if
needed.
Alternatively, we may degrade the output to a table:
$ ovs-appctl dpif-netdev/pmd-sleep-show
Default max sleep: 50 us
core_id numa_id max sleep
--- --- -
8 0 0 us
9 1 0 us
10 050 us
11 150 us
12 050 us
13 150 us
---
It's the most readable option, but least extensible.
> +
> .. _ovs-vswitchd(8):
> http://openvswitch.org/support/dist-docs/ovs-vswitchd.8.html
> diff --git a/lib/dpif-netdev-private-thread.h
> b/lib/dpif-netdev-private-thread.h
> index 1ec3cd794..5c72ce5d9 100644
> --- a/lib/dpif-netdev-private-thread.h
> +++ b/lib/dpif-netdev-private-thread.h
> @@ -181,4 +181,7 @@ struct dp_netdev_pmd_thread {
> bool isolated;
>
> +/* Max sleep request. UINT64_MAX indicates dp default should be used.*/
Can we just store a value here directly after parsing without
fallback logic?
> +atomic_uint64_t
Re: [ovs-dev] [PATCH v3 5/6] dpif-netdev: Add per pmd sleep config.
Bleep bloop. Greetings Kevin Traynor, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 80 characters long (recommended limit is 79) #50 FILE: Documentation/topics/dpdk/pmd.rst:384: sleep and all others PMD threads to be able to request a max sleep of 50 usecs:: Lines checked: 497, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 5/6] dpif-netdev: Add per pmd sleep config.
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
Reviewed-by: David Marchand
---
Documentation/topics/dpdk/pmd.rst | 23 +++
lib/dpif-netdev-private-thread.h | 3 +
lib/dpif-netdev.c | 226 +++---
tests/pmd.at | 32 ++---
4 files changed, 252 insertions(+), 32 deletions(-)
diff --git a/Documentation/topics/dpdk/pmd.rst
b/Documentation/topics/dpdk/pmd.rst
index 40e6b7843..7eb4eb7b3 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 threads on cores 8 and 9 to never request a load based
+sleep and all others PMD threads 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
+
+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 to0 usecs.
+PMD thread core 9 NUMA 1: Max sleep request set to0 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
diff --git a/lib/dpif-netdev-private-thread.h b/lib/dpif-netdev-private-thread.h
index 1ec3cd794..5c72ce5d9 100644
--- a/lib/dpif-netdev-private-thread.h
+++ b/lib/dpif-netdev-private-thread.h
@@ -181,4 +181,7 @@ struct dp_netdev_pmd_thread {
bool isolated;
+/* Max sleep request. UINT64_MAX indicates dp default should be used.*/
+atomic_uint64_t max_sleep;
+
/* Queue id used by this pmd thread to send packets on all netdevs if
* XPS disabled for this netdev. All static_tx_qid's are unique and less
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 522253c82..0e79705d8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -180,4 +180,9 @@ static struct odp_support dp_netdev_support = {
#define PMD_SLEEP_INC_US 1
+struct pmd_sleep {
+unsigned core_id;
+uint64_t max_sleep;
+};
+
struct dpcls {
struct cmap_node node; /* Within dp_netdev_pmd_thread.classifiers */
@@ -290,4 +295,6 @@ struct dp_netdev {
/* Max load based sleep request. */
atomic_uint64_t pmd_max_sleep;
+/* Max load based sleep request user string. */
+char *max_sleep_list;
/* Enable the SMC cache from ovsdb config */
atomic_bool smc_enable_db;
@@ -1013,9 +1020,15 @@ pmd_max_sleep_show(struct ds *reply, struct
dp_netdev_pmd_thread *pmd,
{
if (pmd->core_id != NON_PMD_CORE_ID) {
+uint64_t pmd_max_sleep;
+
+atomic_read_relaxed(>max_sleep, _max_sleep);
ds_put_format(reply,
"PMD thread core %3u NUMA %2d: "
"Max sleep request set to",
pmd->core_id, pmd->numa_id);
-ds_put_format(reply, " %4"PRIu64" usecs.", default_max_sleep);
+ds_put_format(reply, " %4"PRIu64" usecs.",
+ pmd_max_sleep == UINT64_MAX
+ ? default_max_sleep
+ : pmd_max_sleep);
ds_put_cstr(reply, "\n");
}
@@ -1528,7 +1541,8 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc,
const char *argv[],
atomic_read_relaxed(>pmd_max_sleep, _max_sleep);
ds_put_format(, "PMD max sleep request is %"PRIu64" "
- "usecs.", default_max_sleep);
+ "usecs by default.", default_max_sleep);
