Message ID | 1471274386-50924-1-git-send-email-bhanuprakash.bodireddy@intel.com |
---|---|
State | Superseded |
Delegated to: | Daniele Di Proietto |
Headers | show |
I found a crash if apply this patch, "dpdk-lcore-mask" is not set and "-c 0x1" is passed to "dpdk-extra". Also, I believe Flavio had a comment on the previous version of this patch. Would it be enough to use setpriority(2)? Thanks, Daniele 2016-08-15 8:19 GMT-07:00 Bhanuprakash Bodireddy < bhanuprakash.bodireddy@intel.com>: > Set the DPDK pmd thread scheduling policy to SCHED_RR and static > priority to highest priority value of the policy. This is to deal with > pmd thread starvation case where another cpu hogging process can get > scheduled/affinitized on to the same core the pmd thread is running > there by significantly impacting the datapath performance. To reproduce > the pmd thread starvation case: > > $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6 > $ taskset 0x2 cat /dev/zero > /dev/null & > > Setting the realtime scheduling policy to the pmd threads is one step > towards Fastpath Service Assurance in OVS DPDK. > > The realtime scheduling policy is applied only when CPU mask is passed > to 'pmd-cpu-mask'. For example: > > * In the absence of pmd-cpu-mask, one pmd thread shall be created > and default scheduling policy and priority gets applied. > > * If pmd-cpu-mask is specified, one or more pmd threads shall be > spawned on the corresponding core(s) in the mask and real time > scheduling policy SCHED_RR and highest priority of the policy is > applied to the pmd thread(s). > > With this commit, it is recommended that the OVS control thread and pmd > thread shouldn't be pinned to same core ('dpdk-lcore-mask','pmd-cpu-mask' > should be non-overlapping). If dpdk-lcore-mask is set same as pmd-cpu-mask > the pmd thread is not spawned on lowest core of the dpdk-lcore-mask. > Also other processes with same affinity as PMD thread will be unresponsive. > > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> > --- > v5->v6: > * Prohibit spawning pmd thread on the lowest core in dpdk-lcore-mask if > lcore-mask and pmd-mask affinity are identical. > * Updated Note section in INSTALL.DPDK-ADVANCED doc. > * Tested below cases to verify system stability with pmd priority patch > > dpdk-lcore-mask | pmd-cpu-mask | Comment > 1. Not set | Not set | control threads affinity: 0-27 > pmd thread: core 0 > 2. 1 | 1 | pmd thread isn't spawned and warning > logged in logfile. > 3. 1 | c | > 4. F0 | F0 | control threads pinned to core 4. > 3 pmd threads created on core 5,6,7 > but 4. > > v4->v5: > * Reword Note section in DPDK-ADVANCED.md > > v3->v4: > * Document update > * Use ovs_strerror for reporting errors in lib-numa.c > > v2->v3: > * Move set_priority() function to lib/ovs-numa.c > * Apply realtime scheduling policy and priority to pmd thread only if > pmd-cpu-mask is passed. > * Update INSTALL.DPDK-ADVANCED. > > v1->v2: > * Removed #ifdef and introduced dummy function "pmd_thread_setpriority" > in netdev-dpdk.h > * Rebase > > INSTALL.DPDK-ADVANCED.md | 24 ++++++++++++++++++++---- > lib/dpif-netdev.c | 12 ++++++++++++ > lib/netdev-dpdk.c | 22 ++++++++++++++++++++++ > lib/ovs-numa.c | 18 ++++++++++++++++++ > lib/ovs-numa.h | 1 + > 5 files changed, 73 insertions(+), 4 deletions(-) > > diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md > index dd36ae4..f039723 100755 > --- a/INSTALL.DPDK-ADVANCED.md > +++ b/INSTALL.DPDK-ADVANCED.md > @@ -208,8 +208,10 @@ needs to be affinitized accordingly. > pmd thread is CPU bound, and needs to be affinitized to isolated > cores for optimum performance. > > - By setting a bit in the mask, a pmd thread is created and pinned > - to the corresponding CPU core. e.g. to run a pmd thread on core 2 > + By setting a bit in the mask, a pmd thread is created, pinned > + to the corresponding CPU core and the scheduling policy SCHED_RR > + along with maximum priority of the policy applied to the pmd thread. > + e.g. to pin a pmd thread on core 2 > > `ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=4` > > @@ -237,8 +239,10 @@ needs to be affinitized accordingly. > responsible for different ports/rxq's. Assignment of ports/rxq's to > pmd threads is done automatically. > > - A set bit in the mask means a pmd thread is created and pinned > - to the corresponding CPU core. e.g. to run pmd threads on core 1 and 2 > + A set bit in the mask means a pmd thread is created, pinned to the > + corresponding CPU core and the scheduling policy SCHED_RR with highest > + priority of the scheduling policy applied to pmd thread. > + e.g. to run pmd threads on core 1 and 2 > > `ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6` > > @@ -249,6 +253,18 @@ needs to be affinitized accordingly. > > NIC port0 <-> OVS <-> VM <-> OVS <-> NIC port 1 > > + Note: 'dpdk-lcore-mask' and 'pmd-cpu-mask' cpu mask settings should be > + non-overlapping. If you set dpdk-lcore-mask same as pmd-cpu-mask, the > pmd > + thread is not spawned on lowest core of the dpdk-lcore-mask and in case > of > + cpu mask setting where both the control and the pmd thread pinned to > same > + core, pmd thread is not spawned at all if it is the only cpu in the > affinity > + mask. > + > + For example, if dpdk-lcore-mask=F0 and pmd-cpu-mask=F0, pmd threads are > + created on cores 5, 6, and 7 but not on 4. Setting both lcore,pmd mask > to > + core 4 shall pin control threads to core 4 but will prevent pmd thread > from > + spawning. > + > ### 4.3 DPDK physical port Rx Queues > > `ovs-vsctl set Interface <DPDK interface> options:n_rxq=<integer>` > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 96504f5..abe1fde 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3100,6 +3100,18 @@ pmd_thread_main(void *f_) > ovs_numa_thread_setaffinity_core(pmd->core_id); > dpdk_set_lcore_id(pmd->core_id); > poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list); > + > + /* When cpu affinity mask explicitly set using pmd-cpu-mask, pmd > thread's > + * scheduling policy is set to SCHED_RR and priority to highest > priority > + * of SCHED_RR policy. In the absence of pmd-cpu-mask, default > scheduling > + * policy and priority shall apply to pmd thread. > + * > + * If dpdk-lcore-mask is set same as pmd-cpu-mask, the pmd thread is > not > + * spawned on lowest core of the dpdk-lcore-mask. > + */ > + if (pmd->dp->pmd_cmask) { > + ovs_numa_thread_setpriority(SCHED_RR); > + } > reload: > emc_cache_init(&pmd->flow_cache); > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index c767fd4..5d606a2 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -3432,6 +3432,28 @@ dpdk_init__(const struct smap *ovs_other_config) > } > } > > + /* If dpdk-lcore-mask is explicitly specified, auto_determine is > 'false' > + * and control threads are pinned to lowest core of the affinity mask. > + * Mark the corresponding control thread 'core->pinned' status to > 'true' > + * so that pmd thread can't be spawned on the same core. > + */ > + if (!auto_determine) { > + const char *lcore_mask; > + unsigned int core_id; > + int dl_mask; > + > + lcore_mask = smap_get(ovs_other_config, "dpdk-lcore-mask"); > + dl_mask = strtol(lcore_mask, NULL, 16); > + > + /* Get the lowest core_id of dpdk-lcore-mask */ > + core_id = rightmost_1bit_idx(dl_mask); > + > + /* Set the control thread core 'pinned' status to true */ > + if (!ovs_numa_try_pin_core_specific(core_id)) { > + VLOG_ERR("Unable to pin core %d", core_id); > + } > + } > + > dpdk_argv = argv; > dpdk_argc = argc; > > diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c > index c8173e0..428f274 100644 > --- a/lib/ovs-numa.c > +++ b/lib/ovs-numa.c > @@ -613,3 +613,21 @@ int ovs_numa_thread_setaffinity_core(unsigned > core_id OVS_UNUSED) > return EOPNOTSUPP; > #endif /* __linux__ */ > } > + > +void > +ovs_numa_thread_setpriority(int policy) > +{ > + if (dummy_numa) { > + return; > + } > + > + struct sched_param threadparam; > + int err; > + > + memset(&threadparam, 0, sizeof(threadparam)); > + threadparam.sched_priority = sched_get_priority_max(policy); > + err = pthread_setschedparam(pthread_self(), policy, &threadparam); > + if (err) { > + VLOG_ERR("Thread priority error %s",ovs_strerror(err)); > + } > +} > diff --git a/lib/ovs-numa.h b/lib/ovs-numa.h > index be836b2..94f0884 100644 > --- a/lib/ovs-numa.h > +++ b/lib/ovs-numa.h > @@ -56,6 +56,7 @@ void ovs_numa_unpin_core(unsigned core_id); > struct ovs_numa_dump *ovs_numa_dump_cores_on_numa(int numa_id); > void ovs_numa_dump_destroy(struct ovs_numa_dump *); > int ovs_numa_thread_setaffinity_core(unsigned core_id); > +void ovs_numa_thread_setpriority(int policy); > > #define FOR_EACH_CORE_ON_NUMA(ITER, DUMP) \ > LIST_FOR_EACH((ITER), list_node, &(DUMP)->dump) > -- > 2.4.11 > >
>-----Original Message----- >From: Daniele Di Proietto [mailto:diproiettod@ovn.org] >Sent: Tuesday, August 16, 2016 1:44 AM >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >Cc: dev@openvswitch.org; Flavio Leitner <fbl@sysclose.org> >Subject: Re: [PATCH V6] netdev-dpdk: Set pmd thread priority > >I found a crash if apply this patch, "dpdk-lcore-mask" is not set and "-c 0x1" is >passed to "dpdk-extra". My bad, I didn't test with dpdk-extra options. I see that the crash was due to strtol. >Also, I believe Flavio had a comment on the previous version of this >patch. Would it be enough to use setpriority(2)? I sent out my comments in another mail. I agree to Flavio's suggestion as this seems less dangerous and is guaranteed to work even in case of misconfiguration. I tested this approach and have a concern with setpriority(). To apply the new nice value to the thread, thread id is needed and due to absence of glibc wrapper for gettid, I have to use syscall(SYS_gettid). I want to know if this is acceptable in OVS or better way to handle this? Void ovs_numa_thread_setpriority(int nice OVS_UNUSED) { .... #if defined(__linux__) && defined(SYS_gettid) pid_t tid = syscall(SYS_gettid); err = setpriority(PRIO_PROCESS, tid, nice); .... #endif } Without priority patch: $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e pmd 22509 19 4 ovs-vswitchd 22512 19 5 lcore-slave-5 22513 19 6 lcore-slave-6 22514 19 7 lcore-slave-7 22589 19 4 revalidator37 22590 19 4 revalidator52 22591 19 4 revalidator42 22592 19 4 revalidator38 22593 19 4 revalidator39 22594 19 4 revalidator45 22595 19 4 revalidator53 22596 19 4 revalidator54 22598 19 4 pmd61 [Default priority] With priority patch: $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e pmd 24879 19 4 ovs-vswitchd 24881 19 5 lcore-slave-5 24882 19 6 lcore-slave-6 24883 19 7 lcore-slave-7 24951 19 4 revalidator55 24952 19 4 revalidator37 24953 19 4 revalidator52 24954 19 4 revalidator42 24955 19 4 revalidator38 24956 19 4 revalidator39 24957 19 4 revalidator45 24958 19 4 revalidator53 24964 39 4 pmd61 [Higher priority set] Regards, Bhanu Prakash. >Thanks, >Daniele > >2016-08-15 8:19 GMT-07:00 Bhanuprakash Bodireddy ><bhanuprakash.bodireddy@intel.com>: >Set the DPDK pmd thread scheduling policy to SCHED_RR and static >priority to highest priority value of the policy. This is to deal with >pmd thread starvation case where another cpu hogging process can get >scheduled/affinitized on to the same core the pmd thread is running >there by significantly impacting the datapath performance. To reproduce >the pmd thread starvation case: > > $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6 > $ taskset 0x2 cat /dev/zero > /dev/null & > >Setting the realtime scheduling policy to the pmd threads is one step >towards Fastpath Service Assurance in OVS DPDK. > >The realtime scheduling policy is applied only when CPU mask is passed >to 'pmd-cpu-mask'. For example: > > * In the absence of pmd-cpu-mask, one pmd thread shall be created > and default scheduling policy and priority gets applied. > > * If pmd-cpu-mask is specified, one or more pmd threads shall be > spawned on the corresponding core(s) in the mask and real time > scheduling policy SCHED_RR and highest priority of the policy is > applied to the pmd thread(s). > >With this commit, it is recommended that the OVS control thread and pmd >thread shouldn't be pinned to same core ('dpdk-lcore-mask','pmd-cpu-mask' >should be non-overlapping). If dpdk-lcore-mask is set same as pmd-cpu-mask >the pmd thread is not spawned on lowest core of the dpdk-lcore-mask. >Also other processes with same affinity as PMD thread will be unresponsive. > >Signed-off-by: Bhanuprakash Bodireddy ><bhanuprakash.bodireddy@intel.com> >--- >v5->v6: >* Prohibit spawning pmd thread on the lowest core in dpdk-lcore-mask if > lcore-mask and pmd-mask affinity are identical. >* Updated Note section in INSTALL.DPDK-ADVANCED doc. >* Tested below cases to verify system stability with pmd priority patch > > dpdk-lcore-mask | pmd-cpu-mask | Comment >1. Not set | Not set | control threads affinity: 0-27 > pmd thread: core 0 >2. 1 | 1 | pmd thread isn't spawned and warning > logged in logfile. >3. 1 | c | >4. F0 | F0 | control threads pinned to core 4. > 3 pmd threads created on core 5,6,7 but 4. > >v4->v5: >* Reword Note section in DPDK-ADVANCED.md > >v3->v4: >* Document update >* Use ovs_strerror for reporting errors in lib-numa.c > >v2->v3: >* Move set_priority() function to lib/ovs-numa.c >* Apply realtime scheduling policy and priority to pmd thread only if > pmd-cpu-mask is passed. >* Update INSTALL.DPDK-ADVANCED. > >v1->v2: >* Removed #ifdef and introduced dummy function "pmd_thread_setpriority" > in netdev-dpdk.h >* Rebase > > INSTALL.DPDK-ADVANCED.md | 24 ++++++++++++++++++++---- > lib/dpif-netdev.c | 12 ++++++++++++ > lib/netdev-dpdk.c | 22 ++++++++++++++++++++++ > lib/ovs-numa.c | 18 ++++++++++++++++++ > lib/ovs-numa.h | 1 + > 5 files changed, 73 insertions(+), 4 deletions(-) > >diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md >index dd36ae4..f039723 100755 >--- a/INSTALL.DPDK-ADVANCED.md >+++ b/INSTALL.DPDK-ADVANCED.md >@@ -208,8 +208,10 @@ needs to be affinitized accordingly. > pmd thread is CPU bound, and needs to be affinitized to isolated > cores for optimum performance. > >- By setting a bit in the mask, a pmd thread is created and pinned >- to the corresponding CPU core. e.g. to run a pmd thread on core 2 >+ By setting a bit in the mask, a pmd thread is created, pinned >+ to the corresponding CPU core and the scheduling policy SCHED_RR >+ along with maximum priority of the policy applied to the pmd thread. >+ e.g. to pin a pmd thread on core 2 > > `ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=4` > >@@ -237,8 +239,10 @@ needs to be affinitized accordingly. > responsible for different ports/rxq's. Assignment of ports/rxq's to > pmd threads is done automatically. > >- A set bit in the mask means a pmd thread is created and pinned >- to the corresponding CPU core. e.g. to run pmd threads on core 1 and 2 >+ A set bit in the mask means a pmd thread is created, pinned to the >+ corresponding CPU core and the scheduling policy SCHED_RR with highest >+ priority of the scheduling policy applied to pmd thread. >+ e.g. to run pmd threads on core 1 and 2 > > `ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6` > >@@ -249,6 +253,18 @@ needs to be affinitized accordingly. > > NIC port0 <-> OVS <-> VM <-> OVS <-> NIC port 1 > >+ Note: 'dpdk-lcore-mask' and 'pmd-cpu-mask' cpu mask settings should be >+ non-overlapping. If you set dpdk-lcore-mask same as pmd-cpu-mask, the >pmd >+ thread is not spawned on lowest core of the dpdk-lcore-mask and in case of >+ cpu mask setting where both the control and the pmd thread pinned to >same >+ core, pmd thread is not spawned at all if it is the only cpu in the affinity >+ mask. >+ >+ For example, if dpdk-lcore-mask=F0 and pmd-cpu-mask=F0, pmd threads >are >+ created on cores 5, 6, and 7 but not on 4. Setting both lcore,pmd mask to >+ core 4 shall pin control threads to core 4 but will prevent pmd thread from >+ spawning. >+ > ### 4.3 DPDK physical port Rx Queues > > `ovs-vsctl set Interface <DPDK interface> options:n_rxq=<integer>` >diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >index 96504f5..abe1fde 100644 >--- a/lib/dpif-netdev.c >+++ b/lib/dpif-netdev.c >@@ -3100,6 +3100,18 @@ pmd_thread_main(void *f_) > ovs_numa_thread_setaffinity_core(pmd->core_id); > dpdk_set_lcore_id(pmd->core_id); > poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list); >+ >+ /* When cpu affinity mask explicitly set using pmd-cpu-mask, pmd thread's >+ * scheduling policy is set to SCHED_RR and priority to highest priority >+ * of SCHED_RR policy. In the absence of pmd-cpu-mask, default >scheduling >+ * policy and priority shall apply to pmd thread. >+ * >+ * If dpdk-lcore-mask is set same as pmd-cpu-mask, the pmd thread is not >+ * spawned on lowest core of the dpdk-lcore-mask. >+ */ >+ if (pmd->dp->pmd_cmask) { >+ ovs_numa_thread_setpriority(SCHED_RR); >+ } > reload: > emc_cache_init(&pmd->flow_cache); > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >index c767fd4..5d606a2 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -3432,6 +3432,28 @@ dpdk_init__(const struct smap *ovs_other_config) > } > } > >+ /* If dpdk-lcore-mask is explicitly specified, auto_determine is 'false' >+ * and control threads are pinned to lowest core of the affinity mask. >+ * Mark the corresponding control thread 'core->pinned' status to 'true' >+ * so that pmd thread can't be spawned on the same core. >+ */ >+ if (!auto_determine) { >+ const char *lcore_mask; >+ unsigned int core_id; >+ int dl_mask; >+ >+ lcore_mask = smap_get(ovs_other_config, "dpdk-lcore-mask"); >+ dl_mask = strtol(lcore_mask, NULL, 16); >+ >+ /* Get the lowest core_id of dpdk-lcore-mask */ >+ core_id = rightmost_1bit_idx(dl_mask); >+ >+ /* Set the control thread core 'pinned' status to true */ >+ if (!ovs_numa_try_pin_core_specific(core_id)) { >+ VLOG_ERR("Unable to pin core %d", core_id); >+ } >+ } >+ > dpdk_argv = argv; > dpdk_argc = argc; > >diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c >index c8173e0..428f274 100644 >--- a/lib/ovs-numa.c >+++ b/lib/ovs-numa.c >@@ -613,3 +613,21 @@ int ovs_numa_thread_setaffinity_core(unsigned >core_id OVS_UNUSED) > return EOPNOTSUPP; > #endif /* __linux__ */ > } >+ >+void >+ovs_numa_thread_setpriority(int policy) >+{ >+ if (dummy_numa) { >+ return; >+ } >+ >+ struct sched_param threadparam; >+ int err; >+ >+ memset(&threadparam, 0, sizeof(threadparam)); >+ threadparam.sched_priority = sched_get_priority_max(policy); >+ err = pthread_setschedparam(pthread_self(), policy, &threadparam); >+ if (err) { >+ VLOG_ERR("Thread priority error %s",ovs_strerror(err)); >+ } >+} >diff --git a/lib/ovs-numa.h b/lib/ovs-numa.h >index be836b2..94f0884 100644 >--- a/lib/ovs-numa.h >+++ b/lib/ovs-numa.h >@@ -56,6 +56,7 @@ void ovs_numa_unpin_core(unsigned core_id); > struct ovs_numa_dump *ovs_numa_dump_cores_on_numa(int numa_id); > void ovs_numa_dump_destroy(struct ovs_numa_dump *); > int ovs_numa_thread_setaffinity_core(unsigned core_id); >+void ovs_numa_thread_setpriority(int policy); > > #define FOR_EACH_CORE_ON_NUMA(ITER, DUMP) \ > LIST_FOR_EACH((ITER), list_node, &(DUMP)->dump) >-- >2.4.11
On Tue, Aug 16, 2016 at 02:30:04PM +0000, Bodireddy, Bhanuprakash wrote: > >-----Original Message----- > >From: Daniele Di Proietto [mailto:diproiettod@ovn.org] > >Sent: Tuesday, August 16, 2016 1:44 AM > >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> > >Cc: dev@openvswitch.org; Flavio Leitner <fbl@sysclose.org> > >Subject: Re: [PATCH V6] netdev-dpdk: Set pmd thread priority > > > >I found a crash if apply this patch, "dpdk-lcore-mask" is not set and "-c 0x1" is > >passed to "dpdk-extra". > My bad, I didn't test with dpdk-extra options. I see that the crash was due to strtol. > > >Also, I believe Flavio had a comment on the previous version of this > >patch. Would it be enough to use setpriority(2)? > I sent out my comments in another mail. I agree to Flavio's suggestion as this seems less dangerous and > is guaranteed to work even in case of misconfiguration. I tested this approach and have a concern with > setpriority(). > > To apply the new nice value to the thread, thread id is needed and due to absence of glibc wrapper > for gettid, I have to use syscall(SYS_gettid). I want to know if this is acceptable in OVS or better way to handle this? > > Void ovs_numa_thread_setpriority(int nice OVS_UNUSED) > { > .... > #if defined(__linux__) && defined(SYS_gettid) > pid_t tid = syscall(SYS_gettid); > err = setpriority(PRIO_PROCESS, tid, nice); > .... > #endif > } I don't know a better way to implement this and it seems ovs-numa.c already has some ifdefs specific to linux. Do you know if this problem happen on BSD? > Without priority patch: > > $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e pmd > 22509 19 4 ovs-vswitchd > 22512 19 5 lcore-slave-5 > 22513 19 6 lcore-slave-6 > 22514 19 7 lcore-slave-7 > 22589 19 4 revalidator37 > 22590 19 4 revalidator52 > 22591 19 4 revalidator42 > 22592 19 4 revalidator38 > 22593 19 4 revalidator39 > 22594 19 4 revalidator45 > 22595 19 4 revalidator53 > 22596 19 4 revalidator54 > 22598 19 4 pmd61 [Default priority] > > With priority patch: > > $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e pmd > 24879 19 4 ovs-vswitchd > 24881 19 5 lcore-slave-5 > 24882 19 6 lcore-slave-6 > 24883 19 7 lcore-slave-7 > 24951 19 4 revalidator55 > 24952 19 4 revalidator37 > 24953 19 4 revalidator52 > 24954 19 4 revalidator42 > 24955 19 4 revalidator38 > 24956 19 4 revalidator39 > 24957 19 4 revalidator45 > 24958 19 4 revalidator53 > 24964 39 4 pmd61 [Higher priority set] Looks good, so if you affinity your bash to the CPU running pmd61 thread, are you able to use it? Thanks,
>-----Original Message----- >From: Flavio Leitner [mailto:fbl@sysclose.org] >Sent: Thursday, August 18, 2016 2:15 PM >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >Cc: Daniele Di Proietto <diproiettod@ovn.org>; dev@openvswitch.org >Subject: Re: [ovs-dev] [PATCH V6] netdev-dpdk: Set pmd thread priority > >On Tue, Aug 16, 2016 at 02:30:04PM +0000, Bodireddy, Bhanuprakash wrote: >> >-----Original Message----- >> >From: Daniele Di Proietto [mailto:diproiettod@ovn.org] >> >Sent: Tuesday, August 16, 2016 1:44 AM >> >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >> >Cc: dev@openvswitch.org; Flavio Leitner <fbl@sysclose.org> >> >Subject: Re: [PATCH V6] netdev-dpdk: Set pmd thread priority >> > >> >I found a crash if apply this patch, "dpdk-lcore-mask" is not set and >> >"-c 0x1" is passed to "dpdk-extra". >> My bad, I didn't test with dpdk-extra options. I see that the crash was due to >strtol. >> >> >Also, I believe Flavio had a comment on the previous version of this >> >patch. Would it be enough to use setpriority(2)? >> I sent out my comments in another mail. I agree to Flavio's suggestion >> as this seems less dangerous and is guaranteed to work even in case of >> misconfiguration. I tested this approach and have a concern with >setpriority(). >> >> To apply the new nice value to the thread, thread id is needed and due >> to absence of glibc wrapper for gettid, I have to use syscall(SYS_gettid). I >want to know if this is acceptable in OVS or better way to handle this? >> >> Void ovs_numa_thread_setpriority(int nice OVS_UNUSED) { .... >> #if defined(__linux__) && defined(SYS_gettid) >> pid_t tid = syscall(SYS_gettid); >> err = setpriority(PRIO_PROCESS, tid, nice); >> .... >> #endif >> } > >I don't know a better way to implement this and it seems ovs-numa.c already >has some ifdefs specific to linux. > >Do you know if this problem happen on BSD? I don't know if this is a problem on BSD. I searched a bit and found BSD code using "syscall(SYS_thr_self, &tid)" to retrieve the tid. > > >> Without priority patch: >> >> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e >pmd >> 22509 19 4 ovs-vswitchd >> 22512 19 5 lcore-slave-5 >> 22513 19 6 lcore-slave-6 >> 22514 19 7 lcore-slave-7 >> 22589 19 4 revalidator37 >> 22590 19 4 revalidator52 >> 22591 19 4 revalidator42 >> 22592 19 4 revalidator38 >> 22593 19 4 revalidator39 >> 22594 19 4 revalidator45 >> 22595 19 4 revalidator53 >> 22596 19 4 revalidator54 >> 22598 19 4 pmd61 [Default priority] >> >> With priority patch: >> >> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e >pmd >> 24879 19 4 ovs-vswitchd >> 24881 19 5 lcore-slave-5 >> 24882 19 6 lcore-slave-6 >> 24883 19 7 lcore-slave-7 >> 24951 19 4 revalidator55 >> 24952 19 4 revalidator37 >> 24953 19 4 revalidator52 >> 24954 19 4 revalidator42 >> 24955 19 4 revalidator38 >> 24956 19 4 revalidator39 >> 24957 19 4 revalidator45 >> 24958 19 4 revalidator53 >> 24964 39 4 pmd61 [Higher priority set] > >Looks good, so if you affinity your bash to the CPU running >pmd61 thread, are you able to use it? Yes, I tested this case and there seems to be no problem here. Regards, Bhanu Prakash. > >Thanks, >-- >fbl
2016-08-18 14:20 GMT-07:00 Bodireddy, Bhanuprakash < bhanuprakash.bodireddy@intel.com>: > >-----Original Message----- > >From: Flavio Leitner [mailto:fbl@sysclose.org] > >Sent: Thursday, August 18, 2016 2:15 PM > >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> > >Cc: Daniele Di Proietto <diproiettod@ovn.org>; dev@openvswitch.org > >Subject: Re: [ovs-dev] [PATCH V6] netdev-dpdk: Set pmd thread priority > > > >On Tue, Aug 16, 2016 at 02:30:04PM +0000, Bodireddy, Bhanuprakash wrote: > >> >-----Original Message----- > >> >From: Daniele Di Proietto [mailto:diproiettod@ovn.org] > >> >Sent: Tuesday, August 16, 2016 1:44 AM > >> >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> > >> >Cc: dev@openvswitch.org; Flavio Leitner <fbl@sysclose.org> > >> >Subject: Re: [PATCH V6] netdev-dpdk: Set pmd thread priority > >> > > >> >I found a crash if apply this patch, "dpdk-lcore-mask" is not set and > >> >"-c 0x1" is passed to "dpdk-extra". > >> My bad, I didn't test with dpdk-extra options. I see that the crash was > due to > >strtol. > >> > >> >Also, I believe Flavio had a comment on the previous version of this > >> >patch. Would it be enough to use setpriority(2)? > >> I sent out my comments in another mail. I agree to Flavio's suggestion > >> as this seems less dangerous and is guaranteed to work even in case of > >> misconfiguration. I tested this approach and have a concern with > >setpriority(). > >> > >> To apply the new nice value to the thread, thread id is needed and due > >> to absence of glibc wrapper for gettid, I have to use > syscall(SYS_gettid). I > >want to know if this is acceptable in OVS or better way to handle this? > >> > >> Void ovs_numa_thread_setpriority(int nice OVS_UNUSED) { .... > >> #if defined(__linux__) && defined(SYS_gettid) > >> pid_t tid = syscall(SYS_gettid); > >> err = setpriority(PRIO_PROCESS, tid, nice); > >> .... > >> #endif > >> } > > > >I don't know a better way to implement this and it seems ovs-numa.c > already > >has some ifdefs specific to linux. > > > >Do you know if this problem happen on BSD? > I don't know if this is a problem on BSD. I searched a bit and found BSD > code using "syscall(SYS_thr_self, &tid)" > to retrieve the tid. > > The dummy ovs-numa works (and thus compile) everywhere, for pmd tests, but the module only works on linux. I think it's fine to implement something only for linux and return EOPNOTSUPP (only if dummy is not enabled) for everything else. I think passing 0 as thread_id to setpriority() changes the current thread priority, so there's probably no need for SYS_gettid. > > > > > >> Without priority patch: > >> > >> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e > ovs-vswitchd -e > >pmd > >> 22509 19 4 ovs-vswitchd > >> 22512 19 5 lcore-slave-5 > >> 22513 19 6 lcore-slave-6 > >> 22514 19 7 lcore-slave-7 > >> 22589 19 4 revalidator37 > >> 22590 19 4 revalidator52 > >> 22591 19 4 revalidator42 > >> 22592 19 4 revalidator38 > >> 22593 19 4 revalidator39 > >> 22594 19 4 revalidator45 > >> 22595 19 4 revalidator53 > >> 22596 19 4 revalidator54 > >> 22598 19 4 pmd61 [Default priority] > >> > >> With priority patch: > >> > >> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e > ovs-vswitchd -e > >pmd > >> 24879 19 4 ovs-vswitchd > >> 24881 19 5 lcore-slave-5 > >> 24882 19 6 lcore-slave-6 > >> 24883 19 7 lcore-slave-7 > >> 24951 19 4 revalidator55 > >> 24952 19 4 revalidator37 > >> 24953 19 4 revalidator52 > >> 24954 19 4 revalidator42 > >> 24955 19 4 revalidator38 > >> 24956 19 4 revalidator39 > >> 24957 19 4 revalidator45 > >> 24958 19 4 revalidator53 > >> 24964 39 4 pmd61 [Higher priority set] > > > >Looks good, so if you affinity your bash to the CPU running > >pmd61 thread, are you able to use it? > > Yes, I tested this case and there seems to be no problem here. > So there's no need to make it exclusive with dpdk-lcore-mask anymore, right? Thanks, Daniele > > Regards, > Bhanu Prakash. > > > > >Thanks, > >-- > >fbl > >
> >2016-08-18 14:20 GMT-07:00 Bodireddy, Bhanuprakash ><bhanuprakash.bodireddy@intel.com>: >>-----Original Message----- >>From: Flavio Leitner [mailto:fbl@sysclose.org] >>Sent: Thursday, August 18, 2016 2:15 PM >>To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >>Cc: Daniele Di Proietto <diproiettod@ovn.org>; dev@openvswitch.org >>Subject: Re: [ovs-dev] [PATCH V6] netdev-dpdk: Set pmd thread priority >> >>On Tue, Aug 16, 2016 at 02:30:04PM +0000, Bodireddy, Bhanuprakash wrote: >>> >-----Original Message----- >>> >From: Daniele Di Proietto [mailto:diproiettod@ovn.org] >>> >Sent: Tuesday, August 16, 2016 1:44 AM >>> >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >>> >Cc: dev@openvswitch.org; Flavio Leitner <fbl@sysclose.org> >>> >Subject: Re: [PATCH V6] netdev-dpdk: Set pmd thread priority >>> > >>> >I found a crash if apply this patch, "dpdk-lcore-mask" is not set and >>> >"-c 0x1" is passed to "dpdk-extra". >>> My bad, I didn't test with dpdk-extra options. I see that the crash was due >to >>strtol. >>> >>> >Also, I believe Flavio had a comment on the previous version of this >>> >patch. Would it be enough to use setpriority(2)? >>> I sent out my comments in another mail. I agree to Flavio's suggestion >>> as this seems less dangerous and is guaranteed to work even in case of >>> misconfiguration. I tested this approach and have a concern with >>setpriority(). >>> >>> To apply the new nice value to the thread, thread id is needed and due >>> to absence of glibc wrapper for gettid, I have to use syscall(SYS_gettid). I >>want to know if this is acceptable in OVS or better way to handle this? >>> >>> Void ovs_numa_thread_setpriority(int nice OVS_UNUSED) { .... >>> #if defined(__linux__) && defined(SYS_gettid) >>> pid_t tid = syscall(SYS_gettid); >>> err = setpriority(PRIO_PROCESS, tid, nice); >>> .... >>> #endif >>> } >> >>I don't know a better way to implement this and it seems ovs-numa.c >already >>has some ifdefs specific to linux. >> >>Do you know if this problem happen on BSD? >I don't know if this is a problem on BSD. I searched a bit and found BSD code >using "syscall(SYS_thr_self, &tid)" >to retrieve the tid. > > >The dummy ovs-numa works (and thus compile) everywhere, for pmd tests, >but the module only works on linux. >I think it's fine to implement something only for linux and return >EOPNOTSUPP (only if dummy is not enabled) for everything else. > >I think passing 0 as thread_id to setpriority() changes the current thread >priority, so there's probably no need for SYS_gettid. This is helpful, passing 0 for thread_id worked. > >> >> >>> Without priority patch: >>> >>> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e >>pmd >>> 22509 19 4 ovs-vswitchd >>> 22512 19 5 lcore-slave-5 >>> 22513 19 6 lcore-slave-6 >>> 22514 19 7 lcore-slave-7 >>> 22589 19 4 revalidator37 >>> 22590 19 4 revalidator52 >>> 22591 19 4 revalidator42 >>> 22592 19 4 revalidator38 >>> 22593 19 4 revalidator39 >>> 22594 19 4 revalidator45 >>> 22595 19 4 revalidator53 >>> 22596 19 4 revalidator54 >>> 22598 19 4 pmd61 [Default priority] >>> >>> With priority patch: >>> >>> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e >>pmd >>> 24879 19 4 ovs-vswitchd >>> 24881 19 5 lcore-slave-5 >>> 24882 19 6 lcore-slave-6 >>> 24883 19 7 lcore-slave-7 >>> 24951 19 4 revalidator55 >>> 24952 19 4 revalidator37 >>> 24953 19 4 revalidator52 >>> 24954 19 4 revalidator42 >>> 24955 19 4 revalidator38 >>> 24956 19 4 revalidator39 >>> 24957 19 4 revalidator45 >>> 24958 19 4 revalidator53 >>> 24964 39 4 pmd61 [Higher priority set] >> >>Looks good, so if you affinity your bash to the CPU running >>pmd61 thread, are you able to use it? >Yes, I tested this case and there seems to be no problem here. > >So there's no need to make it exclusive with dpdk-lcore-mask anymore, right? Yes, I have sent out another version of patch which doesn't change any of the existing functionality but lower the nice Value of the pmd thread to -20. Regards, Bhanu Prakash.
diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md index dd36ae4..f039723 100755 --- a/INSTALL.DPDK-ADVANCED.md +++ b/INSTALL.DPDK-ADVANCED.md @@ -208,8 +208,10 @@ needs to be affinitized accordingly. pmd thread is CPU bound, and needs to be affinitized to isolated cores for optimum performance. - By setting a bit in the mask, a pmd thread is created and pinned - to the corresponding CPU core. e.g. to run a pmd thread on core 2 + By setting a bit in the mask, a pmd thread is created, pinned + to the corresponding CPU core and the scheduling policy SCHED_RR + along with maximum priority of the policy applied to the pmd thread. + e.g. to pin a pmd thread on core 2 `ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=4` @@ -237,8 +239,10 @@ needs to be affinitized accordingly. responsible for different ports/rxq's. Assignment of ports/rxq's to pmd threads is done automatically. - A set bit in the mask means a pmd thread is created and pinned - to the corresponding CPU core. e.g. to run pmd threads on core 1 and 2 + A set bit in the mask means a pmd thread is created, pinned to the + corresponding CPU core and the scheduling policy SCHED_RR with highest + priority of the scheduling policy applied to pmd thread. + e.g. to run pmd threads on core 1 and 2 `ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6` @@ -249,6 +253,18 @@ needs to be affinitized accordingly. NIC port0 <-> OVS <-> VM <-> OVS <-> NIC port 1 + Note: 'dpdk-lcore-mask' and 'pmd-cpu-mask' cpu mask settings should be + non-overlapping. If you set dpdk-lcore-mask same as pmd-cpu-mask, the pmd + thread is not spawned on lowest core of the dpdk-lcore-mask and in case of + cpu mask setting where both the control and the pmd thread pinned to same + core, pmd thread is not spawned at all if it is the only cpu in the affinity + mask. + + For example, if dpdk-lcore-mask=F0 and pmd-cpu-mask=F0, pmd threads are + created on cores 5, 6, and 7 but not on 4. Setting both lcore,pmd mask to + core 4 shall pin control threads to core 4 but will prevent pmd thread from + spawning. + ### 4.3 DPDK physical port Rx Queues `ovs-vsctl set Interface <DPDK interface> options:n_rxq=<integer>` diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 96504f5..abe1fde 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3100,6 +3100,18 @@ pmd_thread_main(void *f_) ovs_numa_thread_setaffinity_core(pmd->core_id); dpdk_set_lcore_id(pmd->core_id); poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list); + + /* When cpu affinity mask explicitly set using pmd-cpu-mask, pmd thread's + * scheduling policy is set to SCHED_RR and priority to highest priority + * of SCHED_RR policy. In the absence of pmd-cpu-mask, default scheduling + * policy and priority shall apply to pmd thread. + * + * If dpdk-lcore-mask is set same as pmd-cpu-mask, the pmd thread is not + * spawned on lowest core of the dpdk-lcore-mask. + */ + if (pmd->dp->pmd_cmask) { + ovs_numa_thread_setpriority(SCHED_RR); + } reload: emc_cache_init(&pmd->flow_cache); diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index c767fd4..5d606a2 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -3432,6 +3432,28 @@ dpdk_init__(const struct smap *ovs_other_config) } } + /* If dpdk-lcore-mask is explicitly specified, auto_determine is 'false' + * and control threads are pinned to lowest core of the affinity mask. + * Mark the corresponding control thread 'core->pinned' status to 'true' + * so that pmd thread can't be spawned on the same core. + */ + if (!auto_determine) { + const char *lcore_mask; + unsigned int core_id; + int dl_mask; + + lcore_mask = smap_get(ovs_other_config, "dpdk-lcore-mask"); + dl_mask = strtol(lcore_mask, NULL, 16); + + /* Get the lowest core_id of dpdk-lcore-mask */ + core_id = rightmost_1bit_idx(dl_mask); + + /* Set the control thread core 'pinned' status to true */ + if (!ovs_numa_try_pin_core_specific(core_id)) { + VLOG_ERR("Unable to pin core %d", core_id); + } + } + dpdk_argv = argv; dpdk_argc = argc; diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c index c8173e0..428f274 100644 --- a/lib/ovs-numa.c +++ b/lib/ovs-numa.c @@ -613,3 +613,21 @@ int ovs_numa_thread_setaffinity_core(unsigned core_id OVS_UNUSED) return EOPNOTSUPP; #endif /* __linux__ */ } + +void +ovs_numa_thread_setpriority(int policy) +{ + if (dummy_numa) { + return; + } + + struct sched_param threadparam; + int err; + + memset(&threadparam, 0, sizeof(threadparam)); + threadparam.sched_priority = sched_get_priority_max(policy); + err = pthread_setschedparam(pthread_self(), policy, &threadparam); + if (err) { + VLOG_ERR("Thread priority error %s",ovs_strerror(err)); + } +} diff --git a/lib/ovs-numa.h b/lib/ovs-numa.h index be836b2..94f0884 100644 --- a/lib/ovs-numa.h +++ b/lib/ovs-numa.h @@ -56,6 +56,7 @@ void ovs_numa_unpin_core(unsigned core_id); struct ovs_numa_dump *ovs_numa_dump_cores_on_numa(int numa_id); void ovs_numa_dump_destroy(struct ovs_numa_dump *); int ovs_numa_thread_setaffinity_core(unsigned core_id); +void ovs_numa_thread_setpriority(int policy); #define FOR_EACH_CORE_ON_NUMA(ITER, DUMP) \ LIST_FOR_EACH((ITER), list_node, &(DUMP)->dump)
Set the DPDK pmd thread scheduling policy to SCHED_RR and static priority to highest priority value of the policy. This is to deal with pmd thread starvation case where another cpu hogging process can get scheduled/affinitized on to the same core the pmd thread is running there by significantly impacting the datapath performance. To reproduce the pmd thread starvation case: $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6 $ taskset 0x2 cat /dev/zero > /dev/null & Setting the realtime scheduling policy to the pmd threads is one step towards Fastpath Service Assurance in OVS DPDK. The realtime scheduling policy is applied only when CPU mask is passed to 'pmd-cpu-mask'. For example: * In the absence of pmd-cpu-mask, one pmd thread shall be created and default scheduling policy and priority gets applied. * If pmd-cpu-mask is specified, one or more pmd threads shall be spawned on the corresponding core(s) in the mask and real time scheduling policy SCHED_RR and highest priority of the policy is applied to the pmd thread(s). With this commit, it is recommended that the OVS control thread and pmd thread shouldn't be pinned to same core ('dpdk-lcore-mask','pmd-cpu-mask' should be non-overlapping). If dpdk-lcore-mask is set same as pmd-cpu-mask the pmd thread is not spawned on lowest core of the dpdk-lcore-mask. Also other processes with same affinity as PMD thread will be unresponsive. Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> --- v5->v6: * Prohibit spawning pmd thread on the lowest core in dpdk-lcore-mask if lcore-mask and pmd-mask affinity are identical. * Updated Note section in INSTALL.DPDK-ADVANCED doc. * Tested below cases to verify system stability with pmd priority patch dpdk-lcore-mask | pmd-cpu-mask | Comment 1. Not set | Not set | control threads affinity: 0-27 pmd thread: core 0 2. 1 | 1 | pmd thread isn't spawned and warning logged in logfile. 3. 1 | c | 4. F0 | F0 | control threads pinned to core 4. 3 pmd threads created on core 5,6,7 but 4. v4->v5: * Reword Note section in DPDK-ADVANCED.md v3->v4: * Document update * Use ovs_strerror for reporting errors in lib-numa.c v2->v3: * Move set_priority() function to lib/ovs-numa.c * Apply realtime scheduling policy and priority to pmd thread only if pmd-cpu-mask is passed. * Update INSTALL.DPDK-ADVANCED. v1->v2: * Removed #ifdef and introduced dummy function "pmd_thread_setpriority" in netdev-dpdk.h * Rebase INSTALL.DPDK-ADVANCED.md | 24 ++++++++++++++++++++---- lib/dpif-netdev.c | 12 ++++++++++++ lib/netdev-dpdk.c | 22 ++++++++++++++++++++++ lib/ovs-numa.c | 18 ++++++++++++++++++ lib/ovs-numa.h | 1 + 5 files changed, 73 insertions(+), 4 deletions(-)