From patchwork Fri Jul 19 13:37:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Kalcok X-Patchwork-Id: 1962503 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" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=W9khDOaa; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (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 4WQW291j4mz1ySl for ; Fri, 19 Jul 2024 23:37:37 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id D40CD60EA2; Fri, 19 Jul 2024 13:37:34 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 5p8u-DzMg2vM; Fri, 19 Jul 2024 13:37:33 +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 smtp3.osuosl.org 7DC0D607FC Authentication-Results: smtp3.osuosl.org; dkim=fail reason="signature verification failed" (2048-bit key, unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=W9khDOaa Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 7DC0D607FC; Fri, 19 Jul 2024 13:37:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6DF34C0A96; Fri, 19 Jul 2024 13:37:33 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 186F6C0A96 for ; Fri, 19 Jul 2024 13:37:32 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 157C680E34 for ; Fri, 19 Jul 2024 13:37:31 +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 UBnfc94ut2Yp for ; Fri, 19 Jul 2024 13:37:29 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=185.125.188.120; helo=smtp-relay-canonical-0.canonical.com; envelope-from=martin.kalcok@canonical.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp1.osuosl.org 3805C80F0B Authentication-Results: smtp1.osuosl.org; dmarc=pass (p=none dis=none) header.from=canonical.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 3805C80F0B Authentication-Results: smtp1.osuosl.org; dkim=pass (2048-bit key, unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=W9khDOaa Received: from smtp-relay-canonical-0.canonical.com (smtp-relay-canonical-0.canonical.com [185.125.188.120]) by smtp1.osuosl.org (Postfix) with ESMTPS id 3805C80F0B for ; Fri, 19 Jul 2024 13:37:28 +0000 (UTC) Received: from omen-desktop.. (178-143-45-0.static.orange.sk [178.143.45.0]) (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 smtp-relay-canonical-0.canonical.com (Postfix) with ESMTPSA id E89F23F2E9; Fri, 19 Jul 2024 13:37:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1721396246; bh=838NXqnJbtTa12aSjhPhvumdGvHVbqQkOPzRG9ebJGI=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=W9khDOaaegEC/kHc87zH9GNQcYRgP9EpzD7A3TG1FT7YtuKr1P6GQoLBGLwjTcFqV enXAmEuxX/eFUwV2/9WZXR/qB+f2W2csBkn2koCGkrVGapMUHHQE54X9buT9AgN6bb j7w++ssVvsuULFcKETBIvaXejLkPPoBuVHI7yaS8TGkU0E0skcOsRsSf/wx8QBZKHa H+xmtwQgSDb0W9nSw5DGHBS+9DL8w0fafuClZys/3A2baw4VTPlQ5KplaHrJ8AuaQv JCT6jdpqsQ/kMWNrBXvsbNocR4NZEF6Uc1BqzpHvX9gp7aaDsKsqcoWNwVdFSS+VCz bM7jx+07OMrbg== From: Martin Kalcok To: amusil@redhat.com, dev@openvswitch.org Date: Fri, 19 Jul 2024 15:37:21 +0200 Message-Id: <20240719133721.1043327-1-martin.kalcok@canonical.com> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Subject: [ovs-dev] [Patch ovn v2] actions: Explicitly finish CT actions. 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" As discussed here [0], a couple of functions that encode CT-related actions were using older, manual, way of finishing the action. As amusil mentioned here [1], there's a shorter and more explicit way of doing it. This change replaces manual way with the more explicit aproach. [0] https://mail.openvswitch.org/pipermail/ovs-dev/2024-June/414667.html [1] https://mail.openvswitch.org/pipermail/ovs-dev/2024-April/413317.html Signed-off-by: Martin Kalcok Acked-by: Ales Musil --- Thank you for the review, Ales. I added places that I missed originally. For the unit tests, I copied your suggestion for 'ct_lb' action, and for the rest of the actions, I just pre-pended them with `ct_clear`. I hope that's enough. Martin. controller/lflow.c | 11 +++++----- lib/actions.c | 52 +++++++++++++++------------------------------- tests/ovn.at | 22 ++++++++++++++++++++ 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index b4c379044..aa77ed631 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -1795,6 +1795,7 @@ add_lb_ct_snat_hairpin_vip_flow(const struct ovn_controller_lb *lb, { uint64_t stub[1024 / 8]; struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub); + const size_t ct_offset = ofpacts.size; uint8_t address_family; if (IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)) { @@ -1811,10 +1812,6 @@ add_lb_ct_snat_hairpin_vip_flow(const struct ovn_controller_lb *lb, ct->flags = NX_CT_F_COMMIT; ct->alg = 0; - size_t nat_offset; - nat_offset = ofpacts.size; - ofpbuf_pull(&ofpacts, nat_offset); - struct ofpact_nat *nat = ofpact_put_NAT(&ofpacts); nat->flags = NX_NAT_F_SRC; nat->range_af = address_family; @@ -1828,8 +1825,10 @@ add_lb_ct_snat_hairpin_vip_flow(const struct ovn_controller_lb *lb, ? lb->hairpin_snat_ips.ipv6_addrs[0].addr : lb_vip->vip; } - ofpacts.header = ofpbuf_push_uninit(&ofpacts, nat_offset); - ofpact_finish(&ofpacts, &ct->ofpact); + + ct = ofpbuf_at_assert(&ofpacts, ct_offset, sizeof *ct); + ofpacts.header = ct; + ofpact_finish_CT(&ofpacts, &ct); struct match match = MATCH_CATCHALL_INITIALIZER; diff --git a/lib/actions.c b/lib/actions.c index e8cc0994d..9d19dd2dc 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -715,13 +715,18 @@ encode_CT_NEXT(const struct ovnact_ct_next *ct_next, const struct ovnact_encode_params *ep, struct ofpbuf *ofpacts) { + size_t ct_offset = ofpacts->size; + struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); ct->recirc_table = first_ptable(ep, ep->pipeline) + ct_next->ltable; ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE) : mf_from_id(MFF_LOG_DNAT_ZONE); ct->zone_src.ofs = 0; ct->zone_src.n_bits = 16; - ofpact_finish(ofpacts, &ct->ofpact); + + ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); + ofpacts->header = ct; + ofpact_finish_CT(ofpacts, &ct); } static void @@ -761,7 +766,6 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on, struct ofpbuf *ofpacts) { size_t ct_offset = ofpacts->size; - ofpbuf_pull(ofpacts, ct_offset); struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); ct->flags = NX_CT_F_COMMIT; @@ -776,25 +780,17 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on, * collisions at commit time between NATed and firewalled-only sessions. */ if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) { - size_t nat_offset = ofpacts->size; - ofpbuf_pull(ofpacts, nat_offset); - struct ofpact_nat *nat = ofpact_put_NAT(ofpacts); nat->flags = 0; nat->range_af = AF_UNSPEC; nat->flags |= NX_NAT_F_SRC; - ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset); - ct = ofpacts->header; } - size_t set_field_offset = ofpacts->size; - ofpbuf_pull(ofpacts, set_field_offset); - ovnacts_encode(on->nested, on->nested_len, ep, ofpacts); - ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset); - ct = ofpacts->header; - ofpact_finish(ofpacts, &ct->ofpact); - ofpbuf_push_uninit(ofpacts, ct_offset); + + ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); + ofpacts->header = ct; + ofpact_finish_CT(ofpacts, &ct); } static void @@ -1027,20 +1023,16 @@ encode_ct_nat(const struct ovnact_ct_nat *cn, enum mf_field_id zone_src, struct ofpbuf *ofpacts) { const size_t ct_offset = ofpacts->size; - ofpbuf_pull(ofpacts, ct_offset); struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); ct->recirc_table = cn->ltable + first_ptable(ep, ep->pipeline); ct->zone_src.field = mf_from_id(zone_src); ct->zone_src.ofs = 0; ct->zone_src.n_bits = 16; - ct->flags = 0; + ct->flags = cn->commit ? NX_CT_F_COMMIT : 0; ct->alg = 0; struct ofpact_nat *nat; - size_t nat_offset; - nat_offset = ofpacts->size; - ofpbuf_pull(ofpacts, nat_offset); nat = ofpact_put_NAT(ofpacts); nat->range_af = cn->family; @@ -1081,13 +1073,9 @@ encode_ct_nat(const struct ovnact_ct_nat *cn, } } - ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset); - ct = ofpacts->header; - if (cn->commit) { - ct->flags |= NX_CT_F_COMMIT; - } - ofpact_finish(ofpacts, &ct->ofpact); - ofpbuf_push_uninit(ofpacts, ct_offset); + ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); + ofpacts->header = ct; + ofpact_finish_CT(ofpacts, &ct); } static void @@ -1383,11 +1371,9 @@ encode_ct_lb(const struct ovnact_ct_lb *cl, /* ct_lb without any destinations means that this is an established * connection and we just need to do a NAT. */ const size_t ct_offset = ofpacts->size; - ofpbuf_pull(ofpacts, ct_offset); struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); struct ofpact_nat *nat; - size_t nat_offset; ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE) : mf_from_id(MFF_LOG_DNAT_ZONE); ct->zone_src.ofs = 0; @@ -1396,17 +1382,13 @@ encode_ct_lb(const struct ovnact_ct_lb *cl, ct->recirc_table = recirc_table; ct->alg = 0; - nat_offset = ofpacts->size; - ofpbuf_pull(ofpacts, nat_offset); - nat = ofpact_put_NAT(ofpacts); nat->flags = 0; nat->range_af = AF_UNSPEC; - ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset); - ct = ofpacts->header; - ofpact_finish(ofpacts, &ct->ofpact); - ofpbuf_push_uninit(ofpacts, ct_offset); + ct = ofpbuf_at_assert(ofpacts, ct_offset, sizeof *ct); + ofpacts->header = ct; + ofpact_finish_CT(ofpacts, &ct); return; } diff --git a/tests/ovn.at b/tests/ovn.at index 185ba4a21..adeff95a9 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1169,6 +1169,9 @@ ct_lb(); formats as ct_lb; encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]],nat) has prereqs ip +ct_clear; ct_lb; reg8[[7]] = 1; + encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]],nat),set_field:0x8000000000/0x8000000000->xreg4 + has prereqs ip ct_lb(192.168.1.2:80, 192.168.1.3:80); Syntax error at `192.168.1.2' expecting backends. ct_lb(backends=192.168.1.2:80,192.168.1.3:80); @@ -1263,6 +1266,9 @@ ct_lb_mark(backends=192.168.1.2:80,192.168.1.3:80; hash_fields="eth_src,eth_dst, ct_next; encodes as ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) has prereqs ip +ct_clear; ct_next; + encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG13[[0..15]]) + has prereqs ip # ct_commit ct_commit; @@ -1318,6 +1324,10 @@ ct_commit { ip4.dst = 192.168.0.1; }; reg8[[17]] = 1; ct_commit { ct_mark.blocked = 1; }; encodes as set_field:0x2000000000000/0x2000000000000->xreg4,ct(commit,zone=NXM_NX_REG13[[0..15]],exec(set_field:0x1/0x1->ct_mark)) has prereqs ip +ct_clear; ct_commit { }; next; + formats as ct_clear; ct_commit; next; + encodes as ct_clear,ct(commit,zone=NXM_NX_REG13[[0..15]]),resubmit(,oflow_in_table) + has prereqs ip # ct_commit_to_zone ct_commit_to_zone(dnat); @@ -1381,6 +1391,9 @@ ct_dnat(192.168.1.2, 1-3000); formats as ct_dnat(192.168.1.2,1-3000); encodes as ct(commit,table=oflow_in_table,zone=NXM_NX_REG11[[0..15]],nat(dst=192.168.1.2:1-3000,random)) has prereqs ip +ct_clear; ct_dnat; + encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG11[[0..15]],nat) + has prereqs ip ct_dnat(192.168.1.2, 192.168.1.3); Syntax error at `192.168.1.3' expecting Integer for port range. @@ -1415,6 +1428,9 @@ ct_dnat_in_czone(192.168.1.2, 1-3000); formats as ct_dnat_in_czone(192.168.1.2,1-3000); encodes as ct(commit,table=oflow_in_table,zone=NXM_NX_REG11[[0..15]],nat(dst=192.168.1.2:1-3000,random)) has prereqs ip +ct_clear; ct_dnat_in_czone; + encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG11[[0..15]],nat) + has prereqs ip ct_dnat_in_czone(192.168.1.2, 192.168.1.3); Syntax error at `192.168.1.3' expecting Integer for port range. @@ -1449,6 +1465,9 @@ ct_snat(192.168.1.2, 1-3000); formats as ct_snat(192.168.1.2,1-3000); encodes as ct(commit,table=oflow_in_table,zone=NXM_NX_REG12[[0..15]],nat(src=192.168.1.2:1-3000,random)) has prereqs ip +ct_clear; ct_snat; + encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG12[[0..15]],nat) + has prereqs ip ct_snat(192.168.1.2, 192.168.1.3); Syntax error at `192.168.1.3' expecting Integer for port range. @@ -1483,6 +1502,9 @@ ct_snat_in_czone(192.168.1.2, 1-3000); formats as ct_snat_in_czone(192.168.1.2,1-3000); encodes as ct(commit,table=oflow_in_table,zone=NXM_NX_REG11[[0..15]],nat(src=192.168.1.2:1-3000,random)) has prereqs ip +ct_clear; ct_snat_in_czone; + encodes as ct_clear,ct(table=oflow_in_table,zone=NXM_NX_REG11[[0..15]],nat) + has prereqs ip ct_snat_in_czone(192.168.1.2, 192.168.1.3); Syntax error at `192.168.1.3' expecting Integer for port range.