[ovs-dev] [v4 06/12] dpif-netdev: Add additional packet count parameter for study function
Stokes, Ian
ian.stokes at intel.com
Thu Jun 24 18:07:34 UTC 2021
> This commit introduces additonal 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.
>
> $ OVS_DIR/utilities/ovs-appctl dpif-netdev/miniflow-parser-set study 500
>
Hi Amber, thanks for the patch, comments inline below.
> Signed-off-by: Kumar Amber <kumar.amber at 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.
I'd reword above as follows for typos and grammar.
"A user can also provide an additional packet count 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 is chosen."
> +
> +Study can be selected with packet count by the following command ::
> +
> + $ ovs-appctl dpif-netdev/miniflow-parser-set study 1024
>
> 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;
Would prefer variable declarations outside of functions to be declared at the beginning of the file after imports and defines.
> +
> +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;
> +}
> +
> 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);
Would add white space here to split comment below from function above.
BR
Ian
> +/* 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,
> NULL);
> unixctl_command_register("dpif-netdev/dpif-get", "",
> 0, 0, dpif_netdev_impl_get,
> --
> 2.25.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list