Modified the dplcs info-get command output to include
the count for different dpcls implementations.
$ovs-appctl dpif-netdev/subtable-lookup-prio-get
Available dpcls implementations:
autovalidator (Use count: 1, Priority: 5)
generic (Use count: 0, Priority: 1)
avx512_gather (Use count: 0, Priority: 3)
Test case to verify changes:
1021: PMD - dpcls configuration ok
Signed-off-by: Kumar Amber <[email protected]>
Signed-off-by: Harry van Haaren <[email protected]>
Co-authored-by: Harry van Haaren <[email protected]>
---
v3:
- Fix comments on the patch.
- Function API remains same, see discussion on OVS ML here:
"https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388737.html";
v2:
- Dependency merged rebased to master.
---
---
Documentation/topics/dpdk/bridge.rst | 16 +++----
lib/dpif-netdev-lookup.c | 72 +++++++++++++++++++++++-----
lib/dpif-netdev-lookup.h | 19 +++++++-
lib/dpif-netdev.c | 31 +++++-------
tests/pmd.at | 16 +++----
5 files changed, 106 insertions(+), 48 deletions(-)
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..8a95c84e1 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.count = 0,},
/* The default scalar C code implementation. */
{ .prio = 1,
.probe = dpcls_subtable_generic_probe,
- .name = "generic", },
+ .name = "generic",
+ .usage_cnt.count = 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.count = 0,},
#else
/* Disabling AVX512 at compile time, as compile time requirements not met.
* This could be due to a number of reasons:
@@ -93,25 +96,44 @@ 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,
+ dpcls_subtable_lookup_func old_func,
+ bool will_use_result)
{
/* Iter over each subtable impl, and get highest priority one. */
int32_t prio = -1;
+ uint32_t i;
const char *name = NULL;
+ uint32_t best_idx = 0;
dpcls_subtable_lookup_func best_func = NULL;
- for (int i = 0; i < ARRAY_SIZE(subtable_lookups); i++) {
+ for (i = 0; i < ARRAY_SIZE(subtable_lookups); i++) {
int32_t probed_prio = subtable_lookups[i].prio;
+ dpcls_subtable_lookup_func probed_func;
+
+ probed_func = subtable_lookups[i].probe(u0_bit_count,
+ u1_bit_count);
+ if (!probed_func) {
+ continue;
+ }
+
+ /* Better candidate - track this to return it later. */
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;
- }
+ best_func = probed_func;
+ best_idx = i;
+ prio = probed_prio;
+ name = subtable_lookups[i].name;
}
+
+ /* Statistics keeping, reduce old func usage count. */
+ if (probed_func == old_func) {
+ atomic_count_dec(&subtable_lookups[i].usage_cnt);
+ }
+ }
+
+ /* Update stats for usage. */
+ if (will_use_result) {
+ atomic_count_inc(&subtable_lookups[best_idx].usage_cnt);
}
VLOG_DBG("Subtable lookup function '%s' with units (%d,%d), priority %d\n",
@@ -122,3 +144,27 @@ dpcls_subtable_get_best_impl(uint32_t u0_bit_count,
uint32_t u1_bit_count)
return best_func;
}
+
+void
+dp_dpcls_impl_print_stats(struct ds *reply)
+{
+ struct dpcls_subtable_lookup_info_t *lookup_funcs = NULL;
+ int32_t 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..c0dee4ff0 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,12 +63,24 @@ 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);
+/* Lookup the best subtable lookup implementation for the given u0,u1 count.
+ * When replacing an existing lookup func, the old_func pointer is provied
+ * and statistics will be tracked accordingly and will_use parameter would
+ * be set to 1 if the result of the function is going to be used for a
+ * subtable lookup else when set to 0, the statistics will refect that the
+ * result is not being used.
+ */
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,
+ dpcls_subtable_lookup_func old_func,
+ bool will_use);
/* Retrieve the array of lookup implementations for iteration.
* On error, returns a negative number.
@@ -76,4 +89,8 @@ dpcls_subtable_get_best_impl(uint32_t u0_bit_count, uint32_t
u1_bit_count);
int32_t
dpcls_subtable_lookup_info_get(struct dpcls_subtable_lookup_info_t **out_ptr);
+/* Prints dpcls subtables in use for different implementations. */
+void
+dp_dpcls_impl_print_stats(struct ds *reply);
+
#endif /* dpif-netdev-lookup.h */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b078c2da5..194e6914f 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -911,21 +911,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);
- }
+
+ dp_dpcls_impl_print_stats(&reply);
unixctl_command_reply(conn, ds_cstr(&reply));
ds_destroy(&reply);
}
@@ -8940,6 +8928,10 @@ 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_subtable_get_best_impl(subtable->mf_bits_set_unit0,
+ subtable->mf_bits_set_unit1,
+ subtable->lookup_func, false);
ovsrcu_postpone(dpcls_subtable_destroy_cb, subtable);
}
@@ -8987,8 +8979,8 @@ 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, NULL,
+ true);
cmap_insert(&cls->subtables_map, &subtable->cmap_node, mask->hash);
/* Add the new subtable at the end of the pvector (with no hits yet) */
pvector_insert(&cls->subtables, subtable, 0);
@@ -9028,8 +9020,11 @@ 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);
+ subtable->lookup_func = dpcls_subtable_get_best_impl(u0_bits, u1_bits,
+ old_func, true);
+ if (old_func != subtable->lookup_func) {
+ subtables_changed += 1;
+ }
}
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],
--
2.25.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
