[ovs-dev] [v12 12/16] dpif-netdev/dpcls: Specialize more subtable signatures.

[Ferriter, Cian]

Hi Ian,

Thanks for the review. My responses are inline.

> -----Original Message-----
> From: Stokes, Ian <ian.stokes at intel.com>
> Sent: Wednesday 2 June 2021 20:08
> To: Ferriter, Cian <cian.ferriter at intel.com>; ovs-dev at openvswitch.org; Van Haaren, Harry <harry.van.haaren at intel.com>
> Cc: i.maximets at ovn.org
> Subject: RE: [ovs-dev] [v12 12/16] dpif-netdev/dpcls: Specialize more subtable signatures.
> 
> > From: Harry van Haaren <harry.van.haaren at intel.com>
> >
> > 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.

> Other than that LGTM.
> 
> Regards
> Ian
> 
> > Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
> >
> > ---
> >
> > 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
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev