Hi Ilya,

Can you please provide feedback on the patch-set, if it suffices the flow based
testing or needs some change to adapt it to flow based testing for AVX512 MFEX ?

Regards
Amber
 

> -----Original Message-----
> From: Amber, Kumar <kumar.am...@intel.com>
> Sent: Wednesday, April 27, 2022 4:49 PM
> To: ovs-dev@openvswitch.org
> Cc: echau...@redhat.com; ktray...@redhat.com; i.maxim...@ovn.org;
> Ferriter, Cian <cian.ferri...@intel.com>; Stokes, Ian <ian.sto...@intel.com>;
> f...@sysclose.org; Van Haaren, Harry <harry.van.haa...@intel.com>; Amber,
> Kumar <kumar.am...@intel.com>
> Subject: [PATCH v3 3/3] flow: Add autovalidator support to miniflow_extract.
> 
> The patch adds the flag based switch between choice of using
> miniflow_extract in normal pipeline or select mfex_autovalidator in debug
> and test builds.
> 
> The compile time flag used to select autoval can be done using option:
> 
>  ./configure CFLAGS="--enable-mfex-default-autovalidator"
> 
> Signed-off-by: Kumar Amber <kumar.am...@intel.com>
> 
> ---
> v3:
> - Fix comments from Cian.
> ---
> ---
>  lib/dpif-netdev-private-extract.c |  8 ++++----
>  lib/flow.c                        | 34 ++++++++++++++++++++++++++++++-
>  lib/flow.h                        |  4 ++++
>  3 files changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private-
> extract.c
> index 42b970e75..bbc0e3c78 100644
> --- a/lib/dpif-netdev-private-extract.c
> +++ b/lib/dpif-netdev-private-extract.c
> @@ -124,8 +124,8 @@ dpif_miniflow_extract_init(void)
>      /* For the first call, this will be choosen based on the
>       * compile time flag.
>       */
> -    VLOG_INFO("Default MFEX Extract implementation is %s.\n",
> -              mfex_impls[mfex_idx].name);
> +    VLOG_DBG("Default MFEX Extract implementation is %s.\n",
> +             mfex_impls[mfex_idx].name);
>      atomic_store_relaxed(mfex_func, (uintptr_t) mfex_impls
>                           [mfex_idx].extract_func);  } @@ -251,7 +251,7 @@
> dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets,
>      /* Run scalar miniflow_extract to get default result. */
>      DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
>          pkt_metadata_init(&packet->md, in_port);
> -        miniflow_extract(packet, &keys[i]);
> +        miniflow_extract_(packet, &keys[i]);
> 
>          /* Store known good metadata to compare with optimized metadata. */
>          good_l2_5_ofs[i] = packet->l2_5_ofs; @@ -347,7 +347,7 @@
> dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets,
>      }
> 
>      /* Having dumped the debug info for the batch, disable autovalidator. */
> -    if (batch_failed) {
> +    if (batch_failed && (pmd != NULL)) {
>          atomic_store_relaxed(&pmd->miniflow_extract_opt, NULL);
>      }
> 
> diff --git a/lib/flow.c b/lib/flow.c
> index 086096d5e..16698cedd 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -36,6 +36,8 @@
>  #include "openvswitch/match.h"
>  #include "dp-packet.h"
>  #include "dpif-netdev-private-dpcls.h"
> +#include "dpif-netdev-private-dpif.h"
> +#include "dpif-netdev-private-extract.h"
>  #include "openflow/openflow.h"
>  #include "packets.h"
>  #include "odp-util.h"
> @@ -757,7 +759,7 @@ dump_invalid_packet(struct dp_packet *packet,
> const char *reason)
>   *      of interest for the flow, otherwise UINT16_MAX.
>   */
>  void
> -miniflow_extract(struct dp_packet *packet, struct netdev_flow_key *key)
> +miniflow_extract_(struct dp_packet *packet, struct netdev_flow_key
> +*key)
>  {
>      /* Add code to this function (or its callees) to extract new fields. */
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42); @@ -1112,6 +1114,36 @@
> miniflow_extract(struct dp_packet *packet, struct netdev_flow_key *key)
>      key->mf.map = mf.map;
>  }
> 
> +void
> +miniflow_extract(struct dp_packet *packet, struct netdev_flow_key *key)
> +{ #ifdef MFEX_AUTOVALIDATOR_DEFAULT
> +    static struct ovsthread_once once_enable =
> OVSTHREAD_ONCE_INITIALIZER;
> +    if (ovsthread_once_start(&once_enable)) {
> +        dpif_miniflow_extract_init();
> +        ovsthread_once_done(&once_enable);
> +    }
> +    struct dp_packet_batch packets;
> +    const struct pkt_metadata *md = &packet->md;
> +    dp_packet_batch_init(&packets);
> +    dp_packet_batch_add(&packets, packet);
> +    const uint32_t recirc_depth = *recirc_depth_get();
> +
> +    /* Currently AVX512 DPIF dont support recirculation
> +     * Once the support will be added the condition would
> +     * be removed.
> +     */
> +    if (recirc_depth) {
> +        miniflow_extract_(packet, key);
> +    } else {
> +        dpif_miniflow_extract_autovalidator(&packets, key, 1,
> +                                    odp_to_u32(md->in_port.odp_port), NULL);
> +    }
> +#else
> +    miniflow_extract_(packet, key);
> +#endif
> +}
> +
>  static ovs_be16
>  parse_dl_type(const void **datap, size_t *sizep, ovs_be16 *first_vlan_tci_p)
> { diff --git a/lib/flow.h b/lib/flow.h index ba7c3c63a..7b277275f 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -543,6 +543,10 @@ struct pkt_metadata;
>   * were extracted. */
>  void
>  miniflow_extract(struct dp_packet *packet, struct netdev_flow_key *key);
> +
> +void
> +miniflow_extract_(struct dp_packet *packet, struct netdev_flow_key
> +*key);
> +
>  void miniflow_map_init(struct miniflow *, const struct flow *);  void
> flow_wc_map(const struct flow *, struct flowmap *);  size_t
> miniflow_alloc(struct miniflow *dsts[], size_t n,
> --
> 2.25.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to