[ovs-dev] [v12 06/11] dpif-netdev: Add packet count and core id paramters for study

Eelco Chaudron echaudro at redhat.com
Wed Jul 14 15:13:22 UTC 2021



On 14 Jul 2021, at 16:14, kumar Amber wrote:

> From: Kumar Amber <kumar.amber at intel.com>
>
> This commit introduces additional 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.
> Also introduces a third paramter for choosing a particular pmd core.
>
> $ ovs-appctl dpif-netdev/miniflow-parser-set study 500 3
>
> Signed-off-by: Kumar Amber <kumar.amber at intel.com>
>
> ---
> v12:
> - re-work the set command to sweep
> - inlcude fixes to study.c and doc changes
> v11:
> - include comments from Eelco
> - reworked set command as per discussion
> v10:
> - fix review comments Eelco
> v9:
> - fix review comments Flavio
> v7:
> - change the command paramters for core_id and study_pkt_cnt
> v5:
> - fix review comments(Ian, Flavio, Eelco)
> - introucde pmd core id parameter
> ---
> ---
>  Documentation/topics/dpdk/bridge.rst |  38 +++++-
>  lib/dpif-netdev-extract-study.c      |  30 ++++-
>  lib/dpif-netdev-private-extract.h    |   9 ++
>  lib/dpif-netdev.c                    | 178 ++++++++++++++++++++++-----
>  4 files changed, 222 insertions(+), 33 deletions(-)
>
> diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
> index a47153495..8c500c504 100644
> --- a/Documentation/topics/dpdk/bridge.rst
> +++ b/Documentation/topics/dpdk/bridge.rst
> @@ -284,12 +284,46 @@ command also shows whether the CPU supports each implementation ::
>
>  An implementation can be selected manually by the following command ::
>
> -    $ ovs-appctl dpif-netdev/miniflow-parser-set study
> +    $ ovs-appctl dpif-netdev/miniflow-parser-set [-pmd core_id] [name]
> +                                                 [study_cnt]
> +
> +The above command has two optional parameters: study_cnt and core_id.
> +The core_id sets a particular miniflow extract function to a specific
> +pmd thread on the core. The third parameter study_cnt, which is specific
> +to study and ignored by other implementations, means how many packets
> +are needed to choose the best implementation.
>
>  Also user can select the study implementation which studies the traffic for
>  a specific number of packets by applying all available implementations of
>  miniflow extract and then chooses the one with the most optimal result for
> -that traffic pattern.
> +that traffic pattern. The user can optionally provide an packet count
> +[study_cnt] 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, 128 is chosen. Also, as there is no
> +synchronization point between threads, one PMD thread might still be running
> +a previous round, and can now decide on earlier data.
> +
> +The per packet count is a global value, and parallel study executions with
> +differing packet counts will use the most recent count value provided by user.
> +
> +Study can be selected with packet count by the following command ::
> +
> +    $ ovs-appctl dpif-netdev/miniflow-parser-set study 1024
> +
> +Study can be selected with packet count and explicit PMD selection
> +by the following command ::
> +
> +    $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 study 1024
> +
> +In the above command the first parameter is the CORE ID of the PMD
> +thread and this can also be used to explicitly set the miniflow
> +extraction function pointer on different PMD threads.
> +
> +Scalar can be selected on core 3 by the following command where
> +study count should not be provided for any implementation other
> +than study ::
> +
> +    $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 scalar
>
>  Miniflow Extract Validation
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/lib/dpif-netdev-extract-study.c b/lib/dpif-netdev-extract-study.c
> index 02b709f8b..083f940c2 100644
> --- a/lib/dpif-netdev-extract-study.c
> +++ b/lib/dpif-netdev-extract-study.c
> @@ -25,8 +25,7 @@
>
>  VLOG_DEFINE_THIS_MODULE(dpif_mfex_extract_study);
>
> -static atomic_uint32_t mfex_study_pkts_count = 0;
> -
> +static atomic_uint32_t  mfex_study_pkts_count = MFEX_MAX_PKT_COUNT;

You still did not remove the space you introduced (see last couple of patches with the same request)?

You accidentally removed the new line here.

>  /* Struct to hold miniflow study stats. */
>  struct study_stats {
>      uint32_t pkt_count;
> @@ -48,6 +47,28 @@ mfex_study_get_study_stats_ptr(void)
>      return stats;
>  }
>
> +int
> +mfex_set_study_pkt_cnt(uint32_t pkt_cmp_count, const char *name)
> +{
> +    struct dpif_miniflow_extract_impl *miniflow_funcs;
> +    miniflow_funcs = dpif_mfex_impl_info_get();
> +
> +    /* If the packet count is set and implementation called is study then
> +     * set packet counter to requested number or set the packet counter
> +     * to default number else return -EINVAL.
> +     */
> +    if ((strcmp(miniflow_funcs[MFEX_IMPL_STUDY].name, name) == 0) &&
> +        (pkt_cmp_count != 0)) {
> +
> +        atomic_uintptr_t *study_pck_cnt = (void *)&mfex_study_pkts_count;
> +        atomic_store_relaxed(study_pck_cnt, (uintptr_t) pkt_cmp_count );

Once again, please take time, and do not rush out these patches :(

The above two lines need removal once you add the line below (see diff I provided in the previous patch)…

> +        atomic_store_relaxed(&mfex_study_pkts_count, pkt_cmp_count);
> +        return 0;
> +    }
> +
> +    return -EINVAL;
> +}
> +
>  uint32_t
>  mfex_study_traffic(struct dp_packet_batch *packets,
>                     struct netdev_flow_key *keys,
> @@ -86,7 +107,10 @@ 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_PKT_COUNT) {
> +    static uint32_t study_cnt_pkts;

Why is the above static?

> +    atomic_read_relaxed(&mfex_study_pkts_count, &study_cnt_pkts);
> +
> +    if (stats->pkt_count >= study_cnt_pkts) {
>          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++) {
> diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private-extract.h
> index 9c03a1aa0..b93fecbbc 100644
> --- a/lib/dpif-netdev-private-extract.h
> +++ b/lib/dpif-netdev-private-extract.h
> @@ -151,4 +151,13 @@ mfex_study_traffic(struct dp_packet_batch *packets,
>                     uint32_t keys_size, odp_port_t in_port,
>                     struct dp_netdev_pmd_thread *pmd_handle);

Reviewed it up till here, will continue tomorrow.

<SNIP>



More information about the dev mailing list