On Wed, Jun 21, 2023 at 11:26 AM Kevin Traynor <ktray...@redhat.com> 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 <ktray...@redhat.com>

Reviewed-by: David Marchand <david.march...@redhat.com>

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)


> +
> +    $ 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".


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

[snip]


-- 
David Marchand

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to