From patchwork Fri Jan 10 09:34:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1220907 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: 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=S0yNHZJW; dkim-atps=neutral Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47vHsY3cqCz9s1x for ; Fri, 10 Jan 2020 20:35:44 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id D21F487456; Fri, 10 Jan 2020 09:35:42 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tpIT7LqRP91J; Fri, 10 Jan 2020 09:35:40 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 059C3864F6; Fri, 10 Jan 2020 09:35:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E83CFC1D7D; Fri, 10 Jan 2020 09:35:39 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id EF046C0881 for ; Fri, 10 Jan 2020 09:35:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id D85B786AEC for ; Fri, 10 Jan 2020 09:35:38 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id juc5Ek07eraD for ; Fri, 10 Jan 2020 09:35:38 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by fraxinus.osuosl.org (Postfix) with ESMTPS id BE1598623B for ; Fri, 10 Jan 2020 09:35:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1578648936; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=qAoaAQG4b/38WX1sNla/saoatkIgxYC2RDFTo6UE9es=; b=S0yNHZJW1ZnSlt+SJSdgRoJMLLxhAUDyZwdffVr3TZAsCWM9PTtwcxHCvag0VgvvD+WDCI 5itShGLrI75rVrJ3pqQy9Ppn3mzBRXbYBqvw1yOQDydqaxvl6RwkgF1geOi+Qd2qPybh5D OwYmo6CHzLcLAKRuzUEfSWaxR9T+9mk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-250-QFi-Xm_vMc-RHdlphXYqww-1; Fri, 10 Jan 2020 04:35:34 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7FA00DB21 for ; Fri, 10 Jan 2020 09:35:33 +0000 (UTC) Received: from dceara.remote.csb (ovpn-117-126.ams2.redhat.com [10.36.117.126]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9F14D60C88 for ; Fri, 10 Jan 2020 09:35:32 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 10 Jan 2020 10:34:43 +0100 Message-Id: <1578648883-1145-1-git-send-email-dceara@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-MC-Unique: QFi-Xm_vMc-RHdlphXYqww-1 X-Mimecast-Spam-Score: 0 Subject: [ovs-dev] [PATCH v2] ofproto-dpif-trace: Improve NAT tracing. 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" When ofproto/trace detects a recirc action it resumes execution at the specified next table. However, if the ct action performs SNAT/DNAT, e.g., ct(commit,nat(src=1.1.1.1:4000),table=42), the src/dst IPs and ports in the oftrace_recirc_node->flow field are not updated. This leads to misleading outputs from ofproto/trace as real packets would actually first get NATed and might match different flows when recirculated. Assume the first IP/port from the NAT src/dst action will be used by conntrack for the translation and update the oftrace_recirc_node->flow accordingly. This is not entirely correct as conntrack might choose a different IP/port but the result is more realistic than before. This fix covers new connections. However, for reply traffic that executes actions of the form ct(nat, table=42) we still don't update the flow as we don't have any information about conntrack state when tracing. Also move the oftrace_recirc_node processing out of ofproto_trace() and to its own function, ofproto_trace_recirc_node() for better readability/ Signed-off-by: Dumitru Ceara --- v2: - Address Ben's comments: - Move trace node flow code replacement to ofproto_trace_*(). - Add outputs describing the NAT actions. - Move recirc node processing to its own function for better readability (ofproto_trace_recirc_node). --- ofproto/ofproto-dpif-trace.c | 116 +++++++++++++++++++++++++++++++++---------- ofproto/ofproto-dpif-trace.h | 2 + ofproto/ofproto-dpif-xlate.c | 6 ++- tests/ofproto-dpif.at | 36 ++++++++++++++ 4 files changed, 133 insertions(+), 27 deletions(-) diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c index 8ae8a22..5f1a05c 100644 --- a/ofproto/ofproto-dpif-trace.c +++ b/ofproto/ofproto-dpif-trace.c @@ -86,6 +86,7 @@ oftrace_node_destroy(struct oftrace_node *node) bool oftrace_add_recirc_node(struct ovs_list *recirc_queue, enum oftrace_recirc_type type, const struct flow *flow, + const struct ofpact_nat *ofn, const struct dp_packet *packet, uint32_t recirc_id, const uint16_t zone) { @@ -101,6 +102,7 @@ oftrace_add_recirc_node(struct ovs_list *recirc_queue, node->flow = *flow; node->flow.recirc_id = recirc_id; node->flow.ct_zone = zone; + node->nat_act = ofn; node->packet = packet ? dp_packet_clone(packet) : NULL; return true; @@ -179,6 +181,25 @@ oftrace_node_print_details(struct ds *output, } } +static void +oftrace_print_ip_flow(const struct flow *flow, int af, struct ds *output) +{ + if (af == AF_INET) { + ds_put_format(output, "nw_src="IP_FMT",tp_src=%"PRIu16"," + "nw_dst="IP_FMT",tp_dst=%"PRIu16, + IP_ARGS(flow->nw_src), ntohs(flow->tp_src), + IP_ARGS(flow->nw_dst), ntohs(flow->tp_dst)); + } else if (af == AF_INET6) { + ds_put_cstr(output, "ipv6_src="); + ipv6_format_addr_bracket(&flow->ipv6_src, output, true); + ds_put_format(output, ",tp_src=%"PRIu16, ntohs(flow->tp_src)); + ds_put_cstr(output, ",ipv6_dst="); + ipv6_format_addr_bracket(&flow->ipv6_dst, output, true); + ds_put_format(output, ",tp_dst=%"PRIu16, ntohs(flow->tp_dst)); + } + ds_put_char(output, '\n'); +} + /* Parses the 'argc' elements of 'argv', ignoring argv[0]. The following * forms are supported: * @@ -638,6 +659,75 @@ execute_actions_except_outputs(struct dpif *dpif, } static void +ofproto_trace_recirc_node(struct oftrace_recirc_node *node, + struct ovs_list *next_ct_states, + struct ds *output) +{ + ds_put_cstr(output, "\n\n"); + ds_put_char_multiple(output, '=', 79); + ds_put_format(output, "\nrecirc(%#"PRIx32")", node->recirc_id); + + if (next_ct_states && node->type == OFT_RECIRC_CONNTRACK) { + uint32_t ct_state; + if (ovs_list_is_empty(next_ct_states)) { + ct_state = CS_TRACKED | CS_NEW; + ds_put_cstr(output, " - resume conntrack with default " + "ct_state=trk|new (use --ct-next to customize)"); + } else { + ct_state = oftrace_pop_ct_state(next_ct_states); + struct ds s = DS_EMPTY_INITIALIZER; + format_flags(&s, ct_state_to_string, ct_state, '|'); + ds_put_format(output, " - resume conntrack with ct_state=%s", + ds_cstr(&s)); + ds_destroy(&s); + } + node->flow.ct_state = ct_state; + } + ds_put_char(output, '\n'); + + /* If there's any snat/dnat information assume we always translate to + * the first IP/port to make sure we don't match on incorrect flows later + * on. + */ + if (node->nat_act) { + const struct ofpact_nat *ofn = node->nat_act; + + ds_put_cstr(output, "Replacing src/dst IP/ports to simulate NAT:\n"); + ds_put_cstr(output, " Initial flow: "); + oftrace_print_ip_flow(&node->flow, ofn->range_af, output); + + if (ofn->flags & NX_NAT_F_SRC) { + if (ofn->range_af == AF_INET) { + node->flow.nw_src = ofn->range.addr.ipv4.min; + } else if (ofn->range_af == AF_INET6) { + memcpy(&node->flow.ipv6_src, &ofn->range.addr.ipv6.min, + sizeof node->flow.ipv6_src); + } + + if (ofn->range_af != AF_UNSPEC && ofn->range.proto.min) { + node->flow.tp_src = htons(ofn->range.proto.min); + } + } + if (ofn->flags & NX_NAT_F_DST) { + if (ofn->range_af == AF_INET) { + node->flow.nw_dst = ofn->range.addr.ipv4.min; + } else if (ofn->range_af == AF_INET6) { + memcpy(&node->flow.ipv6_dst, &ofn->range.addr.ipv6.min, + sizeof node->flow.ipv6_dst); + } + + if (ofn->range_af != AF_UNSPEC && ofn->range.proto.min) { + node->flow.tp_dst = htons(ofn->range.proto.min); + } + } + ds_put_cstr(output, " Modified flow: "); + oftrace_print_ip_flow(&node->flow, ofn->range_af, output); + } + ds_put_char_multiple(output, '=', 79); + ds_put_cstr(output, "\n\n"); +} + +static void ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow, const struct dp_packet *packet, struct ovs_list *recirc_queue, const struct ofpact ofpacts[], size_t ofpacts_len, @@ -729,31 +819,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow, struct oftrace_recirc_node *recirc_node; LIST_FOR_EACH_POP (recirc_node, node, &recirc_queue) { - ds_put_cstr(output, "\n\n"); - ds_put_char_multiple(output, '=', 79); - ds_put_format(output, "\nrecirc(%#"PRIx32")", - recirc_node->recirc_id); - - if (next_ct_states && recirc_node->type == OFT_RECIRC_CONNTRACK) { - uint32_t ct_state; - if (ovs_list_is_empty(next_ct_states)) { - ct_state = CS_TRACKED | CS_NEW; - ds_put_cstr(output, " - resume conntrack with default " - "ct_state=trk|new (use --ct-next to customize)"); - } else { - ct_state = oftrace_pop_ct_state(next_ct_states); - struct ds s = DS_EMPTY_INITIALIZER; - format_flags(&s, ct_state_to_string, ct_state, '|'); - ds_put_format(output, " - resume conntrack with ct_state=%s", - ds_cstr(&s)); - ds_destroy(&s); - } - recirc_node->flow.ct_state = ct_state; - } - ds_put_char(output, '\n'); - ds_put_char_multiple(output, '=', 79); - ds_put_cstr(output, "\n\n"); - + ofproto_trace_recirc_node(recirc_node, next_ct_states, output); ofproto_trace__(ofproto, &recirc_node->flow, recirc_node->packet, &recirc_queue, ofpacts, ofpacts_len, output); oftrace_recirc_node_destroy(recirc_node); diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h index 63dbb50..4b04f17 100644 --- a/ofproto/ofproto-dpif-trace.h +++ b/ofproto/ofproto-dpif-trace.h @@ -73,6 +73,7 @@ struct oftrace_recirc_node { uint32_t recirc_id; struct flow flow; struct dp_packet *packet; + const struct ofpact_nat *nat_act; }; /* A node within a next_ct_states list. */ @@ -91,6 +92,7 @@ struct oftrace_node *oftrace_report(struct ovs_list *, enum oftrace_node_type, const char *text); bool oftrace_add_recirc_node(struct ovs_list *recirc_queue, enum oftrace_recirc_type, const struct flow *, + const struct ofpact_nat *, const struct dp_packet *, uint32_t recirc_id, const uint16_t zone); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 4407f9c..1946af5 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -4974,7 +4974,8 @@ compose_recirculate_and_fork(struct xlate_ctx *ctx, uint8_t table, if (OVS_UNLIKELY(ctx->xin->trace) && recirc_id) { if (oftrace_add_recirc_node(ctx->xin->recirc_queue, OFT_RECIRC_CONNTRACK, &ctx->xin->flow, - ctx->xin->packet, recirc_id, zone)) { + ctx->ct_nat_action, ctx->xin->packet, + recirc_id, zone)) { xlate_report(ctx, OFT_DETAIL, "A clone of the packet is forked to " "recirculate. The forked pipeline will be resumed at " "table %u.", table); @@ -6163,7 +6164,6 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc, put_ct_label(&ctx->xin->flow, ctx->odp_actions, ctx->wc); put_ct_helper(ctx, ctx->odp_actions, ofc); put_ct_nat(ctx); - ctx->ct_nat_action = NULL; nl_msg_end_nested(ctx->odp_actions, ct_offset); ctx->wc->masks.ct_mark = old_ct_mark_mask; @@ -6174,6 +6174,8 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc, compose_recirculate_and_fork(ctx, ofc->recirc_table, zone); } + ctx->ct_nat_action = NULL; + /* The ct_* fields are only available in the scope of the 'recirc_table' * call chain. */ flow_clear_conntrack(&ctx->xin->flow); diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 88acc4b..e67978d 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -10598,6 +10598,42 @@ AT_CHECK([tail -1 stdout], [0], OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - nat - ofproto/trace]) +OVS_VSWITCHD_START + +add_of_ports br0 1 2 3 + +flow="in_port=1,udp,nw_src=1.1.1.1,nw_dst=1.1.1.2,udp_src=100,udp_dst=200" +AT_DATA([flows.txt], [dnl +table=0,priority=100,ip,nw_src=1.1.1.1,ct_state=-trk,action=ct(commit,nat(src=10.0.0.1-10.0.0.42:1000-1042),table=0) +table=0,priority=100,udp,ct_state=+trk,nw_src=10.0.0.1,nw_dst=1.1.1.2,tp_src=1000,tp_dst=200,action=ct(commit,nat(dst=20.0.0.1-20.0.0.42:2000-2042),table=0) +table=0,priority=100,udp,ct_state=+trk,nw_src=10.0.0.1,nw_dst=20.0.0.1,tp_src=1000,tp_dst=2000,action=3 +table=0,priority=90,ip,ct_state=+trk,action=2 +]) +AT_CHECK([ovs-ofctl del-flows br0]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace br0 "$flow"], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: 3 +]) + +flow="in_port=1,udp6,ipv6_src=1::1,ipv6_dst=1::2,udp_src=100,udp_dst=200" +AT_DATA([flows.txt], [dnl +table=0,priority=100,ip6,ipv6_src=1::1,ct_state=-trk,action=ct(commit,nat(src=[[10::1]]-[[10::42]]:1000-1042),table=0) +table=0,priority=100,udp6,ct_state=+trk,ipv6_src=10::1,ipv6_dst=1::2,tp_src=1000,tp_dst=200,action=ct(commit,nat(dst=[[20::1]]-[[20::42]]:2000-2042),table=0) +table=0,priority=100,udp6,ct_state=+trk,ipv6_src=10::1,ipv6_dst=20::1,tp_src=1000,tp_dst=2000,action=3 +table=0,priority=90,ip6,ct_state=+trk,action=2 +]) +AT_CHECK([ovs-ofctl del-flows br0]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace br0 "$flow"], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: 3 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto - set mtu]) OVS_VSWITCHD_START