[ovs-dev] [RFC 1/3] dpdk: Unify passing of DPDK EAL arguments.

Wed Feb 6 16:07:52 UTC 2019

On 06.02.2019 18:05, Aaron Conole wrote:
> Ilya Maximets <i.maximets at samsung.com> writes:
> 
>> On 05.02.2019 23:19, Flavio Leitner wrote:
>>> On Tue, Feb 05, 2019 at 06:43:08PM +0300, Ilya Maximets wrote:
>>>> Hi Flavio.
>>>> Thanks for taking a look.
>>>>
>>>> On 05.02.2019 15:38, Flavio Leitner wrote:
>>>>>
>>>>> Hi Ilya,
>>>>>
>>>>> Thanks for the patch and I think we knew about that pain when we
>>>>> exposed the very first parameter. I still remember Aaron struggling
>>>>> to find the best path forward. Time flies and here we are.
>>>>>
>>>>> The problem is that this change is not friendly from the community
>>>>> perspective to its users. That is like an exposed API which we should
>>>>> avoid break as much as possible.
>>>>>
>>>>> For instance, there are users (OpenStack) with support life cycle of
>>>>> 5+ years that cannot keep up with this kind of change but they expect
>>>>> to be able to update to newer OVS.
>>>>
>>>> Sure, there are users that wants stable API that will never change.
>>>> But this is really impossible in practice. I'm working with OpenStack
>>>> too and will have to fixup deployment tools with these changes. BTW, from
>>>> the whole OpenStack only few deployment projects (like TripleO) will need
>>>> to make changes and these changes are not very big.
>>>
>>> That's only part of the work. There will be work on QA, documentation
>>> and even migration path from one to another. And we can't change the
>>> past for existing deployments.
>>
>> Sure. But incompatible API changes are almost unavoidable for a young
>> projects that wants to be better.
>>
>>>
>>>>> One idea is to freeze whatever we have now and update the documentation
>>>>> to recommend the new way. We give like a couple OVS releases and only
>>>>> then ignore (or remove?) those parameters.
>>>>
>>>> Yes, In cover letter I proposed these patches to be applied one per release.
>>>> And current (first) patch does not remove the functionality, only docs.
>>>> Users still will be able to use old interface, but will have warnings
>>>> in the log. In the next release cycle we'll start ignore the values
>>>> while still printing the warnings. This should give enough time for adaptation.
>>>> If you feel that we need more time, we could apply the second patch to 2.14
>>>> (or whatever number will be in 2 releases from now).
>>>
>>> I don't think we should remove the docs if the parameters are there as
>>> a first step. I mean, assume an existing deployment, there is a parameter
>>> which might be in use but there is no documentation available. That
>>> doesn't sound like a good user experience to me.
>>
>> Maybe we could save a man pages while removing the guides. There is no much
>> information in Documentation/intro/install/dpdk.rst anyway.
>>
>>>
>>> On another hand, you could introduce the new interface and update the
>>> docs to recommend using the new one because the old one will be removed
>>> in the future. Warning messages come next, and then finally its removal.
>>
>> I'd prefer to have warning messages to be there right from the start to
>> push users to migrate to the new interfaces as early as possible.
>>
>> How about this:
>>
>> First stage (apply now, release in 2.12):
>>   - Introduce new interface 'dpdk-options'.
>>   - Rewrite installation guide with new interface fully removing the old one.
>>   - Add new interface to man pages (vswitch.xml) and mark all the old options
>>     as deprecated by adding something like: "Deprecated. 'dpdk-options' should
>>     be used instead. Will be ignored in the future."
>>   - Add a runtime deprecation warning if old interface is in use.
>>   - Ignore values of old knobs if 'dpdk-options' provided.
>>
>> Second stage (release in 2.13 or 2.14, could wait longer if required):
>>   - Remove old interfaces wile keeping the warnings. (i.e. values always ignored)
>>   - Remove old knobs from the man pages.
>>
>> Third stage (optional):
>>   - Remove warnings.
>>
>> So the main difference from the current patches is delaying removal of the man
>> pages to the second stage.
>>
>>>
>>>
>>>>> IMO in the end we are moving the problem from one place to another
>>>>> because even with a single string, OVS users will be caught off guard
>>>>> when DPDK changes. Yes, less pain to OVS community in the sense that
>>>>> we don't have to add/remove/deprecate stuff, but it is still a bad
>>>>> user experience regardless, which is not what OVS is known for.
>>>>
>>>> Unfortunately, DPDK was never user-friendly enough. It tries, but still not.
>>>
>>> Agreed.
>>>
>>>> We're keeping few sane defaults like providing default lcore and setting the
>>>> socket-limit if needed. And we'll try to do that in the future. The thing
>>>> this patch tries to eliminate is the dependency tracking between different
>>>> EAL arguments, i.e. duplicating the work with rte_eal_init() and having too
>>>> many configuration knobs with similar meanings what does not work at the
>>>> same time.
>>>>
>>>> Right now there are no critical arguments that user must provide.
>>>> So, IMHO, having special configs for them is really unnecessary.
>>>
>>> Do you think the defaults work for the majority of DPDK users?
>>
>> My personal experience is opposite. Most of my deployments and deployments that
>> I had to work with had massive 'dpdk-extra' arguments. These are mostly memory
>> tuning options, pci whitelists and various virtual devices. And these 'dpdk-extra'
>> strings only grows over time with '--socket-limit', '--legacy-mem',
>> '--single-file-segments', '--in-memory', '--file-prefix' and so on.
>>
>> Defaults are OK if your application is the only application you want to see on
>> a system. Otherwise, you need to fight with DPDK for resources/devices and your
>> (almost) only weapon is EAL argument string.
>>
>>>
>>> If we expose only the necessary things to consume DPDK it means we
>>> have tight control (less combinations to worry about), otherwise if
>>> the user can pass anything, a can of worms is opened and we have no
>>> control of all things the user can do.
>>
>> We have only illusion of control. User can pass anything with 'dpdk-extra'.
>> Also, the only necessary thing with dpdk 18.11 is an lcore list.
>>
>> IMHO, non-proficent users are happy with defaults. If user wants to tune,
>> 'dpdk-extra' will be used anyway.
> 
> A few questions about this - doesn't this imply that the dpdk-extra knob
> already does everything you want this new dpdk-options to do?  If so,
> what is the reason to rewrite the arguments?  It was really painful the
> first time we went through this exercise moving from the CLI to the DB.
> It's really difficult to make a change like this since no matter how you
> try to stage it, it *will* break some deployment.  And in this case, we
> have a knob that satisfies the requirement, I think.  Did I
> misunderstand it?

Yes, 'dpdk-extra' could be used to provide same functionality in current
deployments. Few points why I made another knob to rule them all:

 * I wanted to ignore all the values of old parameters by some rule.
   This rule now is that 'dpdk-options' specified.
 * 'dpdk-extra' is not the right name if we have only one knob.
 * We need to stop supporting old options because we can't support them
   properly and this requires big efforts to track all the dependencies
   which will be tracked by DPDK anyway.

> 
>> Best regards, Ilya Maximets.
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> 