> -----Original Message-----
> From: Flavio Leitner <f...@sysclose.org>
> Sent: Tuesday, June 29, 2021 7:11 PM
> To: Van Haaren, Harry <harry.van.haa...@intel.com>
> Cc: Eelco Chaudron <echau...@redhat.com>; Amber, Kumar
> <kumar.am...@intel.com>; d...@openvswitch.org; i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [v4 03/12] dpif-netdev: Add study function to select
> the best
> mfex function
>
> On Tue, Jun 29, 2021 at 04:32:05PM +0000, Van Haaren, Harry wrote:
> > > -----Original Message-----
> > > From: dev <ovs-dev-boun...@openvswitch.org> On Behalf Of Eelco Chaudron
> > > Sent: Tuesday, June 29, 2021 1:38 PM
> > > To: Amber, Kumar <kumar.am...@intel.com>
> > > Cc: d...@openvswitch.org; i.maxim...@ovn.org
> > > Subject: Re: [ovs-dev] [v4 03/12] dpif-netdev: Add study function to
> > > select the
> best
> > > mfex function
> > >
> > > More comments below. FYI I’m only reviewing right now, no testing.
> >
> > Sure, thanks for reviews.
> >
> > > On 17 Jun 2021, at 18:27, Kumar Amber wrote:
> >
> > <snip patch commit and some code>
> >
> > > > +/* Allocate per thread PMD pointer space for study_stats. */
> > > > +static inline struct study_stats *
> > > > +get_study_stats(void)
> > > > +{
> > > > + struct study_stats *stats = study_stats_get();
> > > > + if (OVS_UNLIKELY(!stats)) {
> > > > + stats = xzalloc(sizeof *stats);
> > > > + study_stats_set_unsafe(stats);
> > > > + }
> > > > + return stats;
> > > > +}
> > > > +
> > >
> > > Just got a mind-meld with the code, and realized that the function might
> > > be
> different
> > > per PMD thread due to this auto mode (and autovalidator mode in the
> > > previous
> > > patch).
> > >
> > > This makes it only stronger that we need a way to see the currently
> > > selected
> mode,
> > > and not per datapath, but per PMD per datapath!
> >
> > Study depends on the traffic pattern, so yes you're correct that it depends.
> > The study command was added after community suggested user-experience
> > would improve if the user doesn't have to provide an exact miniflow profile
> > name.
> >
> > Study studies the traffic running on that PMD, compares all MFEX impls, and
> > prints
> out
> > hits. It selects the _first_ implementation that surpasses the threshold of
> > packets.
> >
> > Users are free to use the more specific names of MFEX impls instead of
> > "study"
> > for fine-grained control over the MFEX impl in use, e.g.
> >
> > ovs-appctl dpif-netdev/miniflow-parser-set avx512_vbmi_ipv4_udp
> >
> > > Do we also need a way to set this per PMD?
> >
> > I don't feel there is real value here, but we could investigate adding an
> > optional parameter to the command indicating a PMD thread IDX to set?
> > We have access to "pmd->core_id" in our set() function, so limiting changes
> > to a specific PMD thread can be done ~ easily... but is it really required?
>
> I think the concern here (at least from my side) is that users can
> set the algorithm globally or per DP, not per PMD. However, the
> study can set different algorithms per PMD. For example, say that
> 'study' indicates that alg#1 for PMD#1 and alg#2 for PMD#2 in the
> lab. Now we want to move to production and make that selection
> static, how can we do that?
That's a good question. Today the command doesn't give us per-PMD thread
control. Study can indeed result in different PMDs having different MFEX funcs.
> If we set study, how do we tell from the cmdline the algorithm
> chose for each PMD? Another example of the same situation: if
> we always start with 'study' and suddenly there is a traffic
> processing difference. How one can check what is different in
> the settings? The logs don't tell which PMD was affected.
Sure they do; the "pmd-cX" and "pmd-cY" below show what datapath thread selects
what function.
Note that the first line is from the OVS command thread, which notes that
"study" was selected.
The following two prints are from each datapath thread, noting the resulting
function chosen by study.
2021-06-30T09:05:41Z|00134|dpif_netdev|INFO|Miniflow implementation set to
study.
2021-06-30T09:05:41Z|00001|dpif_mfex_extract_study(pmd-cX/id:X)|INFO|MFEX study
chose impl avx512_vbmi_ipv4_udp: (hits 128/128 pkts)
2021-06-30T09:05:41Z|00001|dpif_mfex_extract_study(pmd-cY/id:Y)|INFO|MFEX study
chose impl avx512_vbmi_ipv4_udp: (hits 128/128 pkts)
> > Perfect is the enemy of good... I'd prefer focus on getting existing code
> > changes
> merged,
> > and add additional (optional) parameters in future if deemed useful in real
> > world
> testing?
>
> True. Perhaps we have different use cases in mind. How do you expect
> users to use this feature? Do you think production users will always
> start with 'study'?
I was expecting OVS users to be aware of what L2-4 traffic they're
running, and to per-instance configure that statically for all datapath
threads, for example by running the command below:
$ ovs-appctl dpif-netdev/miniflow-parser-set avx512_ipv4_udp
There is an assumption here that all datapath threads handle
the same outer traffic type. If that's not the case, we cannot manually
set different MFEX impls to different pmd threads today, as your lab
to production requirement requests above.
If we add an optional PMD thread id parameter, we can support this:
$ ovs-appctl dpif-netdev/miniflow-parser-set avx512_ipv4_udp
<packet_count_to_study> <pmd thread idx>
> Thanks,
> fbl
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
