> Hi Ian,
> 
> Thanks for the review. My responses are inline.
> 
> > -----Original Message-----
> > From: Stokes, Ian <[email protected]>
> > Sent: Wednesday 2 June 2021 20:08
> > To: Ferriter, Cian <[email protected]>; [email protected]; Van
> Haaren, Harry <[email protected]>
> > Cc: [email protected]
> > Subject: RE: [ovs-dev] [v12 12/16] dpif-netdev/dpcls: Specialize more 
> > subtable
> signatures.
> >
> > > From: Harry van Haaren <[email protected]>
> > >
> > > This commit adds more subtables to be specialized. The traffic
> > > pattern here being matched is VXLAN traffic subtables, which commonly
> > > have (5,3), (9,1) and (9,4) subtable fingerprints.
> > >
> >
> > Thanks Cian/Harry.
> >
> > My thoughts here is that this patch could have been applied previously 
> > without
> the DPIF framework?
> >
> > Is there another technical reason besides the refactor why it is included 
> > in this
> patch series?
> >
> 
> Correct, this could be applied independently of the DPIF framework.

Sure, for the moment we can leave it here due to the refactor but there'd be no 
blockers from my POV if it was submitted directly to master separately.

Ian
> 
> > Other than that LGTM.
> >
> > Regards
> > Ian
> >
> > > Signed-off-by: Harry van Haaren <[email protected]>
> > >
> > > ---
> > >
> > > v8: Add NEWS entry.
> > > ---
> > >  NEWS                                   | 2 ++
> > >  lib/dpif-netdev-lookup-avx512-gather.c | 6 ++++++
> > >  lib/dpif-netdev-lookup-generic.c       | 6 ++++++
> > >  3 files changed, 14 insertions(+)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index 6a07cd362..98943c404 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -12,6 +12,8 @@ Post-v2.15.0
> > >         packets. It supports partial HWOL, EMC, SMC and DPCLS lookups.
> > >       * Add commands to get and set the dpif implementations.
> > >       * Enable AVX512 optimized DPCLS to search subtables with larger
> > > miniflows.
> > > +     * Add more specialized DPCLS subtables to cover common rules,
> > > enhancing
> > > +       the lookup performance.
> > >     - ovs-ctl:
> > >       * New option '--no-record-hostname' to disable hostname 
> > > configuration
> > >         in ovsdb on startup.
> > > diff --git a/lib/dpif-netdev-lookup-avx512-gather.c 
> > > b/lib/dpif-netdev-lookup-
> > > avx512-gather.c
> > > index 1bfabdcb1..7adf29914 100644
> > > --- a/lib/dpif-netdev-lookup-avx512-gather.c
> > > +++ b/lib/dpif-netdev-lookup-avx512-gather.c
> > > @@ -299,6 +299,9 @@ avx512_lookup_impl(struct dpcls_subtable
> *subtable,
> > >          return avx512_lookup_impl(subtable, keys_map, keys, rules, U0, 
> > > U1);   \
> > >      }                                                                    
> > >      \
> > >
> > > +DECLARE_OPTIMIZED_LOOKUP_FUNCTION(9, 4)
> > > +DECLARE_OPTIMIZED_LOOKUP_FUNCTION(9, 1)
> > > +DECLARE_OPTIMIZED_LOOKUP_FUNCTION(5, 3)
> > >  DECLARE_OPTIMIZED_LOOKUP_FUNCTION(5, 1)
> > >  DECLARE_OPTIMIZED_LOOKUP_FUNCTION(4, 1)
> > >  DECLARE_OPTIMIZED_LOOKUP_FUNCTION(4, 0)
> > > @@ -331,6 +334,9 @@ dpcls_subtable_avx512_gather_probe(uint32_t
> > > u0_bits, uint32_t u1_bits)
> > >          return NULL;
> > >      }
> > >
> > > +    CHECK_LOOKUP_FUNCTION(9, 4);
> > > +    CHECK_LOOKUP_FUNCTION(9, 1);
> > > +    CHECK_LOOKUP_FUNCTION(5, 3);
> > >      CHECK_LOOKUP_FUNCTION(5, 1);
> > >      CHECK_LOOKUP_FUNCTION(4, 1);
> > >      CHECK_LOOKUP_FUNCTION(4, 0);
> > > diff --git a/lib/dpif-netdev-lookup-generic.c b/lib/dpif-netdev-lookup-
> > > generic.c
> > > index e3b6be4b6..6c74ac3a1 100644
> > > --- a/lib/dpif-netdev-lookup-generic.c
> > > +++ b/lib/dpif-netdev-lookup-generic.c
> > > @@ -282,6 +282,9 @@ dpcls_subtable_lookup_generic(struct
> > > dpcls_subtable *subtable,
> > >          return lookup_generic_impl(subtable, keys_map, keys, rules, U0, 
> > > U1);  \
> > >      }                                                                    
> > >      \
> > >
> > > +DECLARE_OPTIMIZED_LOOKUP_FUNCTION(9, 4)
> > > +DECLARE_OPTIMIZED_LOOKUP_FUNCTION(9, 1)
> > > +DECLARE_OPTIMIZED_LOOKUP_FUNCTION(5, 3)
> > >  DECLARE_OPTIMIZED_LOOKUP_FUNCTION(5, 1)
> > >  DECLARE_OPTIMIZED_LOOKUP_FUNCTION(4, 1)
> > >  DECLARE_OPTIMIZED_LOOKUP_FUNCTION(4, 0)
> > > @@ -303,6 +306,9 @@ dpcls_subtable_generic_probe(uint32_t u0_bits,
> > > uint32_t u1_bits)
> > >  {
> > >      dpcls_subtable_lookup_func f = NULL;
> > >
> > > +    CHECK_LOOKUP_FUNCTION(9, 4);
> > > +    CHECK_LOOKUP_FUNCTION(9, 1);
> > > +    CHECK_LOOKUP_FUNCTION(5, 3);
> > >      CHECK_LOOKUP_FUNCTION(5, 1);
> > >      CHECK_LOOKUP_FUNCTION(4, 1);
> > >      CHECK_LOOKUP_FUNCTION(4, 0);
> > > --
> > > 2.31.1
> > >
> > > _______________________________________________
> > > dev mailing list
> > > [email protected]
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

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

Reply via email to