[ovs-dev] [v4 06/12] dpif-netdev: Add additional packet count parameter for study function

Amber, Kumar kumar.amber at intel.com
Tue Jun 29 15:52:59 UTC 2021


Hi Eelco,

Replies are inline.

> > +++ 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…
> 

Will update the study section accordingly .

> 
> > +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.
>

Thanks seems a fair idea to do will be taken into v5.
 
> > +        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