From: Mikhail Dmitrichenko <[email protected]>
Software segmentation reuses the original packet as the first segment
and trims it with dp_packet_set_size(). Any L2 padding follows the TCP
payload, so it is cut off by the trim, but 'l2_pad_size' is left as is.
dp_packet_l3_size(), dp_packet_l4_size() and their inner variants
subtract 'l2_pad_size' from the packet size, so for the first segment
they under-report the actual length by the size of the former padding.
As a result, dp_packet_gso_update_segment() writes a wrong IPv4 total
length or IPv6 payload length, and the TCP checksum computed later in
software does not cover the whole segment.
If the padding is longer than the TCP header and 'tso_segsz' together,
these sizes wrap around. The IPv6 payload length becomes bogus and
packet_tcp_complete_csum() reads past the end of the packet buffer:
up to 64 KB for IPv6 and until it faults for IPv4, which crashes
ovs-vswitchd:
SIGSEGV detected, backtrace:
csum_continue at lib/csum.c:46
packet_tcp_complete_csum at lib/packets.c:2120
dp_packet_ol_send_prepare at lib/dp-packet.c:601
netdev_send at lib/netdev.c:845
dp_netdev_pmd_flush_output_on_port at lib/dpif-netdev.c:4402
Such a packet can be received on a port with userspace TSO enabled when
the IP length is shorter than the frame, e.g. from a VM, and be sent to
a port without TSO support.
Drop the L2 padding from the first segment unconditionally. When
partial segmentation leaves the whole packet to HW segmentation, also
remove the padding bytes, as they must not reach the NIC. The result
of dp_packet_gso_partial_nr_segs() depends on the padding, so evaluate
it only once. The other segments are freshly allocated and never carry
padding.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: ef762327f6d3 ("dp-packet-gso: Refactor software segmentation code.")
Suggested-by: David Marchand <[email protected]>
Signed-off-by: Mikhail Dmitrichenko <[email protected]>
---
Notes:
v2:
- Drop the L2 padding unconditionally, also when partial segmentation
leaves the whole packet to HW segmentation (David).
- Evaluate dp_packet_gso_partial_nr_segs() only once, as its result
depends on the L2 padding.
- Test: store the packets in files, with Scapy comments (David).
lib/dp-packet-gso.c | 17 ++++++++--
tests/dpif-netdev.at | 75 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
index 8c8817352..78a1ff592 100644
--- a/lib/dp-packet-gso.c
+++ b/lib/dp-packet-gso.c
@@ -183,7 +183,9 @@ static void
dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch *batch,
bool partial_seg)
{
+ bool sw_last_seg = false;
struct dp_packet *seg;
+ uint16_t l2_pad_size;
unsigned int n_segs;
uint16_t tso_segsz;
size_t data_len;
@@ -216,7 +218,10 @@ dp_packet_gso__(struct dp_packet *p, struct
dp_packet_batch *batch,
}
if (partial_seg) {
- if (dp_packet_gso_partial_nr_segs(p) != 1) {
+ /* Evaluated only once, as the result depends on the L2 padding
+ * which is dropped below. */
+ sw_last_seg = dp_packet_gso_partial_nr_segs(p) != 1;
+ if (sw_last_seg) {
goto last_seg;
}
goto first_seg;
@@ -239,13 +244,21 @@ last_seg:
dp_packet_batch_add(batch, seg);
first_seg:
+ /* The L2 padding, if any, follows the TCP payload. Drop it from the
+ * first segment: either it is cut off when trimming the segment below,
+ * or it must not be passed to HW segmentation. */
+ l2_pad_size = dp_packet_l2_pad_size(p);
+ dp_packet_set_l2_pad_size(p, 0);
+
if (partial_seg) {
- if (dp_packet_gso_partial_nr_segs(p) != 1) {
+ if (sw_last_seg) {
dp_packet_set_size(p, hdr_len + (n_segs - 1) * tso_segsz);
if (n_segs == 2) {
/* No need to ask HW segmentation, we already did the job. */
dp_packet_set_tso_segsz(p, 0);
}
+ } else {
+ dp_packet_set_size(p, dp_packet_size(p) - l2_pad_size);
}
} else {
/* Trim the first segment and reset TSO. */
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 14f238e62..fee931750 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -3333,6 +3333,81 @@ AT_CHECK_UNQUOTED([ovs-pcap p2.pcap], [0], [dnl
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([dpif-netdev - tso + l2 padding])
+AT_KEYWORDS([userspace offload])
+OVS_VSWITCHD_START(
+ [set Open_vSwitch . other_config:userspace-tso-enable=true -- \
+ 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-ofctl add-flow br1 in_port=p1,actions=output:p2])
+
+AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap -- \
+ set Interface p1 options:ol_tso_segsz=500])
+
+dnl IPv4/TCP and IPv6/TCP packets with 1000 bytes of payload.
+AT_DATA([tcp4_frame], m4_join([],
+dnl p = Ether(src='8a:bf:7e:2f:05:84', dst='0a:8f:39:4f:e0:73')
+[0a8f394fe0738abf7e2f05840800],
+dnl p /= IP(src='192.168.123.2', dst='192.168.123.1', tos=1, id=0)
+[45010410000000004006ff92c0a87b02c0a87b01],
+dnl p /= TCP(sport=54392, dport=5201, flags='A', window=0)
+[d47814510000000000000000501000004bce0000],
+dnl p /= Raw(bytes(1000))
+m4_format([%02000d], [0])
+))
+
+AT_DATA([tcp6_frame], m4_join([],
+dnl p = Ether(src='8a:bf:7e:2f:05:84', dst='0a:8f:39:4f:e0:73')
+[0a8f394fe0738abf7e2f058486dd],
+dnl p /= IPv6(src='2001:cafe::88', dst='2001:cafe::92', hlim=0)
+[6000000003fc06002001cafe0000000000000000000000882001cafe000000000000000000000092],
+dnl p /= TCP(sport=54392, dport=5201, flags='A', window=0)
+[d4781451000000000000000050100000ec090000],
+dnl p /= Raw(bytes(1000))
+m4_format([%02000d], [0])
+))
+
+dnl Append L2 padding shorter (10 bytes) and longer (600 bytes) than the
+dnl TCP header and payload of the first segment.
+pad_short=$(printf '%0*d' 20 0)
+pad_long=$(printf '%0*d' 1200 0)
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat tcp4_frame)${pad_short}"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat tcp4_frame)${pad_long}"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat tcp6_frame)${pad_short}"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat tcp6_frame)${pad_long}"])
+
+dnl Padding must be dropped: 2x 500 byte payloads per packet.
+zero500=$(printf '%0*d' 1000 0)
+AT_CHECK_UNQUOTED([ovs-pcap p2.pcap], [0], [dnl
+[0a8f394fe0738abf7e2f058408004501021c0000000040060187c0a87b02c0a87b01]dnl
+[d47814510000000000000000501000004dc20000${zero500}]
+[0a8f394fe0738abf7e2f058408004501021c0001000040060186c0a87b02c0a87b01]dnl
+[d4781451000001f400000000501000004bce0000${zero500}]
+[0a8f394fe0738abf7e2f058408004501021c0000000040060187c0a87b02c0a87b01]dnl
+[d47814510000000000000000501000004dc20000${zero500}]
+[0a8f394fe0738abf7e2f058408004501021c0001000040060186c0a87b02c0a87b01]dnl
+[d4781451000001f400000000501000004bce0000${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000000000000000050100000edfd0000]dnl
+[${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000001f40000000050100000ec090000]dnl
+[${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000000000000000050100000edfd0000]dnl
+[${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000001f40000000050100000ec090000]dnl
+[${zero500}]
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([dpif-netdev - tunnel tso fallback])
AT_KEYWORDS([userspace offload])
OVS_VSWITCHD_START([set Open_vSwitch . other_config:userspace-tso-enable=true \
--
2.25.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
