> -----Original Message-----
> From: Amber, Kumar <[email protected]>
> Sent: Sunday 27 March 2022 08:09
> To: [email protected]
> Cc: Stokes, Ian <[email protected]>; [email protected]; 
> [email protected]; [email protected];
> Ferriter, Cian <[email protected]>; [email protected]; Van Haaren, Harry
> <[email protected]>; Amber, Kumar <[email protected]>
> Subject: [PATCH v2 4/4] miniflow_extract: 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 <[email protected]>
> ---
>  lib/dpif-netdev-private-extract.c |  4 ++--
>  lib/flow.c                        | 24 ++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/dpif-netdev-private-extract.c 
> b/lib/dpif-netdev-private-extract.c
> index 8538d069f..0d7091caa 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);
>  }
> diff --git a/lib/flow.c b/lib/flow.c
> index 127de2d7a..ddec31523 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -37,6 +37,7 @@
>  #include "dp-packet.h"
>  #include "dpif-netdev-private-thread.h"
>  #include "dpif-netdev-private-dpcls.h"
> +#include "dpif-netdev-private-extract.h"
>  #include "openflow/openflow.h"
>  #include "packets.h"
>  #include "odp-util.h"
> @@ -1121,7 +1122,30 @@ miniflow_extract_(struct dp_packet *packet, struct 
> netdev_flow_key *key)
>  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) {

I'm just wondering if there is a way to avoid getting and checking the 
recirc_depth. This would avoid the need for patch 1/4 in this series. What 
about the below check instead?

    if (flow_tnl_dst_is_set(&md->tunnel)) {

This is the check that happens inside the generic miniflow extract 
implementation. Do you think this will work instead?

> +        miniflow_extract_(packet, key);
> +    } else {
> +        dpif_miniflow_extract_autovalidator(&packets, key, 1,
> +                                    odp_to_u32(md->in_port.odp_port), NULL);

We need to be careful about the 'NULL' being passed in here. We need to add 
checks to the use of the 'struct dp_netdev_pmd_thread *pmd_handle' in 
'dpif_miniflow_extract_autovalidator()'.

> +    }
> +#else
>      miniflow_extract_(packet, key);
> +#endif
>  }
>  static ovs_be16
>  parse_dl_type(const void **datap, size_t *sizep, ovs_be16 *first_vlan_tci_p)
> --
> 2.25.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to