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 <j...@ovn.org> --- lib/util.h | 22 ++++++++++++++++++++++ ofproto/ofproto-dpif-xlate.c | 4 +++- tests/system-traffic.at | 34 ++++++++++++++++++++++++++++++++++ utilities/ovs-ofctl.8.in | 2 +- 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/lib/util.h b/lib/util.h index 389c093a2fd7..5678671fbcf5 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_or(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 25297851f9c5..b59a5407bb9f 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -4315,7 +4315,9 @@ put_ct_label(const struct flow *flow, struct flow *base_flow, 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); + wc->masks.ct_label = ovs_u128_or(wc->masks.ct_label, + odp_ct_label->mask); } } 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=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),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 -- 2.1.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev