Hi Cian, Please find my replies inline.
> -----Original Message----- > From: Ferriter, Cian <[email protected]> > Sent: Monday, April 25, 2022 5:12 PM > To: Amber, Kumar <[email protected]>; [email protected] > Cc: Stokes, Ian <[email protected]>; [email protected]; > [email protected]; [email protected]; [email protected]; Van Haaren, > Harry <[email protected]> > Subject: RE: [PATCH v2 4/4] miniflow_extract: Add autovalidator support to > miniflow_extract. > > > > > -----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? > I tried the suggested changes but our unit tests still fails due to con-track thus Recirc_depth is the best option to avoid tunnels until we support recirculation in DPIF. > > + 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()'. > Fixed in V3. Regards Amber > > + } > > +#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
