Using the action ct(commit,set_field:0x1/0x1->ct_mark), ie, specifying a
mask, would previously overwrite the entire ct_mark field rather than
modifying only the specified bits. Fix the issue.

Fixes: 8e53fe8cf7a1 ("Add connection tracking mark support.")
Signed-off-by: Joe Stringer <[email protected]>
---
 ofproto/ofproto-dpif-xlate.c |  3 ++-
 tests/system-traffic.at      | 34 ++++++++++++++++++++++++++++++++++
 utilities/ovs-ofctl.8.in     |  2 +-
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 19e690ec1ecb..25297851f9c5 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4291,7 +4291,8 @@ put_ct_mark(const struct flow *flow, struct flow 
*base_flow,
     } odp_attr;
 
     odp_attr.key = flow->ct_mark;
-    odp_attr.mask = wc->masks.ct_mark;
+    odp_attr.mask = base_flow->ct_mark ^ flow->ct_mark;
+    wc->masks.ct_mark |= odp_attr.mask;
 
     if (odp_attr.mask && odp_attr.key != base_flow->ct_mark) {
         nl_msg_put_unspec(odp_actions, OVS_CT_ATTR_MARK, &odp_attr,
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 28adbdcb9ee6..9d2c57faa6d7 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -770,6 +770,40 @@ 
tcp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=<cleared>,dport=<cleared>),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - ct_mark 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_mark. Return traffic should
+dnl cause an additional bit to be set in the connection (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_mark)),2
+priority=100,in_port=2,ct_state=-trk,tcp,action=ct(table=0,commit,exec(set_field:0x2/0x2->ct_mark))
+priority=100,in_port=2,ct_state=+trk,ct_mark=1,tcp,action=1
+priority=100,in_port=2,ct_state=+trk,ct_mark=3,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=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),mark=3,protoinfo=(state=TIME_WAIT)
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([conntrack - ct_mark from register])
 CHECK_CONNTRACK()
 OVS_TRAFFIC_VSWITCHD_START()
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 6e2613207979..51a57f777b37 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1725,7 +1725,7 @@ fields are accepted within the \fBexec\fR action, and 
these fields may only be
 modified with this option. For example:
 .
 .RS
-.IP \fBset_field:\fIvalue\fR->ct_mark\fR
+.IP \fBset_field:\fIvalue\fR[\fB/\fImask\fR]->ct_mark\fR
 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
-- 
2.1.4

_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to