Kevin Traynor <ktray...@redhat.com> writes:
> On 10/05/2023 07:10, Eli Britstein wrote:
>>
>>> -----Original Message-----
>>> From: Ilya Maximets <i.maxim...@ovn.org>
>>> Sent: Tuesday, 9 May 2023 21:04
>>> To: Eli Britstein <el...@nvidia.com>; ovs-discuss@openvswitch.org
>>> Cc: Kevin Traynor <ktray...@redhat.com>; Aaron Conole
>>> <acon...@redhat.com>; i.maxim...@ovn.org
>>> Subject: Re: [ovs-discuss] dpdk-lcore-mask
>>>
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 5/5/23 12:21, Eli Britstein via discuss wrote:
>>>> Hello,
>>>>
>>>> With [1], dpdk-lcore-mask configuration was introduced, instead of passing
>>> explicitly dpdk EAL options.
>>>> Commits [2]-[3] handle this code area.
>>>>
>>>> Currently, the affinity is restored to the original one only if the user
>>>> did not
>>> set any dpdk-lcore-mask, and it is "auto_determine".
>>>> In case it has been set, the affinity is not restored, so the main thread's
>>> affinity is for a single core, dpdk's "main core".
>>>> When other threads, such as "hw-offload" threads, "ct_clean" etc., they
>>> inherit this affinity, thus scaling doesn't work properly.
>>>>
>>>> Also, DPDK create worker threads, but they are not in use anyway by OVS.
>>>> PMD threads are created and registered to dpdk as "non-EAL" threads, and
>>> their affinity is managed by OVS according to "pmd-cpu-mask" configuration.
>>>> They get with an rte_lcore_id after the EAL IDs. Those are in use for
>>> example for mempool cache access, but that's OK as dpdk's cache is always up
>>> to RTE_MAX_LCORE.
>>>>
>>>> I wonder what the use is anyway for configuring dpdk-lcore-mask.
>>>> I think the affinity should always be restored, not only in the
>>> "auto_determine" case, but even so, there are still the worker threads
>>> created for vain.
>>>> Is there a use-case I am missing?
>>>
>>> There is a variety of other threads in dpdk that handle "interrupts",
>>> alarms,
>>> vhost events. And these threads are getting their affinity form the
>>> lcore-mask
>>> as well, IIRC.
>> That is correct. Those are "control threads" that are created by dpdk inside
>> rte_eal_init.
>> Their affinity is set by default to other cores than ones in -c. See those:
>> http://doc.dpdk.org/guides/prog_guide/env_abstraction_layer.html#eal-pthread-and-lcore-affinity
>> 4.3.4. Control Thread API
>> It is calculated in compute_ctrl_threads_cpuset().
>> DPDK support setting the affinity of those threads by "-s" option, but it is
>> not exposed in OVS.
>> See
>> https://doc.dpdk.org/guides/freebsd_gsg/freebsd_eal_parameters.html
>>
>>>
>>> In general, dpdk-lcore-mask config is not particularly useful indeed, so if
>>> you
>>> don't have a use case for it, then it likely shouldn't be set.
>>>
>>> Regarding restoration logic. It seems to come from this comment:
>>>
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
>>> work.ozlabs.org%2Fproject%2Fopenvswitch%2Fpatch%2F1450463278-7931-4-
>>> git-send-email-
>>> aconole%40redhat.com%2F%231222040&data=05%7C01%7Celibr%40nvidia.co
>>> m%7C06aacf4aadf4486d4bf608db50b7c768%7C43083d15727340c1b7db39efd9
>>> ccc17a%7C0%7C0%7C638192522479282270%7CUnknown%7CTWFpbGZsb3d8e
>>> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
>>> %7C3000%7C%7C%7C&sdata=x4RpaN6B1xfxQr2DZ1%2Br%2BBIbLSfCGeFncM
>>> BFM7k5kdI%3D&reserved=0
>>>
>>> The idea appears to be that users may want to run OVS completely on isolated
>>> cores and provide them via lcore-mask. In this case we should not restore
>>> the
>>> affinity.
>> The affinity was always restored in [2], and restricted only to the
>> "auto_determine" case by [3].
>> Currently, if the user sets dpdk-lcore-mask:
>> 1. DPDK's control threads are created with affinity of ~mask.
>> 2. EAL threads are created with affinity of the mask, however with no use in
>> OVS.
>> 3. OVS main thread's affinity is not restored and has the affinity of the
>> "main-lcore".
>> 4. Other OVS threads created (ct_clean, hw_offload etc.) inherit the
>> sole affinity of the main thread (set to "main-lcore") which is
>> wrong behavior.
>> If the affinity is always restored, it will resolve #3,#4 but still
>> have #1,#2.
>> If this configuration is completely removed, all 1-4 will be resolved.
>> Unless I missed something, it is better to completely remove this argument
>> (dpdk-lcore-mask).
>> What do you think?
>>
>
> Hi Eli,
>
> I see your point that using dpdk-lcore-mask thread may not be useful
> in most cases, but it needs some context.
>
> Pre the commits referenced, there was *no* other way to run OVS-DPDK
> than to use a mask for EAL init (either by ovs-vswitchd command line
> or dpdk-lcore-mask) with all the behaviour that you have described.
Right - actually, there was an issue that it was impossible to isolate
the cores originally when you specify the lcore mask. So the behavior
was to always plunk the OVS threads on a single core. That behavior was
retained, but maybe there's a different preference?
> That is the reason dpdk-lcore-mask was made optional and the behaviour
> you prefer is present by default. So if you don't want the behaviour
> of dpdk-lcore-mask then just don't set it.
+1
> I'm not sure it make sense to remove the dpdk-lcore-mask at this
> stage, as some people may use it. I have seen it used in testing where
> there may be issues with OVS main thread running, and it can be moved
> to isolated core as a test.
agreed - and I've seen production deployments that make use of it as
well. Whether it is still required might be up for debate with those
customers, but at least they do use the feature.
> Perhaps there's some documentation updates, or tweaks to the behaviour
> that could be improved that weren't available way back then.
I think there should be some enhancement to the docs - the install doc
for dpdk is rather sparse about the conditions under which the
lcore-mask (and other parameters) matter. Especially since some of
these options are no longer required for DPDK to run - maybe it makes
sense to pull in some description from the dpdk side to that.
WDYT?
> thanks,
> Kevin.
>
>>>
>>> CC: Kevin and Aaron.
>>>
>>> Best regards, Ilya Maximets.
>>>
>>>>
>>>> Thanks,
>>>> Eli
>>>>
>>>> [1] bab694097133 ("netdev-dpdk: Convert initialization from cmdline to
>>>> db") [2] 563c98d86e77 ("netdev-dpdk: Restore thread affinity after
>>>> DPDK init") [3] 88964e6428dc ("netdev-dpdk: Autofill lcore coremask if
>>> absent")
>>
_______________________________________________
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
