On 10/21/21 12:41, Kevin Traynor wrote:
> On 20/10/2021 16:13, Stokes, Ian wrote:
>>> Previously in OVS, a PMD thread running on cpu X used lcore X.
>>> This assumption limited OVS to run PMD threads on physical cpu <
>>> RTE_MAX_LCORE.
>>>
>>> DPDK 20.08 introduced a new API that associates a non-EAL thread to a free
>>> lcore. This new API does not change the thread characteristics (like CPU
>>> affinity) and let OVS run its PMD threads on any cpu regardless of
>>> RTE_MAX_LCORE.
>>>
>>> The DPDK multiprocess feature is not compatible with this new API and is
>>> disabled.
>>>
>>> DPDK still limits the number of lcores to RTE_MAX_LCORE (128 on x86_64)
>>> which should be enough for OVS pmd threads (hopefully).
>>>
>>> DPDK lcore/OVS pmd threads mapping are logged at threads when trying to
>>> attach a OVS PMD thread, and when detaching.
>>> A new command is added to help get DPDK point of view of the DPDK lcores
>>> at any time:
>>>
>>> $ ovs-appctl dpdk/lcore-list
>>> lcore 0, socket 0, role RTE, cpuset 0
>>> lcore 1, socket 0, role NON_EAL, cpuset 1
>>> lcore 2, socket 0, role NON_EAL, cpuset 15
>>>
>>> Signed-off-by: David Marchand <[email protected]>
>>
>> Hi David, thanks for the patch, currently testing but some comments below 
>> just to kick things off while I validate.
>>
>>> ---
>>> Changes since v6:
>>> - handled corner case when registering max number of PMD threads, then
>>>    removing all successfully registered, leaving the ones who had failed,
>>> - reworded warning when reaching max number of PMD threads,
>>> - added a comment in command about reaching max number of PMD threads,
>>> - fixed typo in debug command name,
>>>
>>> Changes since v5:
>>> - rebased,
>>> - commitlog tweaks,
>>> - dropped use of global ALLOW_EXPERIMENTAL flag and pinpointed
>>>    experimental API,
>>>
>>> Changes since v4:
>>> - rebased on the master branch,
>>> - disabled DPDK mp feature,
>>> - updated DPDK documentation and manual with the new command,
>>> - added notes in NEWS,
>>>
>>> Changes since v3:
>>> - rebased on current HEAD,
>>> - switched back to simple warning rather than abort when registering a
>>>    thread fails,
>>>
>>> Changes since v2:
>>> - introduced a new api in DPDK 20.08 (still being discussed), inbox thread 
>>> at
>>>    http://inbox.dpdk.org/dev/20200610144506.30505-1-
>>> [email protected]/T/#t
>>> - this current patch depends on a patch on master I sent:
>>>
>>> https://patchwork.ozlabs.org/project/openvswitch/patch/20200626122738.281
>>> [email protected]/
>>> - dropped 'dpdk-lcore-mask' compat handling,
>>>
>>> Changes since v1:
>>> - rewired existing configuration 'dpdk-lcore-mask' to use --lcores,
>>> - switched to a bitmap to track lcores,
>>> - added a command to dump current mapping (Flavio): used an experimental
>>>    API to get DPDK lcores cpuset since it is the most reliable/portable
>>>    information,
>>> - used the same code for the logs when starting DPDK/PMD threads,
>>> - addressed Ilya comments,
>>>
>>> ---
>>>   Documentation/howto/dpdk.rst |  5 ++++
>>>   NEWS                         |  2 ++
>>>   lib/dpdk-stub.c              |  9 +++++--
>>>   lib/dpdk-unixctl.man         |  3 +++
>>>   lib/dpdk.c                   | 48 +++++++++++++++++++++++++++++++++---
>>>   lib/dpdk.h                   |  3 ++-
>>>   lib/dpif-netdev.c            | 10 +++++++-
>>>   7 files changed, 73 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
>>> index 70b64881ab..81f236d3bd 100644
>>> --- a/Documentation/howto/dpdk.rst
>>> +++ b/Documentation/howto/dpdk.rst
>>> @@ -402,6 +402,11 @@ Supported actions for hardware offload are:
>>>   - Clone/output (tnl_push and output) for encapsulating over a tunnel.
>>>   - Tunnel pop, for packets received on physical ports.
>>>
>>> +Multiprocess
>>> +------------
>>> +
>>> +This DPDK feature is not supported and disabled during OVS initialization.
>>> +
>>
>> This should probably be added to the Limitations section under 
>> intro/install/dpdk.rst also?
>>
>>>   Further Reading
>>>   ---------------
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 90f4b15902..dc9709641c 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -10,6 +10,8 @@ Post-v2.16.0
>>>          limiting behavior.
>>>        * Add hardware offload support for matching IPv4/IPv6 frag types
>>>          (experimental).
>>> +     * Forbid use of DPDK multiprocess feature.
>>> +     * Add support for running threads on cores >= RTE_MAX_LCORE.
>>>
>>>
>>>   v2.16.0 - 16 Aug 2021
>>> diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
>>> index c332c217cb..3eee1f485c 100644
>>> --- a/lib/dpdk-stub.c
>>> +++ b/lib/dpdk-stub.c
>>> @@ -38,10 +38,15 @@ dpdk_init(const struct smap *ovs_other_config)
>>>       }
>>>   }
>>>
>>> +bool
>>> +dpdk_attach_thread(unsigned cpu OVS_UNUSED)
>>> +{
>>> +    return false;
>>> +}
>>> +
>>>   void
>>> -dpdk_set_lcore_id(unsigned cpu OVS_UNUSED)
>>> +dpdk_detach_thread(void)
>>>   {
>>> -    /* Nothing */
>>>   }
>>>
>>>   const char *
>>> diff --git a/lib/dpdk-unixctl.man b/lib/dpdk-unixctl.man
>>> index a0d1fa2ea3..5bac806389 100644
>>> --- a/lib/dpdk-unixctl.man
>>> +++ b/lib/dpdk-unixctl.man
>>> @@ -1,5 +1,8 @@
>>>   .SS "DPDK COMMANDS"
>>>   These commands manage DPDK components.
>>> +.IP "\fBdpdk/lcore-list\fR"
>>> +Lists the DPDK lcores and their cpu affinity.
>>> +When RTE_MAX_LCORE lcores are registered, some OVS PMD threads won't
>>> appear.
>>>   .IP "\fBdpdk/log-list\fR"
>>>   Lists all DPDK components that emit logs and their logging levels.
>>>   .IP "\fBdpdk/log-set\fR [\fIspec\fR]"
>>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>>> index fe201bf29c..f9f6404998 100644
>>> --- a/lib/dpdk.c
>>> +++ b/lib/dpdk.c
>>> @@ -470,6 +470,15 @@ dpdk_init__(const struct smap *ovs_other_config)
>>>           return false;
>>>       }
>>>
>>> +#pragma GCC diagnostic push
>>> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>>> +    if (!rte_mp_disable()) {
>>
>> So I assume the deprecated-declaration is required above because 
>> rte_mp_disable() is experimental (if memory serves correct it triggers 
>> deprecated)
>>
>> I'm surprised to see this is still experimental, if it was introduced in 
>> 20.08 I would have expected it to have moved from this status.
>>
> 
> It's not long compared to many :/
> 
> This will be done anyway in the register() fn, so iirc the idea on earlier 
> reviews was just to make it explicit for the OVS user. If the API was removed 
> in DPDK, I don't think it would impact OVS other than making it implicit.
> 
>> Is there plans for this to become stable/non-experimental?


David, IIRC, you mentioned previously that these functions was introduced
for OVS and adoption in OVS is a prerequisite for them to become 
non-experimental.
Is that right?

If so, will the acceptance of this change to dpdk-latest branch be considered
enough for marking these functions non-experimental in DPDK?

If so, can it be done in 21.11 release assuming fast acceptance of this patch
in dpdk-latest (it's Acked, and testing, I guess, should be finished soon by 
Ian)?

Same questions for the master branch:  If this patch will be accepted to OVS 
master,
can these APIs be marked non-experimental in DPDK 21.11, so 'pragma's can be 
dropped
once OVS moved to that version, and OVS 2.17 can be released without need to use
experimental APIs here?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to