From patchwork Mon Aug 1 13:10:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1662547 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" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Z4e6eiTO; dkim-atps=neutral Authentication-Results: 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=) 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 RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LxJQt1YdWz9sCD for ; Mon, 1 Aug 2022 23:11:01 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 79A0760B35; Mon, 1 Aug 2022 13:10:56 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 79A0760B35 Authentication-Results: smtp3.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=Z4e6eiTO 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 YvZB27xJw_9t; Mon, 1 Aug 2022 13:10:52 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id B3D50605A3; Mon, 1 Aug 2022 13:10:51 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org B3D50605A3 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 83E04C0033; Mon, 1 Aug 2022 13:10:51 +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 E8ECFC002D for ; Mon, 1 Aug 2022 13:10:49 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id B1B53414CB for ; Mon, 1 Aug 2022 13:10:49 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org B1B53414CB 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=Z4e6eiTO 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 5NqBrvoaPEBN for ; Mon, 1 Aug 2022 13:10:45 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org C1F6741295 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id C1F6741295 for ; Mon, 1 Aug 2022 13:10:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659359443; 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=KSwHvkFjZvirZvnQ1sXcdfC3t11tcwqKIuTQDkKD4PY=; b=Z4e6eiTOy/mtigKXAGB6sSodeFc2Fp9QI/VUVyOqlaESrSykNhyftr+p5ys9zYNpM77PJB LYWt68fIFrBO9yS3Q80j0QKhk2KDZkod94SHYqsGyLf5RXWpn4+eQ8dmS8RU0vyU8iOL7b wNFIqwxC6hUcIHTxC+VaRKtmDWy3Wu8= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-591-soau-fM4NwColjiIn9-9kw-1; Mon, 01 Aug 2022 09:10:42 -0400 X-MC-Unique: soau-fM4NwColjiIn9-9kw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 441FF3821C08 for ; Mon, 1 Aug 2022 13:10:42 +0000 (UTC) Received: from amusil.redhat.com (unknown [10.40.193.25]) by smtp.corp.redhat.com (Postfix) with ESMTP id AC1CB906B9; Mon, 1 Aug 2022 13:10:41 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Mon, 1 Aug 2022 15:10:40 +0200 Message-Id: <20220801131040.971525-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v2] ofproto-dpif-xlate: Optimize the clone for patch ports 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" When the packet was traveling through patch port boundary OvS would check if any of the actions is reversible, if not it would clone the packet. However, the check was only at the first level of the second bridge. That caused some issues when the packet had gone through more actions, some of them might have been irreversible. Get the information from the generated nlattr. If any of the actions is irreversible wrap the patch port actions in clone. Signed-off-by: Ales Musil --- v2: Address comments from Ilya. --- lib/odp-util.c | 121 +++++++++++++++++++++++++++++++++++ lib/odp-util.h | 1 + ofproto/ofproto-dpif-xlate.c | 48 +++++++++++++- 3 files changed, 168 insertions(+), 2 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index ba5be4bb3..19cbdf411 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -8768,3 +8768,124 @@ commit_odp_actions(const struct flow *flow, struct flow *base, return slow1 ? slow1 : slow2; } + +static inline bool +nlattr_action_is_reversible(const uint16_t type) +{ + switch (type) { + case OVS_ACTION_ATTR_CT: + case OVS_ACTION_ATTR_CT_CLEAR: + case OVS_ACTION_ATTR_TRUNC: + case OVS_ACTION_ATTR_PUSH_ETH: + case OVS_ACTION_ATTR_POP_ETH: + case OVS_ACTION_ATTR_PUSH_NSH: + case OVS_ACTION_ATTR_POP_NSH: + case OVS_ACTION_ATTR_METER: + case OVS_ACTION_ATTR_TUNNEL_PUSH: + case OVS_ACTION_ATTR_TUNNEL_POP: + case OVS_ACTION_ATTR_DROP: + return false; + + case OVS_ACTION_ATTR_UNSPEC: + case OVS_ACTION_ATTR_OUTPUT: + case OVS_ACTION_ATTR_USERSPACE: + case OVS_ACTION_ATTR_SET: + case OVS_ACTION_ATTR_PUSH_VLAN: + case OVS_ACTION_ATTR_POP_VLAN: + case OVS_ACTION_ATTR_SAMPLE: + case OVS_ACTION_ATTR_RECIRC: + case OVS_ACTION_ATTR_HASH: + case OVS_ACTION_ATTR_SET_MASKED: + case OVS_ACTION_ATTR_CLONE: + case OVS_ACTION_ATTR_CHECK_PKT_LEN: + case OVS_ACTION_ATTR_LB_OUTPUT: + case OVS_ACTION_ATTR_ADD_MPLS: + case OVS_ACTION_ATTR_PUSH_MPLS: + case OVS_ACTION_ATTR_POP_MPLS: + default: + return true; + } +} + +static bool +odp_cpl_contains_irreversible_actions(const struct nlattr *attr) +{ + static const struct nl_policy ovs_cpl_policy[] = { + [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = {.type = NL_A_U16}, + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = {.type = NL_A_NESTED}, + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL] = {.type = NL_A_NESTED}, + }; + struct nlattr *a[ARRAY_SIZE(ovs_cpl_policy)]; + + if (!nl_parse_nested(attr, ovs_cpl_policy, a, ARRAY_SIZE(a))) { + return false; + } + + const struct nlattr *greater = + a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER]; + const struct nlattr *less = + a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL]; + const void *greater_data = nl_attr_get(greater); + const void *less_data = nl_attr_get(less); + size_t greater_len = nl_attr_get_size(greater); + size_t less_len = nl_attr_get_size(less); + + return odp_contains_irreversible_action(greater_data, greater_len) || + odp_contains_irreversible_action(less_data, less_len); +} + +static bool +odp_sample_contains_irreversible_actions(const struct nlattr *attr) +{ + static const struct nl_policy ovs_sample_policy[] = { + [OVS_SAMPLE_ATTR_PROBABILITY] = {.type = NL_A_U32}, + [OVS_SAMPLE_ATTR_ACTIONS] = {.type = NL_A_NESTED} + }; + struct nlattr *a[ARRAY_SIZE(ovs_sample_policy)]; + + if (!nl_parse_nested(attr, ovs_sample_policy, a, ARRAY_SIZE(a))) { + return false; + } + + const struct nlattr *actions = a[OVS_SAMPLE_ATTR_ACTIONS]; + const void *actions_data = nl_attr_get(actions); + size_t actions_len = nl_attr_get_size(actions); + + return odp_contains_irreversible_action(actions_data, actions_len); +} + +/* Check if any of the actions in the ofpbuf is irreversible. */ +bool +odp_contains_irreversible_action(const void *attrs, size_t attrs_len) +{ + const struct nlattr *attr; + int left; + + NL_ATTR_FOR_EACH(attr, left, attrs, attrs_len) { + uint16_t type = attr->nla_type; + + /* Skip "clone" because it already encapsulates irreversible * + * actions. */ + if (type == OVS_ACTION_ATTR_CLONE) { + continue; + } + + /* Check nested actions of "check_packet_len". */ + if (type == OVS_ACTION_ATTR_CHECK_PKT_LEN && + odp_cpl_contains_irreversible_actions(attr)) { + return true; + } + + /* Check nested actions of "sample". */ + if (type == OVS_ACTION_ATTR_SAMPLE && + odp_sample_contains_irreversible_actions(attr)) { + return true; + } + + /* Check any other action. */ + if (!nlattr_action_is_reversible(type)) { + return true; + } + } + return false; +} diff --git a/lib/odp-util.h b/lib/odp-util.h index a1d0d0fba..d842974ba 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -373,6 +373,7 @@ void odp_put_pop_eth_action(struct ofpbuf *odp_actions); void odp_put_push_eth_action(struct ofpbuf *odp_actions, const struct eth_addr *eth_src, const struct eth_addr *eth_dst); +bool odp_contains_irreversible_action(const void *attrs, size_t attrs_len); struct attr_len_tbl { int len; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index fda802e83..8323a790b 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3861,6 +3861,34 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx, const struct flow *flow, co xport_in->xbundle->protected && xport_out->xbundle->protected); } +/* Wraps the patch port odp_actions ofpbuf in clone. */ +static void +patch_port_add_clone(struct xlate_ctx *ctx, struct ofpbuf *patch_port_actions) { + size_t offset, ac_offset; + + if (ctx->xbridge->support.clone) { /* Use clone action */ + /* Use clone action as datapath clone. */ + offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_CLONE); + nl_msg_put(ctx->odp_actions, patch_port_actions->data, + patch_port_actions->size); + nl_msg_end_non_empty_nested(ctx->odp_actions, offset); + } else if (ctx->xbridge->support.sample_nesting > 3) { + /* Use sample action as datapath clone. */ + offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE); + ac_offset = nl_msg_start_nested(ctx->odp_actions, + OVS_SAMPLE_ATTR_ACTIONS); + nl_msg_put(ctx->odp_actions, patch_port_actions->data, + patch_port_actions->size); + if (nl_msg_end_non_empty_nested(ctx->odp_actions, ac_offset)) { + nl_msg_cancel_nested(ctx->odp_actions, offset); + } else { + nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, + UINT32_MAX); /* 100% probability. */ + nl_msg_end_nested(ctx->odp_actions, offset); + } + } +} + /* Function handles when a packet is sent from one bridge to another bridge. * * The bridges are internally connected, either with patch ports or with @@ -3889,7 +3917,12 @@ patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev, struct ofpbuf old_action_set = ctx->action_set; struct ovs_list *old_trace = ctx->xin->trace; uint64_t actset_stub[1024 / 8]; + struct ofpbuf *old_odp_actions = ctx->odp_actions; + uint64_t patch_port_actions_stub[1024 / 8]; + struct ofpbuf patch_port_actions = + OFPBUF_STUB_INITIALIZER(patch_port_actions_stub); + ctx->odp_actions = &patch_port_actions; ofpbuf_use_stub(&ctx->stack, new_stack, sizeof new_stack); ofpbuf_use_stub(&ctx->action_set, actset_stub, sizeof actset_stub); flow->in_port.ofp_port = out_dev->ofp_port; @@ -3920,7 +3953,7 @@ patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev, xport_rstp_forward_state(out_dev)) { xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true, - false, is_last_action, clone_xlate_actions); + false, is_last_action, do_xlate_actions); if (!ctx->freezing) { xlate_action_set(ctx); } @@ -3935,7 +3968,7 @@ patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev, mirror_mask_t old_mirrors2 = ctx->mirrors; xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true, - false, is_last_action, clone_xlate_actions); + false, is_last_action, do_xlate_actions); ctx->mirrors = old_mirrors2; ctx->base_flow = old_base_flow; ctx->odp_actions->size = old_size; @@ -3945,6 +3978,17 @@ patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev, } } + ctx->odp_actions = old_odp_actions; + if (!is_last_action && + odp_contains_irreversible_action(patch_port_actions.data, + patch_port_actions.size)) { + patch_port_add_clone(ctx, &patch_port_actions); + } else { + nl_msg_put(ctx->odp_actions, patch_port_actions.data, + patch_port_actions.size); + } + ofpbuf_uninit(&patch_port_actions); + ctx->xin->trace = old_trace; if (independent_mirrors) { ctx->mirrors = old_mirrors;