From patchwork Fri Dec 23 02:36:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniele Di Proietto X-Patchwork-Id: 708372 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tlCG3218Kz9t0H for ; Fri, 23 Dec 2016 13:36:59 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 4AC6CB77; Fri, 23 Dec 2016 02:36:25 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 620A5B59 for ; Fri, 23 Dec 2016 02:36:24 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from EX13-EDG-OU-002.vmware.com (ex13-edg-ou-002.vmware.com [208.91.0.190]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id B9A471D0 for ; Fri, 23 Dec 2016 02:36:23 +0000 (UTC) Received: from sc9-mailhost2.vmware.com (10.113.161.72) by EX13-EDG-OU-002.vmware.com (10.113.208.156) with Microsoft SMTP Server id 15.0.1156.6; Thu, 22 Dec 2016 18:35:21 -0800 Received: from sc9-mailhost3.vmware.com (htb-1n-eng-dhcp161.eng.vmware.com [10.33.74.161]) by sc9-mailhost2.vmware.com (Postfix) with ESMTP id 6CFADB03B1; Thu, 22 Dec 2016 18:36:22 -0800 (PST) From: Daniele Di Proietto To: Date: Thu, 22 Dec 2016 18:36:17 -0800 Message-ID: <20161223023619.130243-1-diproiettod@vmware.com> X-Mailer: git-send-email 2.10.2 MIME-Version: 1.0 Received-SPF: None (EX13-EDG-OU-002.vmware.com: diproiettod@vmware.com does not designate permitted sender hosts) X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Daniele Di Proietto Subject: [ovs-dev] [PATCH v2 1/3] conntrack: Do not create new connections from ICMP errors. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org ICMP error packets (e.g. destination unreachable messages) are considered 'related' to another connection and are treated as part of that. However: * We shouldn't create new entries in the connection table if the original connection is not found. This is consistent with what the kernel does. * We certainly shouldn't call valid_new() on the packet, because valid_new() assumes the packet l4 type (might be TCP, UDP or ICMP) to be consistent with the conn_key nw_proto type. Found by inspection. Fixes: a489b16854b5("conntrack: New userspace connection tracker.") Signed-off-by: Daniele Di Proietto Acked-by: Darrell Ball --- v2: Handle ICMP error for non existing connection in else branch without restructuring the whole code flow. --- lib/conntrack.c | 6 +++++- tests/system-traffic.at | 27 ++++++++++++++++----------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 7c50a28..9bea3d9 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -247,7 +247,11 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, } } } else { - conn = conn_not_found(ct, pkt, ctx, &state, commit, now); + if (ctx->related) { + state |= CS_INVALID; + } else { + conn = conn_not_found(ct, pkt, ctx, &state, commit, now); + } } write_ct_md(pkt, state, zone, conn ? conn->mark : 0, diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 9ea6d6b..a5023d3 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -1331,12 +1331,8 @@ ADD_VETH(p1, at_ns1, br0, "172.16.0.2/24") dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from ns1->ns0. AT_DATA([flows.txt], [dnl -priority=1,action=drop -priority=10,arp,action=normal -priority=100,in_port=1,udp,ct_state=-trk,action=ct(commit,table=0) -priority=100,in_port=1,ip,ct_state=+trk,actions=controller -priority=100,in_port=2,ip,ct_state=-trk,action=ct(table=0) -priority=100,in_port=2,ip,ct_state=+trk+rel+rpl,action=controller +table=0,ip,action=ct(commit,table=1) +table=1,ip,action=controller ]) AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows.txt]) @@ -1345,22 +1341,31 @@ AT_CAPTURE_FILE([ofctl_monitor.log]) AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir --pidfile 2> ofctl_monitor.log]) dnl 1. Send an ICMP port unreach reply for port 8738, without any previous request -AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 ct\(table=0\) 'f64c473528c9c6f54ecb72db080045c0003d2e8700004001f355ac100004ac1000030303553f0000000045000021317040004011b138ac100003ac10000411112222000d20966369616f0a']) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 resubmit\(,0\) 'f64c473528c9c6f54ecb72db080045c0003d2e8700004001f351ac100004ac1000030303da490000000045000021317040004011b138ac100003ac10000411112222000d20966369616f0a']) dnl 2. Send and UDP packet to port 5555 -AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 1 ct\(commit,table=0\) 'c6f94ecb72dbe64c473528c9080045000021317040004011b138ac100001ac100002a28e15b3000d20966369616f0a']) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 1 resubmit\(,0\) 'c6f94ecb72dbe64c473528c9080045000021317040004011b138ac100001ac100002a28e15b3000d20966369616f0a']) dnl 3. Send an ICMP port unreach reply for port 5555, related to the first packet -AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 ct\(table=0\) 'e64c473528c9c6f94ecb72db080045c0003d2e8700004001f355ac100002ac1000010303553f0000000045000021317040004011b138ac100001ac100002a28e15b3000d20966369616f0a']) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 resubmit\(,0\) 'e64c473528c9c6f94ecb72db080045c0003d2e8700004001f355ac100002ac1000010303553f0000000045000021317040004011b138ac100001ac100002a28e15b3000d20966369616f0a']) dnl Check this output. We only see the latter two packets, not the first. AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=47 ct_state=new|trk,in_port=1 (via action) data_len=47 (unbuffered) +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 ct_state=inv|trk,in_port=2 (via action) data_len=75 (unbuffered) +icmp,vlan_tci=0x0000,dl_src=c6:f5:4e:cb:72:db,dl_dst=f6:4c:47:35:28:c9,nw_src=172.16.0.4,nw_dst=172.16.0.3,nw_tos=192,nw_ecn=0,nw_ttl=64,icmp_type=3,icmp_code=3 icmp_csum:da49 +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=47 ct_state=new|trk,in_port=1 (via action) data_len=47 (unbuffered) udp,vlan_tci=0x0000,dl_src=e6:4c:47:35:28:c9,dl_dst=c6:f9:4e:cb:72:db,nw_src=172.16.0.1,nw_dst=172.16.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=41614,tp_dst=5555 udp_csum:2096 -NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=75 ct_state=rel|rpl|trk,in_port=2 (via action) data_len=75 (unbuffered) +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 ct_state=rel|rpl|trk,in_port=2 (via action) data_len=75 (unbuffered) icmp,vlan_tci=0x0000,dl_src=c6:f9:4e:cb:72:db,dl_dst=e6:4c:47:35:28:c9,nw_src=172.16.0.2,nw_dst=172.16.0.1,nw_tos=192,nw_ecn=0,nw_ttl=64,icmp_type=3,icmp_code=3 icmp_csum:553f ]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.0.1)], [0], [dnl +udp,orig=(src=172.16.0.1,dst=172.16.0.2,sport=,dport=),reply=(src=172.16.0.2,dst=172.16.0.1,sport=,dport=) +]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.0.3)], [0], [dnl +]) + OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP