[ovs-dev] [PATCH] vswitchd: simplify dpdk option parsing.

[Daniele Di Proietto]

> On 3 Mar 2015, at 09:17, Traynor, Kevin <kevin.traynor at intel.com> wrote:
> 
>> 
>> -----Original Message-----
>> From: Daniele Di Proietto [mailto:diproiettod at vmware.com]
>> Sent: Thursday, February 26, 2015 1:54 PM
>> To: Traynor, Kevin
>> Cc: dev at openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH] vswitchd: simplify dpdk option parsing.
>> 
>> 
>>> On 26 Feb 2015, at 13:19, Traynor, Kevin <kevin.traynor at intel.com> wrote:
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Daniele Di
>>>> Proietto
>>>> Sent: Wednesday, February 25, 2015 4:47 PM
>>>> To: dev at openvswitch.org
>>>> Subject: [ovs-dev] [PATCH] vswitchd: simplify dpdk option parsing.
>>>> 
>>>> With this commit the '--dpdk' option doesn't need to be at the beginning
>>>> of the command line. Furthermode, the code that calls 'rte_eal_init()'
>>>> can be slightly simplified by using the 'optind' variable. The change is
>>>> totally backward compatible
>>>> 
>>> 
>>> I've tested various combinations of this and have seen no issues.
>>> 
>> 
>> Thank you for testing this, it's really appreciated.
>> 
>>>> Documentation and manpages are updated accordingly.
>>>> 
>>>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>>>> ---
>>>> INSTALL.DPDK.md            | 14 ++++++++------
>>>> lib/netdev-dpdk.c          | 41 +++++++++++++++++------------------------
>>>> lib/netdev-dpdk.h          | 20 +++++++++++++-------
>>>> vswitchd/ovs-vswitchd.8.in |  9 ++++++---
>>>> vswitchd/ovs-vswitchd.c    | 11 +++++------
>>>> 5 files changed, 49 insertions(+), 46 deletions(-)
>>>> 
>>>> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
>>>> index 4c443e5..72318a8 100644
>>>> --- a/INSTALL.DPDK.md
>>>> +++ b/INSTALL.DPDK.md
>>>> @@ -77,7 +77,7 @@ Using the DPDK with ovs-vswitchd:
>>>> 
>>>> 1. Setup system boot
>>>>   Add the following options to the kernel bootline:
>>>> -
>>>> +
>>>>   `default_hugepagesz=1GB hugepagesz=1G hugepages=1`
>>>> 
>>>> 2. Setup DPDK devices:
>>>> @@ -139,10 +139,12 @@ Using the DPDK with ovs-vswitchd:
>>>> 
>>>> 5. Start vswitchd:
>>>> 
>>>> -   DPDK configuration arguments can be passed to vswitchd via `--dpdk`
>>>> -   argument. This needs to be first argument passed to vswitchd process.
>>>> -   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
>>>> -   for dpdk initialization.
>>>> +   To initialize DPDK support the '--dpdk' option must be used. It is
>>>> +   followed by suboptions that are passed to the DPDK library. The
>>>> suboptions
>>>> +   list is terminated by `--`: the remaining options are parsed by
>> vswitchd.
>>>> +
>>>> +   Please not that `-c` and `-n` DPDK suboptions are required (although
>> `-c`
>>>> +   is ignored by OVS)
>>> 
>>> typo - "Please note”
>>> 
>> 
>> Oops, I’ll fix that
>> 
>>> "-c is ignored by OVS" is true, but it may mislead that it has no
>>> impact on OVS operation - it impacts the core affinity of the vswitchd
>>> process, so might be worth adding that.
>>> 
>> 
>> -c is passed to DPDK, but it is ignored by OVS. To configure the threads and
>> the affinity we currently use the database key "other_config:pmd-cpu-mask”
>> (INSTALL.DPDK.md “Performance tuning”). Perhaps we should add a reference to
>> the database key here? That’s probably going to change somehow, in an effort
>> to ease OVS DPDK deployments. Thoughts?
> 
> The db key is just for the PMD(s). All the other vswitchd threads run on the 
> lowest core specified in the -c option, so probably we should mention that.
> 

You’re right, they do. If you agree I'd prefer changing that (seems a weird
behaviour). I’ll post something soon

Thanks