[ovs-dev] [PATCH v12 0/5] dpcls func ptrs & optimizations

[Ilya Maximets]

On 17.07.2019 16:00, Harry van Haaren wrote:
> Hey Folks,
> 
> Here a v12 of the DPCLS Function Pointer patchset, as has been
> presented at OVS Conf in Nov '18, and discussed on the ML since then.
> I'm aware of the soft-freeze for 2.12, I feel this patchset has had
> enough reviews/versions/testing to be merged in 2.12.
> 
> Thanks Ilya and Ian for review comments on v11, they should all be addressed
> in this v12. Patchset details below, summary of v11 changes as follows:
> - Reworked various minor comments (Capitals, stops, and whitespace)
> - Moved variable declarations to patch they're used in
> - Improved function documentation on probe()
> - Reduced log level from info to debug
> - See per patch --- v12 notes for details.
> 
> Given the nearing soft-freeze and branch deadlines, I'd like to see
> this get merged - and any future minor comments / improvements can be
> handled before release.
> 
> Regards, -Harry
> 
> 
> Patchset details;
> 1) Refactor dpcls_lookup and the subtable for flexibility.
> In particular, add a function pointer to the subtable
> structure, which enables "plugging-in" a lookup function
> at runtime. This enables a number of optimizations in future.
> 
> 2) and 3)
> With the function pointer in place, we refactor the existing
> dpcls_lookup matching code into its own function, and later its
> own file. To split it to its own file requires making various
> dpcls data-structures available in dpif-netdev-private.h.
> 
> 4)
> Refactor the existing code, to favour compute of flat arrays of
> miniflows, instead of the MACRO based iteration. This simplifies
> the code itself, and makes future optimizations possible due to
> simplified loop structures, and loop trip counts pass in via
> function arguments. See commit message for more details.
> 
> 5)
> This patch implements a select few specialized functions, for handling
> miniflows with 5-1, 4-1, and 4-0 miniflow unit bit patterns. More of
> these types of functions can (and should) be added to accelerate other
> patterns of subtable lookups! See commit message for more details.
> 
> 
> Harry van Haaren (5):
>   dpif-netdev: Implement function pointers/subtable
>   dpif-netdev: Move dpcls lookup structures to .h
>   dpif-netdev: Split out generic lookup function
>   dpif-netdev: Refactor generic implementation
>   dpif-netdev: Add specialized generic scalar functions
> 
>  NEWS                             |   4 +
>  lib/automake.mk                  |   2 +
>  lib/dpif-netdev-lookup-generic.c | 290 +++++++++++++++++++++++++++++++
>  lib/dpif-netdev-private.h        | 129 ++++++++++++++
>  lib/dpif-netdev.c                | 197 ++++++++++-----------
>  5 files changed, 525 insertions(+), 97 deletions(-)
>  create mode 100644 lib/dpif-netdev-lookup-generic.c
>  create mode 100644 lib/dpif-netdev-private.h
> 


I performed a few tests with v11 of this patch-set on my usual setup to check
performance of the new generic (non-optimized) implementation. The result is that
new generic implementation is ~5% faster for me than current master (it was 12%
for optimized lookup functions) which is good.
The code looks OK for me in general. I still don't really like the fact that
dpcls depends on the internal structure of a flowmap, but we, probably, could
deal with that while we have a build time assertion. Hope, we'll have some better
implementation with the same level of performance in the future.

Best regards, Ilya Maximets.