[ovs-dev,v5] dpcls: Change info-get function to fetch dpcls usage stats.

Message ID	20211215041511.4097090-1-kumar.amber@intel.com
State	Superseded
Headers	show Return-Path: <ovs-dev-bounces@openvswitch.org> From: Kumar Amber <kumar.amber@intel.com> To: ovs-dev@openvswitch.org Date: Wed, 15 Dec 2021 09:45:11 +0530 Message-Id: <20211215041511.4097090-1-kumar.amber@intel.com> In-Reply-To: <20211202100556.3745682-1-kumar.amber@intel.com> References: <20211202100556.3745682-1-kumar.amber@intel.com> MIME-Version: 1.0 Cc: fbl@sysclose.org, Kumar Amber <kumar.amber@intel.com> Subject: [ovs-dev] [PATCH v5] dpcls: Change info-get function to fetch dpcls usage stats. Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" <ovs-dev-bounces@openvswitch.org>
Series	[ovs-dev,v5] dpcls: Change info-get function to fetch dpcls usage stats. \| expand [ovs-dev,v5] dpcls: Change info-get function to fetch dpcls usage stats.

Context	Check	Description
ovsrobot/apply-robot	success	apply and check: success
ovsrobot/github-robot-_Build_and_Test	success	github build: passed

diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst index f645b9ade..63a54da1c 100644 --- a/Documentation/topics/dpdk/bridge.rst +++ b/Documentation/topics/dpdk/bridge.rst @@ -156,10 +156,10 @@ OVS provides multiple implementations of dpcls. The following command enables the user to check what implementations are available in a running instance :: $ ovs-appctl dpif-netdev/subtable-lookup-prio-get - Available lookup functions (priority : name) - 0 : autovalidator - 1 : generic - 0 : avx512_gather + Available dpcls implementations: + autovalidator (Use count: 1, Priority: 5) + generic (Use count: 0, Priority: 1) + avx512_gather (Use count: 0, Priority: 3) To set the priority of a lookup function, run the ``prio-set`` command :: @@ -172,10 +172,10 @@ function due to the command being run. To verify the prioritization, re-run the get command, note the updated priority of the ``avx512_gather`` function :: $ ovs-appctl dpif-netdev/subtable-lookup-prio-get - Available lookup functions (priority : name) - 0 : autovalidator - 1 : generic - 5 : avx512_gather + Available dpcls implementations: + autovalidator (Use count: 0, Priority: 0) + generic (Use count: 0, Priority: 0) + avx512_gather (Use count: 1, Priority: 5) If two lookup functions have the same priority, the first one in the list is chosen, and the 2nd occurance of that priority is not used. Put in logical diff --git a/lib/dpif-netdev-lookup.c b/lib/dpif-netdev-lookup.c index bd0a99abe..e641e4028 100644 --- a/lib/dpif-netdev-lookup.c +++ b/lib/dpif-netdev-lookup.c @@ -36,18 +36,21 @@ static struct dpcls_subtable_lookup_info_t subtable_lookups[] = { { .prio = 0, #endif .probe = dpcls_subtable_autovalidator_probe, - .name = "autovalidator", }, + .name = "autovalidator", + .usage_cnt = ATOMIC_COUNT_INIT(0), }, /* The default scalar C code implementation. */ { .prio = 1, .probe = dpcls_subtable_generic_probe, - .name = "generic", }, + .name = "generic", + .usage_cnt = ATOMIC_COUNT_INIT(0), }, #if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__) /* Only available on x86_64 bit builds with SSE 4.2 used for OVS core. */ { .prio = 0, .probe = dpcls_subtable_avx512_gather_probe, - .name = "avx512_gather", }, + .name = "avx512_gather", + .usage_cnt = ATOMIC_COUNT_INIT(0), }, #else /* Disabling AVX512 at compile time, as compile time requirements not met. * This could be due to a number of reasons: @@ -64,7 +67,7 @@ static struct dpcls_subtable_lookup_info_t subtable_lookups[] = { #endif }; -int32_t +int dpcls_subtable_lookup_info_get(struct dpcls_subtable_lookup_info_t **out_ptr) { if (out_ptr == NULL) { @@ -76,7 +79,7 @@ dpcls_subtable_lookup_info_get(struct dpcls_subtable_lookup_info_t **out_ptr) } /* sets the priority of the lookup function with "name". */ -int32_t +int dpcls_subtable_set_prio(const char *name, uint8_t priority) { for (int i = 0; i < ARRAY_SIZE(subtable_lookups); i++) { @@ -93,32 +96,81 @@ dpcls_subtable_set_prio(const char *name, uint8_t priority) } dpcls_subtable_lookup_func -dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count) +dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count, + struct dpcls_subtable_lookup_info_t **info) { - /* Iter over each subtable impl, and get highest priority one. */ - int32_t prio = -1; - const char *name = NULL; + struct dpcls_subtable_lookup_info_t *best_info = NULL; dpcls_subtable_lookup_func best_func = NULL; + int prio = -1; + /* Iter over each subtable impl, and get highest priority one. */ for (int i = 0; i < ARRAY_SIZE(subtable_lookups); i++) { - int32_t probed_prio = subtable_lookups[i].prio; - if (probed_prio > prio) { - dpcls_subtable_lookup_func probed_func; - probed_func = subtable_lookups[i].probe(u0_bit_count, - u1_bit_count); - if (probed_func) { - best_func = probed_func; - prio = probed_prio; - name = subtable_lookups[i].name; - } + struct dpcls_subtable_lookup_info_t *impl_info = &subtable_lookups[i]; + dpcls_subtable_lookup_func probed_func; + + if (impl_info->prio <= prio) { + continue; } - } - VLOG_DBG("Subtable lookup function '%s' with units (%d,%d), priority %d\n", - name, u0_bit_count, u1_bit_count, prio); + probed_func = subtable_lookups[i].probe(u0_bit_count, + u1_bit_count); + if (!probed_func) { + continue; + } + + best_func = probed_func; + best_info = impl_info; + prio = impl_info->prio; + } /* Programming error - we must always return a valid func ptr. */ - ovs_assert(best_func != NULL); + ovs_assert(best_func != NULL && best_info != NULL); + VLOG_DBG("Subtable lookup function '%s' with units (%d,%d), priority %d\n", + best_info->name, u0_bit_count, u1_bit_count, prio); + + if (info) { + *info = best_info; + } return best_func; } + +void +dpcls_info_inc_usage(struct dpcls_subtable_lookup_info_t *info) +{ + if (info) { + atomic_count_inc(&info->usage_cnt); + } +} + +void +dpcls_info_dec_usage(struct dpcls_subtable_lookup_info_t *info) +{ + if (info) { + atomic_count_dec(&info->usage_cnt); + } +} + +void +dpcls_impl_print_stats(struct ds *reply) +{ + struct dpcls_subtable_lookup_info_t *lookup_funcs = NULL; + int count = dpcls_subtable_lookup_info_get(&lookup_funcs); + + /* Add all DPCLS functions to reply string. */ + ds_put_cstr(reply, "Available dpcls implementations:\n"); + + for (int i = 0; i < count; i++) { + ds_put_format(reply, " %s (Use count: %d, Priority: %d", + lookup_funcs[i].name, + atomic_count_get(&lookup_funcs[i].usage_cnt), + lookup_funcs[i].prio); + + if (ds_last(reply) == ' ') { + ds_put_cstr(reply, "none"); + } + + ds_put_cstr(reply, ")\n"); + } + +} diff --git a/lib/dpif-netdev-lookup.h b/lib/dpif-netdev-lookup.h index 59f51faa0..7f124a46e 100644 --- a/lib/dpif-netdev-lookup.h +++ b/lib/dpif-netdev-lookup.h @@ -20,6 +20,7 @@ #include <config.h> #include "dpif-netdev.h" #include "dpif-netdev-private-dpcls.h" +#include "dpif-netdev-private-thread.h" /* Function to perform a probe for the subtable bit fingerprint. * Returns NULL if not valid, or a valid function pointer to call for this @@ -62,18 +63,29 @@ struct dpcls_subtable_lookup_info_t { /* Human readable name, used in setting subtable priority commands */ const char *name; + + /* Counter which holds the usage count of each implementations. */ + atomic_count usage_cnt; }; -int32_t dpcls_subtable_set_prio(const char *name, uint8_t priority); +int dpcls_subtable_set_prio(const char *name, uint8_t priority); +void dpcls_info_inc_usage(struct dpcls_subtable_lookup_info_t *info); +void dpcls_info_dec_usage(struct dpcls_subtable_lookup_info_t *info); +/* Lookup the best subtable lookup implementation for the given u0,u1 count. */ dpcls_subtable_lookup_func -dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count); +dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t u1_bit_count, + struct dpcls_subtable_lookup_info_t **info); /* Retrieve the array of lookup implementations for iteration. * On error, returns a negative number. * On success, returns the size of the arrays pointed to by the out parameter. */ -int32_t +int dpcls_subtable_lookup_info_get(struct dpcls_subtable_lookup_info_t **out_ptr); +/* Prints dpcls subtables in use for different implementations. */ +void +dpcls_impl_print_stats(struct ds *reply); + #endif /* dpif-netdev-lookup.h */ diff --git a/lib/dpif-netdev-private-dpcls.h b/lib/dpif-netdev-private-dpcls.h index 7c4a840cb..06e896bef 100644 --- a/lib/dpif-netdev-private-dpcls.h +++ b/lib/dpif-netdev-private-dpcls.h @@ -85,6 +85,7 @@ struct dpcls_subtable { * used for the lookup) then this can point at an optimized version of * the lookup function for this particular subtable. */ dpcls_subtable_lookup_func lookup_func; + struct dpcls_subtable_lookup_info_t *lookup_func_info; /* Caches the masks to match a packet to, reducing runtime calculations. */ uint64_t *mf_masks; diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index a790df5fd..f55fd08ff 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -912,21 +912,9 @@ dpif_netdev_subtable_lookup_get(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) { - /* Get a list of all lookup functions. */ - struct dpcls_subtable_lookup_info_t *lookup_funcs = NULL; - int32_t count = dpcls_subtable_lookup_info_get(&lookup_funcs); - if (count < 0) { - unixctl_command_reply_error(conn, "error getting lookup names"); - return; - } - - /* Add all lookup functions to reply string. */ struct ds reply = DS_EMPTY_INITIALIZER; - ds_put_cstr(&reply, "Available lookup functions (priority : name)\n"); - for (int i = 0; i < count; i++) { - ds_put_format(&reply, " %d : %s\n", lookup_funcs[i].prio, - lookup_funcs[i].name); - } + + dpcls_impl_print_stats(&reply); unixctl_command_reply(conn, ds_cstr(&reply)); ds_destroy(&reply); } @@ -8972,6 +8960,7 @@ dpcls_destroy_subtable(struct dpcls *cls, struct dpcls_subtable *subtable) pvector_remove(&cls->subtables, subtable); cmap_remove(&cls->subtables_map, &subtable->cmap_node, subtable->mask.hash); + dpcls_info_dec_usage(subtable->lookup_func_info); ovsrcu_postpone(dpcls_subtable_destroy_cb, subtable); } @@ -9019,7 +9008,9 @@ dpcls_create_subtable(struct dpcls *cls, const struct netdev_flow_key *mask) * The function is guaranteed to always return a valid implementation, and * possibly an ISA optimized, and/or specialized implementation. */ - subtable->lookup_func = dpcls_subtable_get_best_impl(unit0, unit1); + subtable->lookup_func = dpcls_subtable_get_best_impl( + unit0, unit1, &subtable->lookup_func_info); + dpcls_info_inc_usage(subtable->lookup_func_info); cmap_insert(&cls->subtables_map, &subtable->cmap_node, mask->hash); /* Add the new subtable at the end of the pvector (with no hits yet) */ @@ -9060,8 +9051,22 @@ dpcls_subtable_lookup_reprobe(struct dpcls *cls) uint32_t u0_bits = subtable->mf_bits_set_unit0; uint32_t u1_bits = subtable->mf_bits_set_unit1; void *old_func = subtable->lookup_func; - subtable->lookup_func = dpcls_subtable_get_best_impl(u0_bits, u1_bits); - subtables_changed += (old_func != subtable->lookup_func); + struct dpcls_subtable_lookup_info_t *old_info; + + old_info = subtable->lookup_func_info; + subtable->lookup_func = dpcls_subtable_get_best_impl( + u0_bits, u1_bits, &subtable->lookup_func_info); + + if (old_func != subtable->lookup_func) { + subtables_changed += 1; + } + + if (old_info != subtable->lookup_func_info) { + /* In theory, functions can be shared between implementations, so + * do an explicit check on the function info structures. */ + dpcls_info_dec_usage(old_info); + dpcls_info_inc_usage(subtable->lookup_func_info); + } } pvector_publish(pvec); diff --git a/tests/pmd.at b/tests/pmd.at index c875a744f..e61bb27d3 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -1091,11 +1091,11 @@ AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy-pmd]) AT_CHECK([ovs-vsctl show], [], [stdout]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep generic], [], [dnl - 1 : generic + generic (Use count: 0, Priority: 1) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep autovalidator], [], [dnl - 0 : autovalidator + autovalidator (Use count: 0, Priority: 0) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 3], [0], [dnl @@ -1103,7 +1103,7 @@ Lookup priority change affected 0 dpcls ports and 0 subtables. ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep autovalidator], [], [dnl - 3 : autovalidator + autovalidator (Use count: 0, Priority: 3) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 4], [0], [dnl @@ -1111,7 +1111,7 @@ Lookup priority change affected 0 dpcls ports and 0 subtables. ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep generic], [], [dnl - 4 : generic + generic (Use count: 0, Priority: 4) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 8], [0], [dnl @@ -1119,7 +1119,7 @@ Lookup priority change affected 0 dpcls ports and 0 subtables. ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep generic], [], [dnl - 8 : generic + generic (Use count: 0, Priority: 8) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 8], [0], [dnl @@ -1127,7 +1127,7 @@ Lookup priority change affected 0 dpcls ports and 0 subtables. ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep autovalidator], [], [dnl - 8 : autovalidator + autovalidator (Use count: 0, Priority: 8) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 0], [0], [dnl @@ -1135,7 +1135,7 @@ Lookup priority change affected 0 dpcls ports and 0 subtables. ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep generic], [], [dnl - 0 : generic + generic (Use count: 0, Priority: 0) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 255], [0], [dnl @@ -1143,7 +1143,7 @@ Lookup priority change affected 0 dpcls ports and 0 subtables. ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep generic], [], [dnl - 255 : generic + generic (Use count: 0, Priority: 255) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic -1], [2],

[ovs-dev,v5] dpcls: Change info-get function to fetch dpcls usage stats.

Checks

Commit Message

Comments

Patch