Hi,
On Thu, Jun 17, 2021 at 09:57:48PM +0530, Kumar Amber wrote: > This commit introduces additonal command line paramter ^^^^^^^^ ^^^^^^^^ Typos. > 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. > > $ OVS_DIR/utilities/ovs-appctl dpif-netdev/miniflow-parser-set study 500 There is no need to include "OVS_DIR/utilities/" as it depends on each particular deployment. > > Signed-off-by: Kumar Amber <kumar.am...@intel.com> > --- > Documentation/topics/dpdk/bridge.rst | 8 ++++++- > lib/dpif-netdev-extract-study.c | 15 +++++++++++- > lib/dpif-netdev-private-extract.h | 8 +++++++ > lib/dpif-netdev.c | 34 +++++++++++++++++++++++----- > 4 files changed, 57 insertions(+), 8 deletions(-) > > diff --git a/Documentation/topics/dpdk/bridge.rst > b/Documentation/topics/dpdk/bridge.rst > index 1c78adc75..e7e91289a 100644 > --- a/Documentation/topics/dpdk/bridge.rst > +++ b/Documentation/topics/dpdk/bridge.rst > @@ -288,7 +288,13 @@ An implementation can be selected manually by the > following command :: > Also user can select the study implementation which studies the traffic for > a specific number of packets by applying all availbale implementaions of > miniflow extract and than chooses the one with most optimal result for that > -traffic pattern. > +traffic pattern. User can also provide additonal parameter as packet count > +which is minimum packets which OVS must study before choosing optimal > +implementation, If no packet count is provided than default value is choosen. > + > +Study can be selected with packet count by the following command :: > + > + $ ovs-appctl dpif-netdev/miniflow-parser-set study 1024 Ian already commented here. > > Miniflow Extract Validation > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > diff --git a/lib/dpif-netdev-extract-study.c b/lib/dpif-netdev-extract-study.c > index d063d040c..c48fb125e 100644 > --- a/lib/dpif-netdev-extract-study.c > +++ b/lib/dpif-netdev-extract-study.c > @@ -55,6 +55,19 @@ get_study_stats(void) > return stats; > } > > +static uint32_t pkt_compare_count = 0; If we had a convention as mentioned in patch #3, this could be mfex_study_compare_count or maybe mfex_study_pkts_count. > + > +uint32_t mfex_set_study_pkt_cnt(uint32_t pkt_cmp_count, > + struct dpif_miniflow_extract_impl *opt) > +{ > + if ((opt->extract_func == mfex_study_traffic) && (pkt_cmp_count != 0)) { > + pkt_compare_count = pkt_cmp_count; > + return 0; > + } > + pkt_compare_count = MFEX_MAX_COUNT; > + return -EINVAL; The documentation is not here, so not sure if it's missing or if there will be a patch updating the man-page at least. My suggestion would be to allow 0 to reset to built-in default. > +} > + > uint32_t > mfex_study_traffic(struct dp_packet_batch *packets, > struct netdev_flow_key *keys, > @@ -87,7 +100,7 @@ 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_COUNT) { > + if (stats->pkt_count >= pkt_compare_count) { > uint32_t best_func_index = MFEX_IMPL_START_IDX; > uint32_t max_hits = 0; > for (int i = MFEX_IMPL_START_IDX; i < impl_count; i++) { > diff --git a/lib/dpif-netdev-private-extract.h > b/lib/dpif-netdev-private-extract.h > index d8a284db7..0ec74bef9 100644 > --- a/lib/dpif-netdev-private-extract.h > +++ b/lib/dpif-netdev-private-extract.h > @@ -127,5 +127,13 @@ dpif_miniflow_extract_get_default(void); > * overridden at runtime. */ > void > dpif_miniflow_extract_set_default(miniflow_extract_func func); > +/* Sets the packet count from user to the stats for use in > + * study function to match against the classified packets to choose > + * the optimal implementation. > + * On error, returns EINVAL. > + * On success, returns 0. > + */ > +uint32_t mfex_set_study_pkt_cnt(uint32_t pkt_cmp_count, > + struct dpif_miniflow_extract_impl *opt); > > #endif /* DPIF_NETDEV_AVX512_EXTRACT */ > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 716e0debf..35c927d55 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -1141,14 +1141,29 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn > *conn, int argc, > return; > } > new_func = opt->extract_func; > - /* argv[2] is optional datapath instance. If no datapath name is > provided. > + > + /* argv[2] is optional packet count, which user can provide along with > + * study function to set the minimum packet that must be matched in order > + * to choose the optimal function. */ > + uint32_t pkt_cmp_count = 0; > + uint32_t study_ret; > + if (argc == 3) { > + char *err_str; > + pkt_cmp_count = strtoul(argv[2], &err_str, 10); > + study_ret = mfex_set_study_pkt_cnt(pkt_cmp_count, opt); > + } else { > + /* Default packet compare count when packets count not provided. */ > + study_ret = mfex_set_study_pkt_cnt(0, opt); > + } > + > + /* argv[3] is optional datapath instance. If no datapath name is > provided. > * and only one datapath exists, the one existing datapath is reprobed. > */ > ovs_mutex_lock(&dp_netdev_mutex); > struct dp_netdev *dp = NULL; > > - if (argc == 3) { > - dp = shash_find_data(&dp_netdevs, argv[2]); > + if (argc == 4) { > + dp = shash_find_data(&dp_netdevs, argv[3]); > } else if (shash_count(&dp_netdevs) == 1) { > dp = shash_first(&dp_netdevs)->data; > } > @@ -1182,7 +1197,14 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn > *conn, int argc, > > /* Reply with success to command. */ > struct ds reply = DS_EMPTY_INITIALIZER; > - ds_put_format(&reply, "Miniflow implementation set to %s.\n", mfex_name); > + if (study_ret == 0) { > + ds_put_format(&reply, "Miniflow implementation set to %s" > + "(minimum packet to study: %d)\n", > + mfex_name, pkt_cmp_count); > + } else { > + ds_put_format(&reply, "Miniflow implementation set to %s.\n", > + mfex_name); > + } > const char *reply_str = ds_cstr(&reply); > VLOG_INFO("%s", reply_str); > unixctl_command_reply(conn, reply_str); > @@ -1416,8 +1438,8 @@ dpif_netdev_init(void) > 1, 2, dpif_netdev_impl_set, > NULL); > unixctl_command_register("dpif-netdev/miniflow-parser-set", > - "miniflow implementation name [dp]", > - 1, 2, dpif_miniflow_extract_impl_set, > + "miniflow implementation name [pkt_cnt] [dp]", > + 1, 3, dpif_miniflow_extract_impl_set, Given that this command is generic for all implementations and it is now accepting arguments, I suggest we use parameters to identify them properly. For example: [-count pkt_cnt]. > NULL); > unixctl_command_register("dpif-netdev/dpif-get", "", > 0, 0, dpif_netdev_impl_get, > -- > 2.25.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev -- fbl _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev