Skip to content

Commit

Permalink
netlink: removed incorrect optimization
Browse files Browse the repository at this point in the history
This optimization caused FLOW_TNL_F_UDPIF flag not to be used in
hash calculation for geneve tunnel when revalidating flows which
resulted in different cache hash values and incorrect behaviour.

Added test to prevent regression.

CC: Jesse Gross <jesse@nicira.com>
Fixes: 6728d57 ("dpif-netdev: Translate Geneve options per-flow, not per-packet.")
Reported-at: antrea-io/antrea#897
Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com>
Acked-by: Ansis Atteka <aatteka@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
  • Loading branch information
TomCodeLV authored and igsilya committed Jun 30, 2021
1 parent 3162657 commit c93358a
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/tun-metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ tun_metadata_to_geneve_nlattr(const struct flow_tnl *tun,
} else {
tun_metadata_to_geneve_nlattr_mask(key, tun, flow, b);
}
} else if (flow->metadata.present.len || is_mask) {
} else {
nl_msg_put_unspec(b, OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS,
tun->metadata.opts.gnv,
flow->metadata.present.len);
Expand Down
54 changes: 54 additions & 0 deletions tests/system-traffic.at
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,60 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PI
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP

AT_SETUP([datapath - ping over geneve tunnel, delete flow regression])
OVS_CHECK_GENEVE()

OVS_TRAFFIC_VSWITCHD_START()
ADD_BR([br-underlay])

AT_DATA([flows.txt], [dnl
priority=100,icmp actions=resubmit(,10)
priority=0 actions=NORMAL
table=10, priority=100, ip, actions=ct(table=20,zone=65520)
table=20, priority=200, ip, ct_state=-new+trk, actions=resubmit(,30)
table=20, priority=100, ip, ct_state=+new, actions=resubmit(,30)
table=20, priority=50, ip, actions=DROP
table=30, priority=100, ip, actions=ct(commit,table=40,zone=65520)
table=40, actions=normal
])

AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])

ADD_NAMESPACES(at_ns0)

dnl Set up underlay link from host into the namespace using veth pair.
ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
AT_CHECK([ip link set dev br-underlay up])

dnl Set up tunnel endpoints on OVS outside the namespace and with a native
dnl linux device inside the namespace.
ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.31.1.1], [10.1.1.100/24])
ADD_NATIVE_TUNNEL([geneve], [ns_gnv0], [at_ns0], [172.31.1.100], [10.1.1.1/24],
[vni 0])

dnl First, check the underlay
NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], [0], [dnl
3 packets transmitted, 3 received, 0% packet loss, time 0ms
])

dnl ping over tunnel should work
NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING], [0], [dnl
3 packets transmitted, 3 received, 0% packet loss, time 0ms
])

AT_CHECK([ovs-ofctl del-flows br0 "ct_state=+new"])

dnl ping should not go through after removal of the flow
NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING], [0], [dnl
7 packets transmitted, 0 received, 100% packet loss, time 0ms
])

OVS_TRAFFIC_VSWITCHD_STOP(["/|ERR|/d
/|WARN|/d"])
AT_CLEANUP

AT_SETUP([datapath - flow resume with geneve tun_metadata])
OVS_CHECK_GENEVE()

Expand Down

0 comments on commit c93358a

Please sign in to comment.