Skip to content

Commit 88e3ae5

Browse files
fnordahligsilya
authored andcommitted
ofproto-dpif-xlate: Fix internal CT state for non-recirc traffic.
In some circumstances a flow may get its ct_state set without conscious intervention by the OVS user space code. Commit 355fef6 optimizes out unnecessary ct_clear actions based on an internal struct xlate_ctx->conntracked state flag. Before this commit the xlate_ctx->conntracked state flag would be initialized to 'false' and only set during thawing for recirculation. This patch checks the flow ct_state for the non-recirc case and sets the internal conntracked state appropriately. A system traffic test is also added to avoid regression. Fixes: 355fef6 ("ofproto-dpif-xlate: Avoid successive ct_clear datapath actions.") Signed-off-by: Frode Nordahl <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
1 parent ca44218 commit 88e3ae5

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

ofproto/ofproto-dpif-xlate.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7828,6 +7828,12 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
78287828
goto exit;
78297829
}
78307830

7831+
if (!xin->frozen_state
7832+
&& xin->flow.ct_state
7833+
&& xin->flow.ct_state & CS_TRACKED) {
7834+
ctx.conntracked = true;
7835+
}
7836+
78317837
/* Tunnel metadata in udpif format must be normalized before translation. */
78327838
if (flow->tunnel.flags & FLOW_TNL_F_UDPIF) {
78337839
const struct tun_table *tun_tab = ofproto_get_tun_tab(

tests/system-traffic.at

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6807,6 +6807,53 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE
68076807
OVS_TRAFFIC_VSWITCHD_STOP
68086808
AT_CLEANUP
68096809

6810+
AT_SETUP([conntrack - can match and clear ct_state from outside OVS])
6811+
CHECK_CONNTRACK_LOCAL_STACK()
6812+
OVS_CHECK_TUNNEL_TSO()
6813+
OVS_CHECK_GENEVE()
6814+
6815+
OVS_TRAFFIC_VSWITCHD_START()
6816+
ADD_BR([br-underlay], [set bridge br-underlay other-config:hwaddr=\"f0:00:00:01:01:02\"])
6817+
6818+
AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
6819+
AT_CHECK([ovs-ofctl add-flow br-underlay "priority=100,ct_state=+trk,actions=ct_clear,resubmit(,0)"])
6820+
AT_CHECK([ovs-ofctl add-flow br-underlay "priority=10,actions=normal"])
6821+
6822+
ADD_NAMESPACES(at_ns0)
6823+
6824+
dnl Set up underlay link from host into the namespace using veth pair.
6825+
ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", "f0:00:00:01:01:01")
6826+
AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
6827+
AT_CHECK([ip link set dev br-underlay up])
6828+
6829+
dnl Set up tunnel endpoints on OVS outside the namespace and with a native
6830+
dnl linux device inside the namespace.
6831+
ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.31.1.1], [10.1.1.100/24])
6832+
ADD_NATIVE_TUNNEL([geneve], [ns_gnv0], [at_ns0], [172.31.1.100], [10.1.1.1/24],
6833+
[vni 0])
6834+
6835+
dnl First, check the underlay
6836+
NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], [0], [dnl
6837+
3 packets transmitted, 3 received, 0% packet loss, time 0ms
6838+
])
6839+
6840+
dnl Okay, now check the overlay
6841+
NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING], [0], [dnl
6842+
3 packets transmitted, 3 received, 0% packet loss, time 0ms
6843+
])
6844+
6845+
dnl Confirm that the ct_state and ct_clear action found its way to the dp
6846+
AT_CHECK([ovs-appctl dpctl/dump-flows --names | grep ct_clear | sort | dnl
6847+
grep 'eth(src=f0:00:00:01:01:02,dst=f0:00:00:01:01:01)' | dnl
6848+
strip_stats | strip_used | dnl
6849+
sed 's/,packet_type(ns=[[0-9]]*,id=[[0-9]]*),/,/'],
6850+
[0], [dnl
6851+
recirc_id(0),in_port(br-underlay),ct_state(+trk),eth(src=f0:00:00:01:01:02,dst=f0:00:00:01:01:01),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:0.0s, actions:ct_clear,ovs-p0
6852+
])
6853+
6854+
OVS_TRAFFIC_VSWITCHD_STOP
6855+
AT_CLEANUP
6856+
68106857
AT_BANNER([IGMP])
68116858

68126859
AT_SETUP([IGMP - flood under normal action])

0 commit comments

Comments
 (0)