[ovs-dev] [v4 03/12] dpif-netdev: Add study function to select the best mfex function

Van Haaren, Harry harry.van.haaren at intel.com
Wed Jun 30 09:43:44 UTC 2021


> -----Original Message-----
> From: Flavio Leitner <fbl at sysclose.org>
> Sent: Tuesday, June 29, 2021 7:11 PM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>
> Cc: Eelco Chaudron <echaudro at redhat.com>; Amber, Kumar
> <kumar.amber at intel.com>; dev at openvswitch.org; i.maximets at 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-bounces at openvswitch.org> On Behalf Of Eelco Chaudron
> > > Sent: Tuesday, June 29, 2021 1:38 PM
> > > To: Amber, Kumar <kumar.amber at intel.com>
> > > Cc: dev at openvswitch.org; i.maximets at 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


More information about the dev mailing list