> -----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?
Regards, -Harry
<snip previous content>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
