[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
Thu Jun 24 14:38:25 UTC 2021


> -----Original Message-----
> From: Stokes, Ian <ian.stokes at intel.com>
> Sent: Thursday, June 24, 2021 2:20 PM
> To: Amber, Kumar <kumar.amber at intel.com>; dev at openvswitch.org; Van
> Haaren, Harry <harry.van.haaren at intel.com>
> Cc: Amber, Kumar <kumar.amber at intel.com>; i.maximets at ovn.org
> Subject: RE: [ovs-dev] [v4 03/12] dpif-netdev: Add study function to select the
> best mfex function
> 
> > The study function runs all the available implementations
> > of miniflow_extract and makes a choice whose hitmask has
> > maximum hits and sets the mfex to that function.
> 
> Hi Amber/Harry,
> 
> Thanks for the patch, a few comments inline below.

Thanks for review. Just addressing the stats get/TLS topic here.
<snip other patch changes>

> > +/* Struct to hold miniflow study stats. */
> > +struct study_stats {
> > +    uint32_t pkt_count;
> > +    uint32_t impl_hitcount[MFEX_IMPLS_MAX_SIZE];
> > +};
> > +
> > +/* Define per thread data to hold the study stats. */
> > +DEFINE_PER_THREAD_MALLOCED_DATA(struct study_stats *, study_stats);
> > +
> > +/* Allocate per thread PMD pointer space for study_stats. */
> > +static inline struct study_stats *
> > +get_study_stats(void)
> 
> Would maybe suggest a name change here, get_study_stats sounds as if info is
> being returned whereas whats actually happening is that the memory for the
> stats are being provisioned.

More context for explaining below...

> > +{
> > +    struct study_stats *stats = study_stats_get();
> > +    if (OVS_UNLIKELY(!stats)) {
> > +       stats = xzalloc(sizeof *stats);
> > +       study_stats_set_unsafe(stats);
> Can you explain why above is set unsafe? Where does that function originate
> from?

Yes, this is how the OVS "per thread data" (also called "Thread Local Storage" or TLS)
is implemented. The "get()" function indeed allocates the memory first time that this
thread actually accesses it, and any time after that it just returns the per-thread allocated
data pointer.

The "unsafe" is essentially the API used to change a TLS variable. It must only be called
by the thread that's using it itself, hence the unsafe() AFAIK.

The same function naming etc is used in DPCLS already, where this was the recommended
method of getting/using TLS data.

dpif-netdev-lookup-generic.c +47   function has "get_blocks_scratch()" which performs
approximately the same functionality as here. 

Hope that clears up that topic, regards, -Harry 


More information about the dev mailing list