diff options
| -rw-r--r-- | Documentation/topics/dpdk/bridge.rst | 20 | ||||
| -rw-r--r-- | lib/dpif-netdev-lookup.c | 98 | ||||
| -rw-r--r-- | lib/dpif-netdev-lookup.h | 18 | ||||
| -rw-r--r-- | lib/dpif-netdev-private-dpcls.h | 1 | ||||
| -rw-r--r-- | lib/dpif-netdev.c | 40 | ||||
| -rw-r--r-- | tests/pmd.at | 32 |
6 files changed, 138 insertions, 71 deletions
diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst index ceee91015..1f626c7c2 100644 --- a/Documentation/topics/dpdk/bridge.rst +++ b/Documentation/topics/dpdk/bridge.rst @@ -179,11 +179,11 @@ these CPU ISA additions are available, and to allow the user to enable them. 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 + $ ovs-appctl dpif-netdev/subtable-lookup-info-get + 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:: @@ -195,11 +195,11 @@ above indicates that one subtable of one DPCLS port is has changed its lookup 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 + $ ovs-appctl dpif-netdev/subtable-lookup-info-get + Available dpcls implementations: + autovalidator (Use count: 1, Priority: 5) + generic (Use count: 0, Priority: 1) + avx512_gather (Use count: 0, Priority: 3) 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 0d5da73c7..2a9279437 100644 --- a/lib/dpif-netdev-private-dpcls.h +++ b/lib/dpif-netdev-private-dpcls.h @@ -87,6 +87,7 @@ struct dpcls_subtable { * can be used at any time by a PMD thread, so it's declared as an atomic * here to prevent garbage from being read. */ ATOMIC(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 21277b236..0e7a7d16e 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1000,21 +1000,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); } @@ -1617,7 +1605,7 @@ dpif_netdev_init(void) "[lookup_func] [prio]", 2, 2, dpif_netdev_subtable_lookup_set, NULL); - unixctl_command_register("dpif-netdev/subtable-lookup-prio-get", "", + unixctl_command_register("dpif-netdev/subtable-lookup-info-get", "", 0, 0, dpif_netdev_subtable_lookup_get, NULL); unixctl_command_register("dpif-netdev/dpif-impl-set", @@ -9704,6 +9692,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); } @@ -9754,7 +9743,9 @@ dpcls_create_subtable(struct dpcls *cls, const struct netdev_flow_key *mask) * by the PMD thread. */ atomic_init(&subtable->lookup_func, - dpcls_subtable_get_best_impl(unit0, unit1)); + 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) */ @@ -9799,12 +9790,23 @@ 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; - + struct dpcls_subtable_lookup_info_t *old_info; + old_info = subtable->lookup_func_info; /* Set the subtable lookup function atomically to avoid garbage data * being read by the PMD thread. */ atomic_store_relaxed(&subtable->lookup_func, - dpcls_subtable_get_best_impl(u0_bits, u1_bits)); - subtables_changed += (old_func != 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); + } } return subtables_changed; diff --git a/tests/pmd.at b/tests/pmd.at index 0a451f33c..e6b173dab 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -1130,60 +1130,60 @@ OVS_VSWITCHD_START([], [], [], [--dummy-numa 0,0]) 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 +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep generic], [], [dnl + generic (Use count: 0, Priority: 1) ]) -AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get | grep autovalidator], [], [dnl - 0 : autovalidator +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep autovalidator], [], [dnl + autovalidator (Use count: 0, Priority: 0) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 3], [0], [dnl 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 +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep autovalidator], [], [dnl + autovalidator (Use count: 0, Priority: 3) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 4], [0], [dnl 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 +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep generic], [], [dnl + generic (Use count: 0, Priority: 4) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 8], [0], [dnl 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 +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep generic], [], [dnl + generic (Use count: 0, Priority: 8) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 8], [0], [dnl 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 +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep autovalidator], [], [dnl + autovalidator (Use count: 0, Priority: 8) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 0], [0], [dnl 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 +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep generic], [], [dnl + generic (Use count: 0, Priority: 0) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 255], [0], [dnl 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 +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-info-get | grep generic], [], [dnl + generic (Use count: 0, Priority: 255) ]) AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic -1], [2], |