Hi Cian,
> -----Original Message----- > From: Ferriter, Cian <[email protected]> > Sent: Thursday, September 29, 2022 8:58 PM > To: Amber, Kumar <[email protected]>; [email protected] > Cc: [email protected]; [email protected]; Stokes, Ian > <[email protected]>; [email protected]; Van Haaren, Harry > <[email protected]> > Subject: RE: [PATCH v5 7/9] dpif-mfex: Change mfex fn pointer prototype to > include md_is_valid. > > > > > -----Original Message----- > > From: Amber, Kumar <[email protected]> > > Sent: Friday 26 August 2022 00:31 > > To: [email protected] > > Cc: [email protected]; [email protected]; Ferriter, Cian > > <[email protected]>; Stokes, Ian <[email protected]>; > > [email protected]; Van Haaren, Harry <[email protected]>; > > Amber, Kumar <[email protected]> > > 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 <[email protected]> > > --- > > 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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
