Hi Eelco, Yeah I missed one Comments below but all have been fixed waiting for more reviews on this patch before sending new one.
> -----Original Message----- > From: Eelco Chaudron <[email protected]> > Sent: Wednesday, July 14, 2021 8:43 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: [v12 06/11] dpif-netdev: Add packet count and core id paramters > for study > > > > On 14 Jul 2021, at 16:14, kumar Amber wrote: > > > From: Kumar Amber <[email protected]> > > > > This commit introduces additional command line paramter for mfex study > > function. If user provides additional packet out it is used in study > > to compare minimum packets which must be processed else a default > > value is choosen. > > Also introduces a third paramter for choosing a particular pmd core. > > > > $ ovs-appctl dpif-netdev/miniflow-parser-set study 500 3 > > > > Signed-off-by: Kumar Amber <[email protected]> > > > > --- > > v12: > > - re-work the set command to sweep > > - inlcude fixes to study.c and doc changes > > v11: > > - include comments from Eelco > > - reworked set command as per discussion > > v10: > > - fix review comments Eelco > > v9: > > - fix review comments Flavio > > v7: > > - change the command paramters for core_id and study_pkt_cnt > > v5: > > - fix review comments(Ian, Flavio, Eelco) > > - introucde pmd core id parameter > > --- > > --- > > Documentation/topics/dpdk/bridge.rst | 38 +++++- > > lib/dpif-netdev-extract-study.c | 30 ++++- > > lib/dpif-netdev-private-extract.h | 9 ++ > > lib/dpif-netdev.c | 178 ++++++++++++++++++++++----- > > 4 files changed, 222 insertions(+), 33 deletions(-) > > > > diff --git a/Documentation/topics/dpdk/bridge.rst > > b/Documentation/topics/dpdk/bridge.rst > > index a47153495..8c500c504 100644 > > --- a/Documentation/topics/dpdk/bridge.rst > > +++ b/Documentation/topics/dpdk/bridge.rst > > @@ -284,12 +284,46 @@ command also shows whether the CPU supports > each implementation :: > > > > An implementation can be selected manually by the following command :: > > > > - $ ovs-appctl dpif-netdev/miniflow-parser-set study > > + $ ovs-appctl dpif-netdev/miniflow-parser-set [-pmd core_id] [name] > > + [study_cnt] > > + > > +The above command has two optional parameters: study_cnt and core_id. > > +The core_id sets a particular miniflow extract function to a specific > > +pmd thread on the core. The third parameter study_cnt, which is > > +specific to study and ignored by other implementations, means how > > +many packets are needed to choose the best implementation. > > > > Also user can select the study implementation which studies the > > traffic for a specific number of packets by applying all available > > implementations of miniflow extract and then chooses the one with the > > most optimal result for -that traffic pattern. > > +that traffic pattern. The user can optionally provide an packet count > > +[study_cnt] parameter which is the minimum number of packets that OVS > > +must study before choosing an optimal implementation. If no packet > > +count is provided, then the default value, 128 is chosen. Also, as > > +there is no synchronization point between threads, one PMD thread > > +might still be running a previous round, and can now decide on earlier > > data. > > + > > +The per packet count is a global value, and parallel study executions > > +with differing packet counts will use the most recent count value provided > > by > user. > > + > > +Study can be selected with packet count by the following command :: > > + > > + $ ovs-appctl dpif-netdev/miniflow-parser-set study 1024 > > + > > +Study can be selected with packet count and explicit PMD selection by > > +the following command :: > > + > > + $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 study 1024 > > + > > +In the above command the first parameter is the CORE ID of the PMD > > +thread and this can also be used to explicitly set the miniflow > > +extraction function pointer on different PMD threads. > > + > > +Scalar can be selected on core 3 by the following command where study > > +count should not be provided for any implementation other than study > > +:: > > + > > + $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 scalar > > > > Miniflow Extract Validation > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/lib/dpif-netdev-extract-study.c > > b/lib/dpif-netdev-extract-study.c index 02b709f8b..083f940c2 100644 > > --- a/lib/dpif-netdev-extract-study.c > > +++ b/lib/dpif-netdev-extract-study.c > > @@ -25,8 +25,7 @@ > > > > VLOG_DEFINE_THIS_MODULE(dpif_mfex_extract_study); > > > > -static atomic_uint32_t mfex_study_pkts_count = 0; > > - > > +static atomic_uint32_t mfex_study_pkts_count = MFEX_MAX_PKT_COUNT; > > You still did not remove the space you introduced (see last couple of patches > with the same request)? > > You accidentally removed the new line here. > > > /* Struct to hold miniflow study stats. */ struct study_stats { > > uint32_t pkt_count; > > @@ -48,6 +47,28 @@ mfex_study_get_study_stats_ptr(void) > > return stats; > > } > > > > +int > > +mfex_set_study_pkt_cnt(uint32_t pkt_cmp_count, const char *name) { > > + struct dpif_miniflow_extract_impl *miniflow_funcs; > > + miniflow_funcs = dpif_mfex_impl_info_get(); > > + > > + /* If the packet count is set and implementation called is study then > > + * set packet counter to requested number or set the packet counter > > + * to default number else return -EINVAL. > > + */ > > + if ((strcmp(miniflow_funcs[MFEX_IMPL_STUDY].name, name) == 0) && > > + (pkt_cmp_count != 0)) { > > + > > + atomic_uintptr_t *study_pck_cnt = (void *)&mfex_study_pkts_count; > > + atomic_store_relaxed(study_pck_cnt, (uintptr_t) pkt_cmp_count > > + ); > > Once again, please take time, and do not rush out these patches :( > > The above two lines need removal once you add the line below (see diff I > provided in the previous patch)… > > > + atomic_store_relaxed(&mfex_study_pkts_count, pkt_cmp_count); > > + return 0; > > + } > > + > > + return -EINVAL; > > +} > > + > > uint32_t > > mfex_study_traffic(struct dp_packet_batch *packets, > > struct netdev_flow_key *keys, @@ -86,7 +107,10 @@ > > mfex_study_traffic(struct dp_packet_batch *packets, > > /* Choose the best implementation after a minimum packets have been > > * processed. > > */ > > - if (stats->pkt_count >= MFEX_MAX_PKT_COUNT) { > > + static uint32_t study_cnt_pkts; > > Why is the above static? > > > + atomic_read_relaxed(&mfex_study_pkts_count, &study_cnt_pkts); > > + > > + if (stats->pkt_count >= study_cnt_pkts) { > > uint32_t best_func_index = MFEX_IMPL_START_IDX; > > uint32_t max_hits = 0; > > for (int i = MFEX_IMPL_START_IDX; i < MFEX_IMPL_MAX; i++) { > > diff --git a/lib/dpif-netdev-private-extract.h > > b/lib/dpif-netdev-private-extract.h > > index 9c03a1aa0..b93fecbbc 100644 > > --- a/lib/dpif-netdev-private-extract.h > > +++ b/lib/dpif-netdev-private-extract.h > > @@ -151,4 +151,13 @@ mfex_study_traffic(struct dp_packet_batch > *packets, > > uint32_t keys_size, odp_port_t in_port, > > struct dp_netdev_pmd_thread *pmd_handle); > > Reviewed it up till here, will continue tomorrow. > > <SNIP> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
