Bad packets were still being validated in software when entering
conntrack. Trust decision taken wrt IP checksum offloading (checking
dp_packet_hwol_l3_csum_ipv4_ol()) and avoid revalidating a known
bad checksum.
While at it, add coverage counters so that checksum validation impact
can be monitored, and unit tests.
Signed-off-by: David Marchand <david.march...@redhat.com>
---
lib/conntrack.c | 37 +++++++----
tests/dpif-netdev.at | 154 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 177 insertions(+), 14 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f4b150bee5..08823234d4 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -47,7 +47,9 @@
VLOG_DEFINE_THIS_MODULE(conntrack);
COVERAGE_DEFINE(conntrack_full);
+COVERAGE_DEFINE(conntrack_l3csum_checked);
COVERAGE_DEFINE(conntrack_l3csum_err);
+COVERAGE_DEFINE(conntrack_l4csum_checked);
COVERAGE_DEFINE(conntrack_l4csum_err);
COVERAGE_DEFINE(conntrack_lookup_natted_miss);
COVERAGE_DEFINE(conntrack_zone_full);
@@ -1692,9 +1694,12 @@ extract_l3_ipv4(struct conn_key *key, const void *data,
size_t size,
return false;
}
- if (validate_checksum && csum(data, ip_len) != 0) {
- COVERAGE_INC(conntrack_l3csum_err);
- return false;
+ if (validate_checksum) {
+ COVERAGE_INC(conntrack_l3csum_checked);
+ if (csum(data, ip_len)) {
+ COVERAGE_INC(conntrack_l3csum_err);
+ return false;
+ }
}
if (new_data) {
@@ -1761,6 +1766,7 @@ checksum_valid(const struct conn_key *key, const void
*data, size_t size,
valid = false;
}
+ COVERAGE_INC(conntrack_l4csum_checked);
if (!valid) {
COVERAGE_INC(conntrack_l4csum_err);
}
@@ -1773,19 +1779,19 @@ sctp_checksum_valid(const void *data, size_t size)
{
struct sctp_header *sctp = (struct sctp_header *) data;
ovs_be32 rcvd_csum, csum;
- bool ret;
rcvd_csum = get_16aligned_be32(&sctp->sctp_csum);
put_16aligned_be32(&sctp->sctp_csum, 0);
csum = crc32c(data, size);
put_16aligned_be32(&sctp->sctp_csum, rcvd_csum);
- ret = (rcvd_csum == csum);
- if (!ret) {
+ COVERAGE_INC(conntrack_l4csum_checked);
+ if (rcvd_csum != csum) {
COVERAGE_INC(conntrack_l4csum_err);
+ return false;
}
- return ret;
+ return true;
}
static inline bool
@@ -1868,12 +1874,15 @@ check_l4_sctp(const void *data, size_t size, bool
validate_checksum)
static inline bool
check_l4_icmp(const void *data, size_t size, bool validate_checksum)
{
- if (validate_checksum && csum(data, size) != 0) {
- COVERAGE_INC(conntrack_l4csum_err);
- return false;
- } else {
- return true;
+ if (validate_checksum) {
+ COVERAGE_INC(conntrack_l4csum_checked);
+ if (csum(data, size)) {
+ COVERAGE_INC(conntrack_l4csum_err);
+ return false;
+ }
}
+
+ return true;
}
static inline bool
@@ -2222,8 +2231,8 @@ conn_key_extract(struct conntrack *ct, struct dp_packet
*pkt, ovs_be16 dl_type,
/* Validate the checksum only when hwol is not supported and the
* packet's checksum status is not known. */
ok = extract_l3_ipv4(&ctx->key, l3, dp_packet_l3_size(pkt), NULL,
- !dp_packet_hwol_is_ipv4(pkt) &&
- !dp_packet_ip_checksum_good(pkt));
+ !dp_packet_hwol_l3_csum_ipv4_ol(pkt)
+ && !dp_packet_ip_checksum_good(pkt));
}
} else if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
ok = extract_l3_ipv6(&ctx->key, l3, dp_packet_l3_size(pkt), NULL);
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 58977b8fa1..36acd7c0bc 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -1609,6 +1609,160 @@ AT_CHECK([ovs-vsctl set Interface p1
options:ol_l4_rx_csum_set_good=false])
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([userspace offload - conntrack])
+OVS_VSWITCHD_START(
+ [add-br br1 -- set bridge br1 datapath-type=dummy -- \
+ add-port br1 p1 -- \
+ set Interface p1 type=dummy -- \
+ add-port br1 p2 -- \
+ set Interface p2 type=dummy --])
+
+AT_CHECK([ovs-appctl vlog/set netdev_dummy:file:dbg])
+
+dnl Modify the ip_dst addr to force changing the IP csum.
+AT_CHECK([ovs-ofctl add-flow br1
'in_port=p1,ip,ct_state=-trk,actions=ct(commit,nat(dst=192.168.1.1),table=0),p2'])
+
+flow_s="\
+ eth_src=8a:bf:7e:2f:05:84,eth_dst=0a:8f:39:4f:e0:73,\
+ tcp,ip_src=192.168.123.2,ip_dst=192.168.123.1,ip_frag=no,\
+ tcp_src=54392,tcp_dst=5201,tcp_flags=ack"
+
+good_frame=$(ovs-ofctl compose-packet --bare "${flow_s}")
+
+AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap])
+
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [0
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [0
+])
+
+dnl Checks for good packet (Tx offloads disabled).
+flow_expected=$(echo "${flow_s}" | sed 's/192.168.123.1/192.168.1.1/g')
+good_expected=$(ovs-ofctl compose-packet --bare "${flow_expected}")
+
+dnl No Rx flag.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [1
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [0
+])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
+])
+
+dnl Flag as Rx good.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=false])
+
+dnl Flag as Rx bad.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [1
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [1
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=false])
+
+dnl Checks for bad packet (Tx offloads disabled).
+bad_frame=$(ovs-ofctl compose-packet --bare --bad-csum "${flow_s}")
+
+dnl No Rx flag.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [2
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [2
+])
+
+dnl Flag as Rx good.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+dnl In this case, datapath will fix the csum as it trusts the Rx status.
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=false])
+
+dnl Flag as Rx bad.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [2
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [3
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=false])
+
+dnl Checks for good packet (Tx offloads enabled).
+AT_CHECK([ovs-vsctl set Interface p2 options:ol_ip_tx_csum=true])
+
+dnl No Rx flag.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [3
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [3
+])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
+])
+
+dnl Flag as Rx good.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [3
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [3
+])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=false])
+
+dnl Flag as Rx bad.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [3
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [4
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=false])
+
+dnl Checks for bad packet (Tx offloads enabled).
+
+dnl No Rx flag.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [4
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [5
+])
+
+dnl Flag as Rx good.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [4
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [5
+])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+dnl In this case, datapath will fix the csum as it trusts the Rx status.
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=false])
+
+dnl Flag as Rx bad.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_checked], [0], [4
+])
+AT_CHECK([ovs-appctl coverage/read-counter conntrack_l3csum_err], [0], [6
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_bad=false])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([userspace offload - tso])
OVS_VSWITCHD_START(
[set Open_vSwitch . other_config:userspace-tso-enable=true -- \
--
2.48.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
