[PATCH v2] tap: Trim Ethernet padding from short IPv4 frames instead of dropping them

[PATCH v2] tap: Trim Ethernet padding from short IPv4 frames instead of dropping them

[David du Colombier]

tap4_handler() requires the L2 payload after the IP header to match
the IP datagram length exactly. Guests whose drivers pad transmitted
frames to the 60 byte Ethernet minimum, as real hardware requires and
as drivers modelled on hardware do (Plan 9's virtio-net, for one),
send pure ACK and FIN segments as 60 byte frames: 14 byte Ethernet
header, 40 byte IPv4 datagram, 6 padding octets. Those frames fail
the exact length check and are dropped without trace.

passt then never sees such a guest's acknowledgements: it
retransmits from the lowest unacknowledged sequence with exponential
backoff while the guest, which received and acknowledged everything,
waits. Every fresh connection stalls for minutes (a 1 MiB HTTP fetch
over --map-host-loopback measured 248 s before this change, 0.27 s
after; bulk transfer over established connections, whose ACKs ride
data segments above the padding threshold, is unaffected). FIN
segments are padded too, so teardown hangs as well. Note that
tap_send_single() pads passt's own outbound frames to ETH_ZLEN, so
the receive path was already stricter than the send path.

Trim the trailing padding to the IP datagram length instead, using a
new iov_tail_trim() helper, and keep dropping frames genuinely
shorter than the datagram they claim to carry. IPv6 is unaffected:
its minimal TCP frame is 74 bytes, above the padding threshold.

Signed-off-by: David du Colombier <0intro@gmail.com>
---
v2:
  - store the iov_tail_size() result in a local variable instead of
    computing it twice
  - pass ARRAY_SIZE(trim_iov) instead of UIO_MAXIOV

 iov.c | 35 +++++++++++++++++++++++++++++++++++
 iov.h |  2 ++
 tap.c | 15 +++++++++++++--
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/iov.c b/iov.c
index 6fd684a..968a365 100644
--- a/iov.c
+++ b/iov.c
@@ -450,3 +450,38 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
 
 	return j;
 }
+
+/**
+ * iov_tail_trim() - Limit a tail to @len bytes via a scratch iovec array
+ * @tail:	 Pointer to the iov_tail to trim; rebuilt on success to
+ *		 reference @scratch
+ * @len:	 Number of bytes to keep
+ * @scratch:	 Scratch iovec array backing the trimmed tail; must stay
+ *		 valid as long as the trimmed tail is in use
+ * @scratch_cnt: Number of elements in @scratch
+ *
+ * Return: true on success, false if @tail is shorter than @len or does
+ *	   not fit in @scratch (@tail is unchanged on failure)
+ */
+bool iov_tail_trim(struct iov_tail *tail, size_t len,
+		   struct iovec *scratch, size_t scratch_cnt)
+{
+	ssize_t cnt = iov_tail_clone(scratch, scratch_cnt, tail);
+	size_t left = len;
+	unsigned int i;
+
+	if (cnt < 0)
+		return false;
+
+	for (i = 0; i < (size_t)cnt && left; i++) {
+		if (scratch[i].iov_len > left)
+			scratch[i].iov_len = left;
+		left -= scratch[i].iov_len;
+	}
+
+	if (left)
+		return false;
+
+	*tail = IOV_TAIL(scratch, i, 0);
+	return true;
+}
diff --git a/iov.h b/iov.h
index 4fdf14a..3af467e 100644
--- a/iov.h
+++ b/iov.h
@@ -97,6 +97,8 @@ size_t iov_push_header_(struct iov_tail *tail, const void *v, size_t len);
 void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t align);
 ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
 		       struct iov_tail *tail);
+bool iov_tail_trim(struct iov_tail *tail, size_t len,
+		   struct iovec *scratch, size_t scratch_cnt);
 
 /**
  * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail
diff --git a/tap.c b/tap.c
index 4cba4c7..6ed7f8d 100644
--- a/tap.c
+++ b/tap.c
@@ -716,7 +716,8 @@ static int tap4_handler(struct ctx *c, const struct pool *in,
 	i = 0;
 resume:
 	for (seq_count = 0, seq = NULL; i < in->count; i++) {
-		size_t l3len, hlen, l4len;
+		struct iovec trim_iov[UIO_MAXIOV];
+		size_t l3len, hlen, l4len, check;
 		struct ethhdr eh_storage;
 		struct iphdr iph_storage;
 		struct udphdr uh_storage;
@@ -775,7 +776,17 @@ resume:
 
 		if (!iov_drop_header(&data, hlen))
 			continue;
-		if (iov_tail_size(&data) != l4len)
+
+		check = iov_tail_size(&data);
+		if (check < l4len)
+			continue;
+
+		/* Drivers modelled on real hardware (Plan 9's virtio, for
+		 * one) pad short frames to the 60 byte Ethernet minimum:
+		 * trim trailing padding instead of dropping the packet.
+		 */
+		if (check > l4len &&
+		    !iov_tail_trim(&data, l4len, trim_iov, ARRAY_SIZE(trim_iov)))
 			continue;
 
 		if (iph->protocol == IPPROTO_ICMP) {
-- 
2.54.0