diff mbox series

[ovs-dev] utilities: taskset for non-pmd threads

Message ID 20230421130718.95794-1-wanjunjie@bytedance.com
State Rejected
Headers show
Series [ovs-dev] utilities: taskset for non-pmd threads | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Wan Junjie April 21, 2023, 1:07 p.m. UTC 
Deployment with dpdk and spdk should limit the core of non-pmd
threads. When starting ovs with ovs-ctl or systemd, the process will
inherit the affinity of its parent process. Even if we manuanlly
set the affinity, restarting after crash will lose the setting.

Prepend taskset in ovs-ctl when start daemon, so it will set the
affinity each time starting the ovs.

Signed-off-by: Wan Junjie <wanjunjie@bytedance.com>
---
 Documentation/ref/ovs-ctl.8.rst | 5 +++++
 utilities/ovs-ctl.in            | 8 ++++++--
 utilities/ovs-lib.in            | 6 ++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

Comments

Ilya Maximets April 26, 2023, 8:19 a.m. UTC | #1 
On 4/21/23 15:07, Wan Junjie via dev wrote:
> Deployment with dpdk and spdk should limit the core of non-pmd
> threads. When starting ovs with ovs-ctl or systemd, the process will
> inherit the affinity of its parent process. Even if we manuanlly
> set the affinity, restarting after crash will lose the setting.
> 
> Prepend taskset in ovs-ctl when start daemon, so it will set the
> affinity each time starting the ovs.

Hi,

systemd has various ways of specifying CPU affinity and resource limits
globally and for a specific service.  Services like tuned can also be
used for that purpose.  Is it not enough for your use case?

Best regards, Ilya Maximets.
Wan Junjie April 26, 2023, 9:51 a.m. UTC | #2 
On Wed, Apr 26, 2023 at 4:19 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 4/21/23 15:07, Wan Junjie via dev wrote:
> > Deployment with dpdk and spdk should limit the core of non-pmd
> > threads. When starting ovs with ovs-ctl or systemd, the process will
> > inherit the affinity of its parent process. Even if we manuanlly
> > set the affinity, restarting after crash will lose the setting.
> >
> > Prepend taskset in ovs-ctl when start daemon, so it will set the
> > affinity each time starting the ovs.
>
> Hi,
>
> systemd has various ways of specifying CPU affinity and resource limits
> globally and for a specific service.  Services like tuned can also be
> used for that purpose.  Is it not enough for your use case?
>
> Best regards, Ilya Maximets.

Hi Ilya,

Thanks for your kind reminder. Didn't notice that before.
Yes, systemd service affinity works for my case.
Please ignore this patch. Sorry to have taken up your time.

Regards
Wan Junjie
diff mbox series

Patch

diff --git a/Documentation/ref/ovs-ctl.8.rst b/Documentation/ref/ovs-ctl.8.rst
index 9f077a122..603e9ca1e 100644
--- a/Documentation/ref/ovs-ctl.8.rst
+++ b/Documentation/ref/ovs-ctl.8.rst
@@ -199,6 +199,11 @@  The following options are less important:
   Sets the ``nice(1)`` level used for each daemon.  All of them
   default to ``-10``.
 
+* ``--ovsdb-server-affinity=<corelist>`` or
+  ``--ovs-vswitchd-affinity=<corelist>``
+
+  Sets the ``taskset(1)`` affinity for each daemon process.
+
 * ``--ovsdb-server-wrapper=<wrapper>`` or
   ``--ovs-vswitchd-wrapper=<wrapper>``
 
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 0b2820c36..3f033a98e 100644
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -157,7 +157,7 @@  do_start_ovsdb () {
         [ "$OVSDB_SERVER_OPTIONS" != "" ] && set "$@" $OVSDB_SERVER_OPTIONS
 
         start_daemon "$OVSDB_SERVER_PRIORITY" "$OVSDB_SERVER_WRAPPER" \
-            "$OVSDB_SERVER_UMASK" "$@" || return 1
+            "$OVSDB_SERVER_UMASK" "$OVSDB_SERVER_AFFINITY" "$@" || return 1
 
         # Initialize database settings.
         ovs_vsctl -- init -- set Open_vSwitch . db-version="$schemaver" \
@@ -227,7 +227,7 @@  do_start_forwarding () {
         [ "$OVS_VSWITCHD_OPTIONS" != "" ] &&set "$@" $OVS_VSWITCHD_OPTIONS
 
         start_daemon "$OVS_VSWITCHD_PRIORITY" "$OVS_VSWITCHD_WRAPPER" \
-            "$OVS_VSWITCHD_UMASK" "$@" || return 1
+            "$OVS_VSWITCHD_UMASK" "$OVS_VSWITCHD_AFFINITY" "$@" || return 1
     fi
 }
 
@@ -344,6 +344,8 @@  set_defaults () {
     OVS_VSWITCHD=yes
     OVSDB_SERVER_PRIORITY=-10
     OVS_VSWITCHD_PRIORITY=-10
+    OVSDB_SERVER_AFFINITY=
+    OVS_VSWITCHD_AFFINITY=
     OVSDB_SERVER_WRAPPER=
     OVS_VSWITCHD_WRAPPER=
     OVSDB_SERVER_OPTIONS=
@@ -436,8 +438,10 @@  Less important options for "start", "restart" and "force-reload-kmod":
   --dump-hugepages               include hugepages in core dumps
   --no-mlockall                  do not lock all of ovs-vswitchd into memory
   --ovsdb-server-priority=NICE   set ovsdb-server's niceness (default: $OVSDB_SERVER_PRIORITY)
+  --ovsdb-server-affinity=CORE   set ovsdb-server's affinity (default: $OVSDB_SERVER_AFFINITY)
   --ovsdb-server-options=OPTIONS additional options for ovsdb-server (example: '-vconsole:dbg -vfile:dbg')
   --ovs-vswitchd-priority=NICE   set ovs-vswitchd's niceness (default: $OVS_VSWITCHD_PRIORITY)
+  --ovs-vswitchd-affinity=CORE   set ovs-vswitchd's affinity (default: $OVS_VSWITCHD_AFFINITY)
   --ovs-vswitchd-options=OPTIONS additional options for ovs-vswitchd (example: '-vconsole:dbg -vfile:dbg')
   --no-full-hostname             set short hostname instead of full hostname
   --no-record-hostname           do not attempt to determine/record system
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 7812a94ee..f48d62376 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -168,6 +168,7 @@  start_daemon () {
     priority=$1 && shift
     wrapper=$1 && shift
     umask=$1 && shift
+    affinity=$1 && shift
     daemon=$1
     strace=""
 
@@ -223,6 +224,11 @@  start_daemon () {
         set nice -n "$priority" "$@"
     fi
 
+    # affinity
+    if test X"$affinity" != X; then
+        set taskset -c "$affinity" "$@"
+    fi
+
     # Set requested umask if any and turn previous value back.
     if [ -n "$umask" ]; then
         previuos_umask_value=$(umask)