From patchwork Wed Dec 15 04:15:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Amber X-Patchwork-Id: 1568010 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=dDLFcHen; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JDMsR51w8z9t6g for ; Wed, 15 Dec 2021 15:36:57 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id B1B8E4089F; Wed, 15 Dec 2021 04:36:54 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OuuD527d7WxD; Wed, 15 Dec 2021 04:36:53 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 769C640197; Wed, 15 Dec 2021 04:36:52 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 304E4C002F; Wed, 15 Dec 2021 04:36:52 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 357F8C0012 for ; Wed, 15 Dec 2021 04:36:50 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 1DC82401C0 for ; Wed, 15 Dec 2021 04:36:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id thAFRsqjmdop for ; Wed, 15 Dec 2021 04:36:47 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by smtp4.osuosl.org (Postfix) with ESMTPS id D25AE40197 for ; Wed, 15 Dec 2021 04:36:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1639543007; x=1671079007; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=xvuNUa9X7BNDISn4UCp40WyFJnR7kEccnMyWJRy2lOo=; b=dDLFcHenzk4fXcisnPmSRShPACgHiVz2+Bv1pHszAcp0droSkcPIC0Qy qKmw+QxiqeXWkgQnVH9CiJUx+4htKRZiPB30qjESpQMNxxiK/G60kuzae yZ+jQiryCUg/xRXvVIof9+ZXvCLPgWdRd1HxqRQyNRgfttQ16ukyrBbOa hFRCURAV0fH7nsOrmGZyA32T6bynpm/TMYrOuD8NlW7Goe/06IpXMNSnD Zpcrw6yWGDYBKQmsGHrX6J1vAY6uk1QHeR4BZAFazW84ryHkL2d012PfR g/tV8ItTbULZMA3womGlKJW9FPfFvfRZfQ+DCSAKu64NZLZgA4rGCP4R1 A==; X-IronPort-AV: E=McAfee;i="6200,9189,10198"; a="236682208" X-IronPort-AV: E=Sophos;i="5.88,207,1635231600"; d="scan'208";a="236682208" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Dec 2021 20:36:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.88,207,1635231600"; d="scan'208";a="518594051" Received: from bmca4bf01706bbf.iind.intel.com (HELO localhost.localdomain) ([10.190.213.111]) by orsmga008.jf.intel.com with ESMTP; 14 Dec 2021 20:36:43 -0800 From: Kumar Amber To: ovs-dev@openvswitch.org Date: Wed, 15 Dec 2021 09:45:11 +0530 Message-Id: <20211215041511.4097090-1-kumar.amber@intel.com> X-Mailer: git-send-email 2.25.1 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 Subject: [ovs-dev] [PATCH v5] dpcls: Change info-get function to fetch dpcls usage stats. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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 Signed-off-by: Harry van Haaren Signed-off-by: Eelco Chaudron Co-authored-by: Harry van Haaren Co-authored-by: Eelco Chaudron Acked-by: Eelco Chaudron --- v5: - change the info-incr and decr APIs. - Reduce the complexity of dpcls stats APIs. v4: - Fix comments on the patch. - Change API from an overloaded method of counting, to returning the old and new subtable structs. This allows the caller to identify the modified subtable implementations, and update the statistics accordingly. 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 | 98 +++++++++++++++++++++------- lib/dpif-netdev-lookup.h | 18 ++++- lib/dpif-netdev-private-dpcls.h | 1 + lib/dpif-netdev.c | 39 ++++++----- tests/pmd.at | 16 ++--- 6 files changed, 129 insertions(+), 59 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..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 #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],