From patchwork Wed Aug 14 08:49:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eelco Chaudron X-Patchwork-Id: 1972251 X-Patchwork-Delegate: echaudro@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VMrjxsRq; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WkMQX0FKgz1yfZ for ; Wed, 14 Aug 2024 18:50:11 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id D7571811D2; Wed, 14 Aug 2024 08:50:09 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id jtuBQnhAebMz; Wed, 14 Aug 2024 08:50:08 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.9.56; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 717E380D42 Authentication-Results: smtp1.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VMrjxsRq Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 717E380D42; Wed, 14 Aug 2024 08:50:08 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D75A7C0A97; Wed, 14 Aug 2024 08:50:07 +0000 (UTC) X-Original-To: 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 6CCF5C0A96 for ; Wed, 14 Aug 2024 08:50:07 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 43444402FE for ; Wed, 14 Aug 2024 08:50:07 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 2Nudwx_H2Auh for ; Wed, 14 Aug 2024 08:50:06 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.133.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=echaudro@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp4.osuosl.org BF13D4038E Authentication-Results: smtp4.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org BF13D4038E Authentication-Results: smtp4.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VMrjxsRq Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id BF13D4038E for ; Wed, 14 Aug 2024 08:50:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723625403; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=d6kD7kicfHpUZY3wwn8NC1RI5IvBl/S8BXtMEBKkxWo=; b=VMrjxsRqQbWCzgIfHctLp61SYBijRR62ls6bMTEo8/FRG8Of2VnUaHO73pM1FnQm1X7220 KwlaJhMPB/aYpt7G33W89mFwDQQsK1kURRh5M/THqbtM4wbtZIrbE5OPPXZbXMrmyeTmqp M+c3A+spOteWG61pgrTpHX427a057ZQ= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-650-n0CqftlsOk-6XUPxt3-thw-1; Wed, 14 Aug 2024 04:50:02 -0400 X-MC-Unique: n0CqftlsOk-6XUPxt3-thw-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 102371944F06; Wed, 14 Aug 2024 08:50:01 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.194.72]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 283A71955F66; Wed, 14 Aug 2024 08:49:59 +0000 (UTC) From: Eelco Chaudron To: dev@openvswitch.org Date: Wed, 14 Aug 2024 10:49:42 +0200 Message-ID: <3519b107b74a3301783492cc649eec26cdeef087.1723625382.git.echaudro@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v3] ofproto-dpif-upcall: Avoid stale ukeys leaks. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" It is observed in some environments that there are much more ukeys than actual DP flows. For example: $ ovs-appctl upcall/show system@ovs-system: flows : (current 7) (avg 6) (max 117) (limit 2125) offloaded flows : 525 dump duration : 1063ms ufid enabled : true 23: (keys 3612) 24: (keys 3625) 25: (keys 3485) The revalidator threads are busy revalidating the stale ukeys leading to high CPU and long dump duration. This patch records the last dump timestamp. If the flow was not dumped for at least twice the idle time, we can assume the datapath flow now longer exists and the ukey should be deleted. Reported-by: Roi Dayan Co-authored-by: Han Zhou Co-authored-by: Roi Dayan Signed-off-by: Han Zhou Signed-off-by: Roi Dayan Signed-off-by: Eelco Chaudron Acked-by: Simon Horman --- v3: Rewrote fix to use actual dump state, and added a tests case. --- ofproto/ofproto-dpif-upcall.c | 14 ++++- tests/system-offloads-traffic.at | 64 ++++++++++++++++++++ utilities/usdt-scripts/flow_reval_monitor.py | 2 + 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 4d39bc5a7..5c130b694 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -57,6 +57,7 @@ COVERAGE_DEFINE(dumped_inconsistent_flow); COVERAGE_DEFINE(dumped_new_flow); COVERAGE_DEFINE(handler_duplicate_upcall); COVERAGE_DEFINE(revalidate_missed_dp_flow); +COVERAGE_DEFINE(revalidate_missing_dp_flow); COVERAGE_DEFINE(ukey_dp_change); COVERAGE_DEFINE(ukey_invalid_stat_reset); COVERAGE_DEFINE(ukey_replace_contention); @@ -278,6 +279,7 @@ enum flow_del_reason { FDR_BAD_ODP_FIT, /* Bad ODP flow fit. */ FDR_FLOW_IDLE, /* Flow idle timeout. */ FDR_FLOW_LIMIT, /* Kill all flows condition reached. */ + FDR_FLOW_MISSING_DP, /* Flow is missing from the datapath. */ FDR_FLOW_WILDCARDED, /* Flow needs a narrower wildcard mask. */ FDR_NO_OFPROTO, /* Bridge not found. */ FDR_PURGE, /* User requested flow deletion. */ @@ -315,6 +317,7 @@ struct udpif_key { struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/ const char *dp_layer OVS_GUARDED; /* Last known dp_layer. */ long long int created OVS_GUARDED; /* Estimate of creation time. */ + long long int last_dumped OVS_GUARDED; /* Flow last dump time. */ uint64_t dump_seq OVS_GUARDED; /* Tracks udpif->dump_seq. */ uint64_t reval_seq OVS_GUARDED; /* Tracks udpif->reval_seq. */ enum ukey_state state OVS_GUARDED; /* Tracks ukey lifetime. */ @@ -1825,6 +1828,7 @@ ukey_create__(const struct nlattr *key, size_t key_len, ukey->state_thread = ovsthread_id_self(); ukey->state_where = OVS_SOURCE_LOCATOR; ukey->created = ukey->flow_time = time_msec(); + ukey->last_dumped = 0; memset(&ukey->stats, 0, sizeof ukey->stats); ukey->stats.used = used; ukey->dp_layer = NULL; @@ -2456,7 +2460,14 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey, log_unexpected_stats_jump(ukey, stats); } - if (need_revalidate) { + if ((ukey->last_dumped ? ukey->last_dumped : ukey->created) + < udpif->dpif->current_ms - (2 * ofproto_max_idle)) { + /* If the flow was not dumped for at least twice the idle time, + * we can assume the datapath flow now longer exists and the ukey + * should be deleted. */ + COVERAGE_INC(revalidate_missing_dp_flow); + *del_reason = FDR_FLOW_MISSING_DP; + } else if (need_revalidate) { if (should_revalidate(udpif, ukey, push.n_packets)) { if (!ukey->xcache) { ukey->xcache = xlate_cache_new(); @@ -2890,6 +2901,7 @@ revalidate(struct revalidator *revalidator) continue; } + ukey->last_dumped = now; ukey->offloaded = f->attrs.offloaded; if (!ukey->dp_layer || (!dpif_synced_dp_layers(udpif->dpif) diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index d1da33d96..a9416cbce 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -93,6 +93,70 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows : [[1-9]]"], [0], [i OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([offloads - Forced removed datapath entries]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") +AT_CHECK([ovs-appctl dpctl/dump-flows], [0], [ignore]) + +AT_CHECK([ovs-appctl vlog/set ofproto_dpif_upcall:dbg]) + +dnl Set idle timeout to 1 second. +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:max-idle=1000]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -W 2 10.1.1.2 | FORMAT_PING], [0], [dnl +10 packets transmitted, 10 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | DUMP_CLEAN_SORTED], [0], [dnl +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:0.001s, actions:output +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9, bytes:756, used:0.001s, actions:output +]) + +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep -q "arp" ], [0]) + +dnl Delete IPv4 entries, but keep the ARP ones. +AT_CHECK([tc filter del dev ovs-p0 ingress protocol ip pref 2]) +AT_CHECK([tc filter del dev ovs-p1 ingress protocol ip pref 2]) + +dnl Bring the remote ports down to avoid traffic. +AT_CHECK([ip -n at_ns0 link set p0 down]) +AT_CHECK([ip -n at_ns1 link set p1 down]) + +dnl Wait until the ARP flow has timed out. +OVS_WAIT_UNTIL([test `ovs-appctl dpctl/dump-flows type=tc,offloaded | \ + grep "arp" | wc -l` -eq 0]) + +dnl Set max-idle to 10 ms, so we are sure the max-idle * 2 has been reached. +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:max-idle=10]) + +dnl Add and delete a port to force the revalidation. +for i in `seq 3`; do + REV_RECONFIGURE=$(ovs-appctl coverage/read-counter rev_reconfigure) + AT_CHECK([ovs-vsctl add-port br0 ovs-p2 -- \ + set interface ovs-p2 type=internal]) + AT_CHECK([ovs-vsctl del-port ovs-p2 ]) + OVS_WAIT_UNTIL([test `ovs-appctl coverage/read-counter rev_reconfigure` \ + -gt $REV_RECONFIGURE]) +done + +dnl Wait for another full round of revalidation. After this all ukeys should +dnl be gone. +AT_CHECK([ovs-appctl revalidator/wait], [0]) +AT_CHECK([ovs-appctl revalidator/wait], [0]) + +dnl Make sure no more ukeys exists. +AT_CHECK([ovs-appctl upcall/show | grep '(keys' | awk '{print $3}' | \ + grep -qv '0)'], [1]) + +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to flow_del (No such file or directory)/d"]) +AT_CLEANUP + AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - offloads disabled]) AT_KEYWORDS([ingress_policing]) OVS_CHECK_TC_QDISC() diff --git a/utilities/usdt-scripts/flow_reval_monitor.py b/utilities/usdt-scripts/flow_reval_monitor.py index 28479a565..1de06c73b 100755 --- a/utilities/usdt-scripts/flow_reval_monitor.py +++ b/utilities/usdt-scripts/flow_reval_monitor.py @@ -249,6 +249,7 @@ FdrReasons = IntEnum( "FDR_BAD_ODP_FIT", "FDR_FLOW_IDLE", "FDR_FLOW_LIMIT", + "FDR_FLOW_MISSING_DP", "FDR_FLOW_WILDCARDED", "FDR_NO_OFPROTO", "FDR_PURGE", @@ -265,6 +266,7 @@ FdrReasonStrings = { FdrReasons.FDR_BAD_ODP_FIT: "Bad ODP flow fit", FdrReasons.FDR_FLOW_IDLE: "Flow idle timeout", FdrReasons.FDR_FLOW_LIMIT: "Kill all flows condition reached", + FdrReasons.FDR_FLOW_MISSING_DP: "Flow is missing from the datapath", FdrReasons.FDR_FLOW_WILDCARDED: "Flow needs a narrower wildcard mask", FdrReasons.FDR_NO_OFPROTO: "Bridge not found", FdrReasons.FDR_PURGE: "User requested flow deletion",