From patchwork Wed Mar 30 11:21:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Stringer X-Patchwork-Id: 603320 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3qZlbM4x49z9syq for ; Wed, 30 Mar 2016 22:21:51 +1100 (AEDT) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 59FF710601; Wed, 30 Mar 2016 04:21:45 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id 22820105DD for ; Wed, 30 Mar 2016 04:21:43 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id 9FC791624E6 for ; Wed, 30 Mar 2016 05:21:42 -0600 (MDT) X-ASG-Debug-ID: 1459336901-0b323769960e020001-byXFYA Received: from mx3-pf3.cudamail.com ([192.168.14.3]) by bar6.cudamail.com with ESMTP id QMdQtBLud12jejqM (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 30 Mar 2016 05:21:41 -0600 (MDT) X-Barracuda-Envelope-From: joe@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.3 Received: from unknown (HELO relay6-d.mail.gandi.net) (217.70.183.198) by mx3-pf3.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 30 Mar 2016 11:21:41 -0000 Received-SPF: pass (mx3-pf3.cudamail.com: SPF record at ovn.org designates 217.70.183.198 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.198 X-Barracuda-RBL-IP: 217.70.183.198 Received: from mfilter35-d.gandi.net (mfilter35-d.gandi.net [217.70.178.166]) by relay6-d.mail.gandi.net (Postfix) with ESMTP id C00B7FB8C7; Wed, 30 Mar 2016 13:21:39 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter35-d.gandi.net Received: from relay6-d.mail.gandi.net ([IPv6:::ffff:217.70.183.198]) by mfilter35-d.gandi.net (mfilter35-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id O52gNVqWqEym; Wed, 30 Mar 2016 13:21:37 +0200 (CEST) X-Originating-IP: 121.98.70.189 Received: from localhost.localdomain (unknown [121.98.70.189]) (Authenticated sender: joe@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 92301FB8B8; Wed, 30 Mar 2016 13:21:35 +0200 (CEST) X-CudaMail-Envelope-Sender: joe@ovn.org From: Joe Stringer To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V3-329006008 X-CudaMail-DTE: 033016 X-CudaMail-Originating-IP: 217.70.183.198 Date: Thu, 31 Mar 2016 00:21:14 +1300 X-ASG-Orig-Subj: [##CM-V3-329006008##][PATCHv2 2/2] ofproto-dpif-xlate: Fix bitwise ops on ct_labels. Message-Id: <1459336874-2956-2-git-send-email-joe@ovn.org> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1459336874-2956-1-git-send-email-joe@ovn.org> References: <1459336874-2956-1-git-send-email-joe@ovn.org> X-Barracuda-Connect: UNKNOWN[192.168.14.3] X-Barracuda-Start-Time: 1459336901 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCHv2 2/2] ofproto-dpif-xlate: Fix bitwise ops on ct_labels. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" Using the action ct(commit,set_field:0x1/0x1->ct_label), ie, specifying a mask, would previously overwrite the entire ct_labels field rather than modifying only the specified bits. Fix the issue. Fixes: 9daf23484fb1 ("Add connection tracking label support.") Signed-off-by: Joe Stringer Acked-by: Ben Pfaff --- v2: Remove wildcards from put_ct_label(). --- lib/util.h | 22 ++++++++++++++++++++++ ofproto/ofproto-dpif-xlate.c | 11 +++++------ tests/system-traffic.at | 34 ++++++++++++++++++++++++++++++++++ utilities/ovs-ofctl.8.in | 2 +- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/lib/util.h b/lib/util.h index 389c093a2fd7..ece0b552860f 100644 --- a/lib/util.h +++ b/lib/util.h @@ -611,6 +611,28 @@ ovs_be128_is_zero(const ovs_be128 *val) return !(val->be64.hi || val->be64.lo); } +static inline ovs_u128 +ovs_u128_xor(const ovs_u128 a, const ovs_u128 b) +{ + ovs_u128 dst; + + dst.u64.hi = a.u64.hi ^ b.u64.hi; + dst.u64.lo = a.u64.lo ^ b.u64.lo; + + return dst; +} + +static inline ovs_u128 +ovs_u128_and(const ovs_u128 a, const ovs_u128 b) +{ + ovs_u128 dst; + + dst.u64.hi = a.u64.hi & b.u64.hi; + dst.u64.lo = a.u64.lo & b.u64.lo; + + return dst; +} + void xsleep(unsigned int seconds); bool is_stdout_a_tty(void); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 0c226201a580..f26e02940dee 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -4301,10 +4301,9 @@ put_ct_mark(const struct flow *flow, struct flow *base_flow, static void put_ct_label(const struct flow *flow, struct flow *base_flow, - struct ofpbuf *odp_actions, struct flow_wildcards *wc) + struct ofpbuf *odp_actions) { - if (!ovs_u128_is_zero(&wc->masks.ct_label) - && !ovs_u128_equals(&flow->ct_label, &base_flow->ct_label)) { + if (!ovs_u128_equals(&flow->ct_label, &base_flow->ct_label)) { struct { ovs_u128 key; ovs_u128 mask; @@ -4313,8 +4312,8 @@ put_ct_label(const struct flow *flow, struct flow *base_flow, odp_ct_label = nl_msg_put_unspec_uninit(odp_actions, OVS_CT_ATTR_LABELS, sizeof(*odp_ct_label)); - odp_ct_label->key = flow->ct_label; - odp_ct_label->mask = wc->masks.ct_label; + odp_ct_label->mask = ovs_u128_xor(base_flow->ct_label, flow->ct_label); + odp_ct_label->key = ovs_u128_and(flow->ct_label, odp_ct_label->mask); } } @@ -4414,7 +4413,7 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc) } nl_msg_put_u16(ctx->odp_actions, OVS_CT_ATTR_ZONE, zone); put_ct_mark(&ctx->xin->flow, &ctx->base_flow, ctx->odp_actions); - put_ct_label(&ctx->xin->flow, &ctx->base_flow, ctx->odp_actions, ctx->wc); + put_ct_label(&ctx->xin->flow, &ctx->base_flow, ctx->odp_actions); put_ct_helper(ctx->odp_actions, ofc); put_ct_nat(ctx); ctx->ct_nat_action = NULL; diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 9d2c57faa6d7..9c0068410d85 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -889,6 +889,40 @@ NS_CHECK_EXEC([at_ns2], [wget 10.1.1.4 -t 3 -T 1 -v -o wget1.log], [4]) OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - ct_label bit-fiddling]) +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START() + +ADD_NAMESPACES(at_ns0, at_ns1, at_ns2, at_ns3) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +dnl Allow traffic between ns0<->ns1 using the ct_labels. Return traffic should +dnl cause an additional bit to be set in the connection labels (and be allowed) +AT_DATA([flows.txt], [dnl +priority=1,action=drop +priority=10,arp,action=normal +priority=10,icmp,action=normal +priority=100,in_port=1,tcp,action=ct(commit,exec(set_field:0x1/0x1->ct_label)),2 +priority=100,in_port=2,ct_state=-trk,tcp,action=ct(table=0,commit,exec(set_field:0x200000000/0x200000000->ct_label)) +priority=100,in_port=2,ct_state=+trk,ct_label=0x1,tcp,action=1 +priority=100,in_port=2,ct_state=+trk,ct_label=0x200000001,tcp,action=1 +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +dnl HTTP requests from p0->p1 should work fine. +NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py]], [http0.pid]) +NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o wget0.log]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | grep TIME], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),labels=0x200000001,protoinfo=(state=TIME_WAIT) +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([conntrack - ICMP related]) CHECK_CONNTRACK() OVS_TRAFFIC_VSWITCHD_START() diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 51a57f777b37..8188b5352349 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1730,7 +1730,7 @@ Store a 32-bit metadata value with the connection. If the connection is committed, then subsequent lookups for packets in this connection will populate the \fBct_mark\fR flow field when the packet is sent to the connection tracker with the \fBtable\fR specified. -.IP \fBset_field:\fIvalue\fR->ct_label\fR +.IP \fBset_field:\fIvalue\fR[\fB/\fImask\fR]->ct_label\fR Store a 128-bit metadata value with the connection. If the connection is committed, then subsequent lookups for packets in this connection will populate the \fBct_label\fR flow field when the packet is sent to the