Hi Eelco, The requested changes for the rework of MFEX-set commands are now available in v12 😊
Br Amber > -----Original Message----- > From: Eelco Chaudron <[email protected]> > Sent: Wednesday, July 14, 2021 4:04 PM > To: Van Haaren, Harry <[email protected]> > Cc: Amber, Kumar <[email protected]>; [email protected]; > [email protected]; [email protected]; Ferriter, Cian > <[email protected]>; Stokes, Ian <[email protected]> > Subject: Re: [v10 06/12] dpif-netdev: Add packet count and core id paramters > for study > > > > On 13 Jul 2021, at 18:11, Van Haaren, Harry wrote: > > >> -----Original Message----- > >> From: Eelco Chaudron <[email protected]> > >> Sent: Tuesday, July 13, 2021 3:26 PM > >> To: Van Haaren, Harry <[email protected]> > >> Cc: Amber, Kumar <[email protected]>; [email protected]; > >> [email protected]; [email protected]; Ferriter, Cian > >> <[email protected]>; Stokes, Ian <[email protected]> > >> Subject: Re: [v10 06/12] dpif-netdev: Add packet count and core id > >> paramters for study > >> > >> > >> > >> On 13 Jul 2021, at 16:09, Van Haaren, Harry wrote: > >> > >>> (Off Topic; Eelco, I think your email client is sending HTML > >>> replies, perhaps > >> check settings to disable?) > >>> > >>> Eelco Wrote; > >>>> If you ask for a specific existing pmd to run a specific > >>>> implementation, it > >> should NOT affect any existing or newly created pmd. > >>> > >>> OK, thanks for explaining it clearly with examples below! > >>> > >>> In general the behavior below is OK, except that we set the default > >>> when a > >> specific PMD is requested. > >>> As a solution, we can set the default behavior only when "-pmd" is > >>> NOT > >> provided. > >>> > >>> 1) "all pmds" (no args) mode: we set the default, update all current > >>> PMD > >> threads, and will update future PMD threads via default. > >>> 2) "-pmd <core>" mode: we set only for that PMD thread, and if the > >>> PMD > >> thread isn't active, existing code will provide good error to user. > >>> > >>> Does that seem a pragmatic solution? -Harry > >> > >> Yes, this is exactly what I had in mind when I added the comment. > > > > Ah great, good stuff. > > > > I've looked at the enabling code & patchset, the command parsing is getting > complex. > > I tend to agree with Eelco's review, that a single sweep of the args > > would be a better and more readable implementation, however today that > > command is built in multiple patchs, and each change would cause a > > rebase-conflict, so rework would cost more time than we would like, > particularly given the upcoming deadlines. > > > > Here a suggestion to pragmatically move forward: > > - Merge the code approximately as in this patchset, but with the 1) and 2) > suggestions applied above to fixup functional correctness. > > Amber has a v11 almost ready to go that addresses the main issues. > > > > - Commit to reworking the command in a follow-up patch next week. This > refactor would use the "sweep argc" method, > > and will continue to have the same user-visible > > functionality/error-messages, > just a cleaner implementation. > > (This avoids the rebase-heavy workflow of refactoring now, as the > > command is enabled over multiple commits.) > > > > Hope that's a workable solution for all? > > Just reviewed the v11, and I agree it looks even messier. I think it needs a > proper > rewrite and contradicting to your statement above, all changes are > concentrated in patch 6, so I think it needs to be done in the v12 as we are > almost there. > > Cheers, > > Eelco _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
