Hi Eelco, Fixed both and hence keeping the Ack 😊
> -----Original Message----- > From: Eelco Chaudron <[email protected]> > Sent: Tuesday, July 13, 2021 2:58 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: [v10 03/12] dpif-netdev: Add study function to select the best > mfex > function > > > > On 13 Jul 2021, at 7:32, Kumar Amber wrote: > > > 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. > > > > Study can be run at runtime using the following command: > > > > $ ovs-appctl dpif-netdev/miniflow-parser-set study > > > > Signed-off-by: Kumar Amber <[email protected]> > > Co-authored-by: Harry van Haaren <[email protected]> > > Signed-off-by: Harry van Haaren <[email protected]> > > Acked-by: Eelco Chaudron <[email protected]> > > > > I did ack the previous patch by accident, See some more comments below, if > fixed in v11 keep my ack. > > > --- > > v10: > > - fix minor comments from Eelco > > v9: > > - fix comments Flavio > > v8: > > - fix review comments Flavio > > v7: > > - fix review comments(Eelco) > > v5: > > <SNIP> > > > +uint32_t > > +mfex_study_traffic(struct dp_packet_batch *packets, > > + struct netdev_flow_key *keys, > > + uint32_t keys_size, odp_port_t in_port, > > + struct dp_netdev_pmd_thread *pmd_handle) { > > + uint32_t hitmask = 0; > > + uint32_t mask = 0; > > + struct dp_netdev_pmd_thread *pmd = pmd_handle; > > + struct dpif_miniflow_extract_impl *miniflow_funcs; > > + struct study_stats *stats = mfex_study_get_study_stats_ptr(); > > + miniflow_funcs = dpif_mfex_impl_info_get(); > > + > > + /* Run traffic optimized miniflow_extract to collect the hitmask > > + * to be compared after certain packets have been hit to choose > > + * the best miniflow_extract version for that traffic. > > + */ > > + for (int i = MFEX_IMPL_START_IDX; i < MFEX_IMPL_MAX; i++) { > > + if (!miniflow_funcs[i].available) { > > + continue; > > + } > > + > > + hitmask = miniflow_funcs[i].extract_func(packets, keys, keys_size, > > + in_port, pmd_handle); > > + stats->impl_hitcount[i] += count_1bits(hitmask); > > + > > + /* If traffic is not classified then we dont overwrite the keys > > + * array in minfiflow implementations so its safe to create a > > + * mask for all those packets whose miniflow have been created. > > + */ > > + mask |= hitmask; > > + } > > + > > + stats->pkt_count += dp_packet_batch_size(packets); > > + > > + /* Choose the best implementation after a minimum packets have been > > + * processed. > > + */ > > + if (stats->pkt_count >= MFEX_MAX_PKT_COUNT) { > > + 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++) { > > + if (stats->impl_hitcount[i] > max_hits) { > > + max_hits = stats->impl_hitcount[i]; > > + best_func_index = i; > > + } > > + } > > + > > + /* If 50% of the packets hit, enable the function. */ > > + if (max_hits >= (mfex_study_pkts_count / 2)) { > > + miniflow_extract_func mf_func = > > + miniflow_funcs[best_func_index].extract_func; > > + atomic_uintptr_t *pmd_func = (void > > *)&pmd->miniflow_extract_opt; > > + atomic_store_relaxed(pmd_func, (uintptr_t) mf_func); > > + VLOG_INFO("MFEX study chose impl %s: (hits %u/%u pkts)", > > + miniflow_funcs[best_func_index].name, max_hits, > > + stats->pkt_count); > > You only change the %d/%d here, but there are two more occurrences below. > > > + } else { > > + /* Set the implementation to null for default miniflow. */ > > + miniflow_extract_func mf_func = > > + miniflow_funcs[MFEX_IMPL_SCALAR].extract_func; > > + atomic_uintptr_t *pmd_func = (void > > *)&pmd->miniflow_extract_opt; > > + atomic_store_relaxed(pmd_func, (uintptr_t) mf_func); > > + VLOG_INFO("Not enough packets matched (%d/%d), disabling" > > + " optimized MFEX.", max_hits, stats->pkt_count); > > + } > > + > > + /* In debug mode show stats for all the counters. */ > > + if (VLOG_IS_DBG_ENABLED()) { > > + > > + for (int i = MFEX_IMPL_START_IDX; i < MFEX_IMPL_MAX; i++) { > > + VLOG_DBG("MFEX study results for implementation %s:" > > + " (hits %d/%d pkts)", > > + miniflow_funcs[i].name, stats->impl_hitcount[i], > > + stats->pkt_count); > > + } > > + } > > + > > <SNIP> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
