From patchwork Thu Mar 7 17:25:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Conole X-Patchwork-Id: 1909403 X-Patchwork-Delegate: i.maximets@samsung.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=G2X7RSk4; dkim-atps=neutral Authentication-Results: legolas.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=patchwork.ozlabs.org) 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 ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TrGQc11GSz1yX3 for ; Fri, 8 Mar 2024 04:25:12 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id F3C6B418D9; Thu, 7 Mar 2024 17:25:09 +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 ittIIMkAMmap; Thu, 7 Mar 2024 17:25:08 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 428CC408B5 Authentication-Results: smtp4.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=G2X7RSk4 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id 428CC408B5; Thu, 7 Mar 2024 17:25:07 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 91479C0072; Thu, 7 Mar 2024 17:25:07 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 83173C0037 for ; Thu, 7 Mar 2024 17:25:06 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 6FE32605F0 for ; Thu, 7 Mar 2024 17:25:06 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eM906em28cLi for ; Thu, 7 Mar 2024 17:25:05 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=aconole@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp3.osuosl.org 2331E60616 Authentication-Results: smtp3.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 2331E60616 Authentication-Results: smtp3.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=G2X7RSk4 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 2331E60616 for ; Thu, 7 Mar 2024 17:25:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709832303; 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=R89DWzvzQ8+Ab7avxhd6jjq5+iswlv8PXyeSdRy87/4=; b=G2X7RSk4r+3yPqpfXbIIsVajQcDHdOJPOuM0kY09fc/peeKiG2VdlxptVPZKIoKO1q6MZL Z9EXOr7wPvdgxfrx98eK4/YR3HX7dLW6n+xqhtgoYaxsCmAb3f1FNdJVKdDLlBghMAAfI3 DHcP6z/al5a4nrm0kltbbQra7t1GZp8= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-467-sHyunDqIO3mbn-2-XYq_Yg-1; Thu, 07 Mar 2024 12:25:01 -0500 X-MC-Unique: sHyunDqIO3mbn-2-XYq_Yg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6FC198007B0 for ; Thu, 7 Mar 2024 17:25:01 +0000 (UTC) Received: from RHTPC1VM0NT.lan (unknown [10.22.17.159]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A050C15772; Thu, 7 Mar 2024 17:25:01 +0000 (UTC) From: Aaron Conole To: dev@openvswitch.org Date: Thu, 7 Mar 2024 12:25:00 -0500 Message-ID: <20240307172500.318917-1-aconole@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: Dumitru Ceara Subject: [ovs-dev] [PATCH] ofproto-dpif-xlate: Fix continuations with associated metering. 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" Open vSwitch supports the ability to invoke a controller action by way of a sample action with a specified meter. In the normal case, this sample action is transparently generated during xlate processing. However, when executing via a continuation, the logic to generate the sample action when finishing the context freeze was missing. The result is that the behavior when action is 'controller(pause,meter_id=1)' does not match the behavior when action is 'controller(meter_id=1)'. OVN and other controller solutions may rely on this metering to protect the control path, so it is critical to preserve metering, whether we are doing a plain old send to controller, or a continuation. Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in "continuations".") Reported-at: https://issues.redhat.com/browse/FDP-455 Tested-by: Alex Musil Signed-off-by: Aaron Conole --- ofproto/ofproto-dpif-xlate.c | 66 +++++++++++++++++++----------------- tests/ofproto-dpif.at | 50 +++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 32 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 89f183182e..4f1e57e6d1 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5080,10 +5080,37 @@ put_controller_user_action(struct xlate_ctx *ctx, bool dont_send, bool continuation, uint32_t recirc_id, int len, enum ofp_packet_in_reason reason, + uint32_t provider_meter_id, uint16_t controller_id) { struct user_action_cookie cookie; + /* If the controller action didn't request a meter (indicated by a + * 'meter_id' argument other than NX_CTLR_NO_METER), see if one was + * configured through the "controller" virtual meter. + * + * Internally, ovs-vswitchd uses UINT32_MAX to indicate no meter is + * configured. */ + uint32_t meter_id; + if (provider_meter_id == UINT32_MAX) { + meter_id = ctx->xbridge->ofproto->up.controller_meter_id; + } else { + meter_id = provider_meter_id; + } + + size_t offset; + size_t ac_offset; + if (meter_id != UINT32_MAX) { + /* If controller meter is configured, generate clone(meter, + * userspace) action. */ + offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE); + nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, + UINT32_MAX); + ac_offset = nl_msg_start_nested(ctx->odp_actions, + OVS_SAMPLE_ATTR_ACTIONS); + nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER, meter_id); + } + memset(&cookie, 0, sizeof cookie); cookie.type = USER_ACTION_COOKIE_CONTROLLER; cookie.ofp_in_port = OFPP_NONE, @@ -5101,6 +5128,11 @@ put_controller_user_action(struct xlate_ctx *ctx, uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port); odp_put_userspace_action(pid, &cookie, sizeof cookie, ODPP_NONE, false, ctx->odp_actions, NULL); + + if (meter_id != UINT32_MAX) { + nl_msg_end_nested(ctx->odp_actions, ac_offset); + nl_msg_end_nested(ctx->odp_actions, offset); + } } static void @@ -5145,32 +5177,6 @@ xlate_controller_action(struct xlate_ctx *ctx, int len, } recirc_refs_add(&ctx->xout->recircs, recirc_id); - /* If the controller action didn't request a meter (indicated by a - * 'meter_id' argument other than NX_CTLR_NO_METER), see if one was - * configured through the "controller" virtual meter. - * - * Internally, ovs-vswitchd uses UINT32_MAX to indicate no meter is - * configured. */ - uint32_t meter_id; - if (provider_meter_id == UINT32_MAX) { - meter_id = ctx->xbridge->ofproto->up.controller_meter_id; - } else { - meter_id = provider_meter_id; - } - - size_t offset; - size_t ac_offset; - if (meter_id != UINT32_MAX) { - /* If controller meter is configured, generate clone(meter, userspace) - * action. */ - offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE); - nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, - UINT32_MAX); - ac_offset = nl_msg_start_nested(ctx->odp_actions, - OVS_SAMPLE_ATTR_ACTIONS); - nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER, meter_id); - } - /* Generate the datapath flows even if we don't send the packet-in * so that debugging more closely represents normal state. */ bool dont_send = false; @@ -5178,12 +5184,7 @@ xlate_controller_action(struct xlate_ctx *ctx, int len, dont_send = true; } put_controller_user_action(ctx, dont_send, false, recirc_id, len, - reason, controller_id); - - if (meter_id != UINT32_MAX) { - nl_msg_end_nested(ctx->odp_actions, ac_offset); - nl_msg_end_nested(ctx->odp_actions, offset); - } + reason, provider_meter_id, controller_id); } /* Creates a frozen state, and allocates a unique recirc id for the given @@ -5235,6 +5236,7 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table) put_controller_user_action(ctx, false, true, recirc_id, ctx->pause->max_len, ctx->pause->reason, + ctx->pause->provider_meter_id, ctx->pause->controller_id); } else { if (ctx->recirc_update_dp_hash) { diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index a1393f7f8e..db205a5316 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -6040,6 +6040,7 @@ m4_define([CHECK_CONTINUATION], [dnl ]) ]) + # Check that pause at the end of the pipeline works OK. # # (xlate_continuation() has a special case for no-op actions; this @@ -6160,6 +6161,7 @@ AT_CHECK([test 1 = `$PYTHON3 "$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap | wc OVS_VSWITCHD_STOP AT_CLEANUP + AT_SETUP([ofproto-dpif - continuation with conntrack]) AT_KEYWORDS([continuations pause resume]) OVS_VSWITCHD_START @@ -6195,6 +6197,54 @@ AT_CHECK([test 1 = `$PYTHON3 "$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap | wc OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - continuation with meters]) +AT_KEYWORDS([continuations pause meters]) +OVS_VSWITCHD_START +add_of_ports br0 1 2 + +dnl Add meter with id=1 +AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps bands=type=drop rate=1']) + +AT_DATA([flows.txt], [dnl +table=0 dl_dst=50:54:00:00:00:0a actions=goto_table(1) +table=1 dl_dst=50:54:00:00:00:0a actions=controller(pause,meter_id=1) +]) +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 flows.txt]) + +AT_CAPTURE_FILE([ofctl_monitor.log]) +AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log]) + +ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)' + +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 2]) +OVS_APP_EXIT_AND_WAIT([ovs-ofctl]) +AT_CHECK([cat ofctl_monitor.log], [0], [dnl +NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via action) data_len=14 (unbuffered) +vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234 +]) + +AT_CHECK([ovs-appctl revalidator/purge], [0]) +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 | ofctl_strip | sort], [0], [dnl + n_packets=1, n_bytes=14, dl_dst=50:54:00:00:00:0a actions=goto_table:1 + table=1, n_packets=1, n_bytes=14, dl_dst=50:54:00:00:00:0a actions=controller(pause,meter_id=1) +OFPST_FLOW reply (OF1.3): +]) + +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0 | ofctl_strip | sort], [0], [dnl +OFPST_METER_CONFIG reply (OF1.3): +meter=1 pktps bands= +type=drop rate=1 +]) + +AT_CHECK([ovs-ofctl -O OpenFlow13 meter-stats br0 | strip_timers], [0], [dnl +OFPST_METER reply (OF1.3) (xid=0x2): +meter:1 flow_count:0 packet_in_count:1 byte_in_count:14 duration:0.0s bands: +0: packet_count:0 byte_count:0 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - continuation with patch port]) AT_KEYWORDS([continuations pause resume]) OVS_VSWITCHD_START(