[ovs-dev] [v4 06/12] dpif-netdev: Add additional packet count parameter for study function
Flavio Leitner
fbl at sysclose.org
Mon Jun 28 02:55:38 UTC 2021
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.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.
> +
> +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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--
fbl
More information about the dev
mailing list