Hi Cian,
> -----Original Message----- > From: Ferriter, Cian <cian.ferri...@intel.com> > Sent: Thursday, September 29, 2022 8:58 PM > To: Amber, Kumar <kumar.am...@intel.com>; ovs-dev@openvswitch.org > Cc: echau...@redhat.com; i.maxim...@ovn.org; Stokes, Ian > <ian.sto...@intel.com>; f...@sysclose.org; Van Haaren, Harry > <harry.van.haa...@intel.com> > Subject: RE: [PATCH v5 7/9] dpif-mfex: Change mfex fn pointer prototype to > include md_is_valid. > > > > > -----Original Message----- > > From: Amber, Kumar <kumar.am...@intel.com> > > Sent: Friday 26 August 2022 00:31 > > To: ovs-dev@openvswitch.org > > Cc: echau...@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 v5 7/9] dpif-mfex: Change mfex fn pointer prototype to > include md_is_valid. > > > > The md_is_valid parameter is passed from DPIF to MFEX to allow mfex > > functions to detect the tunneling and decide the processing of Inner > > packets in static predictable branches. > > > > Signed-off-by: Kumar Amber <kumar.am...@intel.com> > > --- > > lib/dpif-netdev-avx512.c | 3 ++- > > lib/dpif-netdev-extract-avx512.c | 9 +++++---- > > lib/dpif-netdev-extract-study.c | 6 ++++-- > > lib/dpif-netdev-private-extract.c | 6 ++++-- > > lib/dpif-netdev-private-extract.h | 13 ++++++++----- > > 5 files changed, 23 insertions(+), 14 deletions(-) > > > > Hey Amber, > > I'm OK with the API changes in this patch if we need them, but after looking > at the later 2 patches in the series, I can see that you're using another > check > in the heart of the MFEX AVX512 code (mfex_avx512_process()): > /* Dummy pmd dont always pass correct md_is_valid and hence > * need to check the tunnel data to ensure correct behaviour. > */ > bool tunnel = flow_tnl_dst_is_set(&md->tunnel); > > If we need to use the flow_tnl_dst_is_set() check, then it makes me think we > don't need the md_is_valid bool at all. Can you check if we can get away with > just using the flow_tnl_dst_is_set() check instead? If so, we don't need this > patch at all as we could use the flow_tnl_dst_is_set() check for both the > study and AVX512 MFEX implementations. > > Hopefully that makes sense. > Thanks, > Cian Using md_is_valid is better as it deterministic for the compiler. And I have removed bool tunnel = flow_tnl_dst_is_set(&md->tunnel); as it is not required. Regards Amber _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
