> > -----Original Message----- > > From: Stokes, Ian <ian.sto...@intel.com> > > Sent: Thursday, June 24, 2021 2:20 PM > > To: Amber, Kumar <kumar.am...@intel.com>; d...@openvswitch.org; Van > > Haaren, Harry <harry.van.haa...@intel.com> > > Cc: Amber, Kumar <kumar.am...@intel.com>; i.maxim...@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. >
Ah that makes more sense, have followed up on the existing code since and indeed it follows the same logic. > 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 Thanks for clarifying. BR Ian _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev