Packets with L4 partial status for a simple match flow would not get L4
checksums offloads applied.
This was not caught in unit tests, because packets from netdev-dummy
(calling miniflow_extract) would get Tx flags set early, before
parse_tcp_flags() got called during packet processing.
Signed-off-by: David Marchand <david.march...@redhat.com>
---
lib/flow.c | 33 ++++++++++++++++---
lib/netdev-dummy.c | 3 +-
lib/netdev-linux.c | 58 ---------------------------------
tests/dpif-netdev.at | 76 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 106 insertions(+), 64 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 61e732aff8..c2e1e4ad6f 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1271,6 +1271,10 @@ parse_tcp_flags(struct dp_packet *packet,
size = tot_len; /* Never pull padding. */
data_pull(&data, &size, ip_len);
+ dp_packet_hwol_set_tx_ipv4(packet);
+ if (dp_packet_ip_checksum_good(packet)) {
+ dp_packet_hwol_set_tx_ip_csum(packet);
+ }
} else if (dl_type == htons(ETH_TYPE_IPV6)) {
const struct ovs_16aligned_ip6_hdr *nh = data;
uint16_t plen;
@@ -1283,6 +1287,7 @@ parse_tcp_flags(struct dp_packet *packet,
}
data_pull(&data, &size, sizeof *nh);
+ dp_packet_hwol_set_tx_ipv6(packet);
plen = ntohs(nh->ip6_plen); /* Never pull padding. */
dp_packet_set_l2_pad_size(packet, size - plen);
size = plen;
@@ -1300,11 +1305,29 @@ parse_tcp_flags(struct dp_packet *packet,
}
packet->l4_ofs = (uint16_t)((char *)data - frame);
- if (!(nw_frag & FLOW_NW_FRAG_LATER) && nw_proto == IPPROTO_TCP &&
- size >= TCP_HEADER_LEN) {
- const struct tcp_header *tcp = data;
-
- return TCP_FLAGS(tcp->tcp_ctl);
+ if (!(nw_frag & FLOW_NW_FRAG_LATER)) {
+ if (nw_proto == IPPROTO_TCP && size >= TCP_HEADER_LEN) {
+ const struct tcp_header *tcp = data;
+
+ dp_packet_ol_l4_csum_check_partial(packet);
+ if (dp_packet_l4_checksum_good(packet)
+ || dp_packet_ol_l4_csum_partial(packet)) {
+ dp_packet_hwol_set_csum_tcp(packet);
+ }
+ return TCP_FLAGS(tcp->tcp_ctl);
+ } else if (nw_proto == IPPROTO_UDP && size >= UDP_HEADER_LEN) {
+ dp_packet_ol_l4_csum_check_partial(packet);
+ if (dp_packet_l4_checksum_good(packet)
+ || dp_packet_ol_l4_csum_partial(packet)) {
+ dp_packet_hwol_set_csum_udp(packet);
+ }
+ } else if (nw_proto == IPPROTO_SCTP && size >= SCTP_HEADER_LEN) {
+ dp_packet_ol_l4_csum_check_partial(packet);
+ if (dp_packet_l4_checksum_good(packet)
+ || dp_packet_ol_l4_csum_partial(packet)) {
+ dp_packet_hwol_set_csum_sctp(packet);
+ }
+ }
}
return 0;
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index a54083ccd2..7d5abb87e3 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1194,6 +1194,8 @@ netdev_dummy_rxq_recv(struct netdev_rxq *rxq_, struct
dp_packet_batch *batch,
netdev->custom_stats[0].value++;
netdev->custom_stats[1].value++;
+ *dp_packet_ol_flags_ptr(packet) &= ~DP_PACKET_OL_TX_ANY_CKSUM;
+
if (netdev->ol_ip_rx_csum_set_good) {
dp_packet_ol_set_ip_csum_good(packet);
} else if (netdev->ol_ip_rx_csum_set_bad) {
@@ -1215,7 +1217,6 @@ netdev_dummy_rxq_recv(struct netdev_rxq *rxq_, struct
dp_packet_batch *batch,
if (userspace_tso_enabled() && netdev->ol_tso_segsz) {
dp_packet_set_tso_segsz(packet, netdev->ol_tso_segsz);
dp_packet_hwol_set_tcp_seg(packet);
- dp_packet_hwol_set_csum_tcp(packet);
}
if (VLOG_IS_DBG_ENABLED()) {
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 19bf62ecee..8a463c2efd 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -7038,55 +7038,6 @@ af_packet_sock(void)
return sock;
}
-static int
-netdev_linux_parse_l2(struct dp_packet *b, uint16_t *l4proto)
-{
- struct eth_header *eth_hdr;
- ovs_be16 eth_type;
- int l2_len;
-
- eth_hdr = dp_packet_at(b, 0, ETH_HEADER_LEN);
- if (!eth_hdr) {
- return -EINVAL;
- }
-
- l2_len = ETH_HEADER_LEN;
- eth_type = eth_hdr->eth_type;
- if (eth_type_vlan(eth_type)) {
- struct vlan_header *vlan = dp_packet_at(b, l2_len, VLAN_HEADER_LEN);
-
- if (!vlan) {
- return -EINVAL;
- }
-
- eth_type = vlan->vlan_next_type;
- l2_len += VLAN_HEADER_LEN;
- }
-
- if (eth_type == htons(ETH_TYPE_IP)) {
- struct ip_header *ip_hdr = dp_packet_at(b, l2_len, IP_HEADER_LEN);
-
- if (!ip_hdr) {
- return -EINVAL;
- }
-
- *l4proto = ip_hdr->ip_proto;
- dp_packet_hwol_set_tx_ipv4(b);
- } else if (eth_type == htons(ETH_TYPE_IPV6)) {
- struct ovs_16aligned_ip6_hdr *nh6;
-
- nh6 = dp_packet_at(b, l2_len, IPV6_HEADER_LEN);
- if (!nh6) {
- return -EINVAL;
- }
-
- *l4proto = nh6->ip6_ctlun.ip6_un1.ip6_un1_nxt;
- dp_packet_hwol_set_tx_ipv6(b);
- }
-
- return 0;
-}
-
/* Initializes packet 'b' with features enabled in the prepended
* struct virtio_net_hdr. Returns 0 if successful, otherwise a
* positive errno value. */
@@ -7104,15 +7055,6 @@ netdev_linux_parse_vnet_hdr(struct dp_packet *b)
}
if (vnet->flags == VIRTIO_NET_HDR_F_NEEDS_CSUM) {
- uint16_t l4proto = 0;
-
- if (netdev_linux_parse_l2(b, &l4proto)) {
- return EINVAL;
- }
-
- if (l4proto == IPPROTO_UDP) {
- dp_packet_hwol_set_csum_udp(b);
- }
/* The packet has offloaded checksum. However, there is no
* additional information like the protocol used, so it would
* require to parse the packet here. The checksum starting point
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 4060e356ea..5f57537388 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -1087,6 +1087,82 @@ AT_CHECK([ovs-vsctl set Interface p1
options:ol_l4_rx_csum_set_good=false])
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([userspace offload - tcp csum offload (simple match)])
+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])
+
+AT_CHECK([ovs-ofctl add-flow br1 in_port=p1,actions=output: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}")
+bad_frame=$(echo $good_frame | sed -e "s/6b72/dead/")
+
+AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap])
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${bad_frame}
+])
+dnl First packet, no simple matching.
+AT_CHECK([ovs-appctl dpif-netdev/pmd-stats-show | grep 'simple match hits'],
[0], [dnl
+ simple match hits: 0
+])
+
+dnl No Rx flag.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${bad_frame}
+])
+AT_CHECK([ovs-appctl dpif-netdev/pmd-stats-show | grep 'simple match hits'],
[0], [dnl
+ simple match hits: 1
+])
+
+dnl Flag as Rx good.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_l4_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])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${bad_frame}
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_l4_rx_csum_set_good=false])
+AT_CHECK([ovs-appctl dpif-netdev/pmd-stats-show | grep 'simple match hits'],
[0], [dnl
+ simple match hits: 2
+])
+
+dnl Flag as Rx bad.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_l4_rx_csum_set_bad=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${bad_frame}
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_l4_rx_csum_set_bad=false])
+AT_CHECK([ovs-appctl dpif-netdev/pmd-stats-show | grep 'simple match hits'],
[0], [dnl
+ simple match hits: 3
+])
+
+dnl Flag as Rx partial.
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_l4_rx_csum_set_partial=true])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
+AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_frame}
+])
+AT_CHECK([ovs-vsctl set Interface p1 options:ol_l4_rx_csum_set_partial=false])
+AT_CHECK([ovs-appctl dpif-netdev/pmd-stats-show | grep 'simple match hits'],
[0], [dnl
+ simple match hits: 4
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([userspace offload - udp csum offload])
OVS_VSWITCHD_START(
[add-br br1 -- set bridge br1 datapath-type=dummy -- \
--
2.48.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
