On 06.02.2019 18:05, Aaron Conole wrote: > Ilya Maximets <i.maxim...@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 >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev