[ovs-dev] [v4 06/12] dpif-netdev: Add additional packet count parameter for study function
Eelco Chaudron
echaudro at redhat.com
Tue Jun 29 13:06:43 UTC 2021
See some additional comments below.
//Eelco
On 17 Jun 2021, at 18:27, Kumar Amber wrote:
> 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
>
> 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.
> +
Should we mention the default value?
Also, thinking about configuring the study option, as there is no synchronization point between threads, do we need to mention that one PMD thread might still be running a previous round, and can now decide on earlier data?
Let say you do:
ovs-appctl dpif-netdev/miniflow-parser-set study 30000
3 busy threads are done, and a 4th is still busy as it has only done 10000 packets. Now you do:
ovs-appctl dpif-netdev/miniflow-parser-set study 10000
And one thread will be done instantly, will the other might take a while…
> +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;
> +
> +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);
> +/* 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);
Guess you might want to use the ovs str_to_uint() like functions to verify the input string is valid.
> + 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