> -----Original Message-----
> From: Eelco Chaudron <[email protected]>
> Sent: Tuesday, July 13, 2021 1:04 PM
> To: Amber, Kumar <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; Van
> Haaren, Harry <[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 7:32, Kumar Amber wrote:
<snip lots of patch>
> > + }
> >
> > int err = dp_mfex_impl_set_default_by_name(mfex_name);
>
> The previous comment was not act upon/replied to:
>
> “””
> > Should we call dp_mfex_impl_set_default_by_name() if we only set a single
> > PMD? I do not think, as we change the default for all threads not just
> this
> specific one.
> “””
There's a bigger picture to see here;
Before PMDs threads start polling RXQs, there is nothing.
If we run the MFEX parser set command at this point, it has no effect,
as there is no PMD thread structure to update the function pointer on.
The set_default() here causes that implementation to be saved for later,
as a result when the PMD-structure is created, it picks up the users desired
MFEX pointer, and behaves as expected.
Yes it sets the default here every time, even with a single PMD... actually,
even
when there are zero PMDs! But if we don't, then we cause headaches for the user
in that certain commands require specific states of PMD-threads launching &
polling.
This method is consistent, and always gives the user their desired behaviour.
<snip>
> >
> > + /* If PMD thread was specified, but it wasn't found, return error. */
> > + if (pmd_thread_specified && !pmd_thread_update_ok) {
>
> As mentioned the previous review, this error needs to move up before we
> change the default value, i.e. before dp_mfex_impl_set_default_by_name()
See above comment for explanation as to why that is first.
It comes down to the fact that we want to set the default, even when there
are zero PMD threads to update.
If the user passes a specific pmd-thread, we must iterate them to identify if
there's an error, hence the user-facing error-logging is here (and not above).
Hope that clarifies the current implementation/design. Code level changes
that were <snipped> from this reply are being addressed by Amber.
Regards, -Harry
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
