Fix bugs in TCP checksumming

Wasn't computing pseudo-header checksums (need to use tcp_v*_check).
Also, wasn't even trying to compute checksums for control packets.

Author: johnousterhout

homa_devel.c

@@ -424,7 +424,6 @@ void homa_freeze_peers(void)
 	freeze.common.type = FREEZE;
 	freeze.common.sport = htons(hsk->port);
 	freeze.common.dport = 0;
-	IF_NO_STRIP(homa_set_hijack(&freeze.common));
 	freeze.common.sender_id = 0;
 
 	rhashtable_walk_enter(&hnet->homa->peertab->ht, &iter);
@@ -1270,3 +1269,33 @@ void homa_validate_rbtree(struct rb_node *node, int depth, char *message)
 #endif /* __UNIT_TEST__ */
 }
 #endif /* See strip.py */
+
+/**
+ * homa_tcp_checksum() - Compute the TCP checksum for a packet. This is
+ * done "from scratch", i.e. not using any existing information such
+ * as skb->csum.
+ * @skb:     Contains the packet to checksum
+ * Return:   Checksum for the packet: 0 is the "correct" value.
+ */
+int homa_tcp_checksum(struct sk_buff *skb)
+{
+    int tcp_len = skb->len - skb_transport_offset(skb);
+    __wsum data_csum;
+
+    // Calculate the sum of the TCP header + data manually
+    data_csum = skb_checksum(skb, skb_transport_offset(skb), tcp_len, 0);
+
+    if (skb_is_ipv6(skb)) {
+        const struct ipv6hdr *ip6h = ipv6_hdr(skb);
+
+        // Fold the manual sum with the IPv6 pseudo-header
+        return csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr, tcp_len,
+                               IPPROTO_TCP, data_csum);
+    } else {
+        const struct iphdr *iph = ip_hdr(skb);
+
+        // Fold the manual sum with the IPv4 pseudo-header
+        return csum_tcpudp_magic(iph->saddr, iph->daddr, tcp_len,
+                                 IPPROTO_TCP, data_csum);
+    }
+}

homa_devel.h

@@ -134,6 +134,7 @@ int      homa_snprintf(char *buffer, int size, int used,
 		       const char *format, ...) __printf(4, 5);
 char    *homa_symbol_for_type(uint8_t type);
 char    *homa_symbol_for_state(struct homa_rpc *rpc);
+int      homa_tcp_checksum(struct sk_buff *skb);
 int      homa_validate_incoming(struct homa *homa, int verbose,
 				int *link_errors);
 

homa_impl.h

@@ -572,19 +572,16 @@ static inline struct homa_skb_info *homa_get_skb_info(struct sk_buff *skb)
 }
 
 /**
- * homa_set_doff() - Fills in the doff TCP header field for a Homa packet.
- * @h:     Packet header whose doff field is to be set.
+ * homa_set_doff() - Fills in the doff TCP header field for a packet.
+ * @skb:   Packet whose doff field is to be set.
  * @size:  Size of the "header", bytes (must be a multiple of 4). This
  *         information is used only for TSO; it's the number of bytes
  *         that should be replicated in each segment. The bytes after
  *         this will be distributed among segments.
  */
-static inline void homa_set_doff(struct homa_data_hdr *h, int size)
+static inline void homa_set_doff(struct sk_buff *skb, int size)
 {
-	/* This fills in the high-order 4 bits of doff with the number of
-	 * words in the neader.
-	 */
-	h->common.doff = size << 2;
+	tcp_hdr(skb)->doff = size >> 2;
 }
 
 /** skb_is_ipv6() - Return true if the packet is encapsulated with IPv6,

homa_incoming.c

@@ -997,7 +997,6 @@ void homa_need_ack_pkt(struct sk_buff *skb, struct homa_sock *hsk,
 	ack.common.type = ACK;
 	ack.common.sport = h->dport;
 	ack.common.dport = h->sport;
-	IF_NO_STRIP(homa_set_hijack(&ack.common));
 	ack.common.sender_id = cpu_to_be64(id);
 	ack.num_acks = htons(homa_peer_get_acks(peer,
 						HOMA_MAX_ACKS_PER_PKT,

homa_outgoing.c

@@ -17,6 +17,39 @@
 #include "homa_stub.h"
 #endif /* See strip.py */
 
+#ifndef __STRIP__ /* See strip.py */
+/**
+ * homa_set_hijack() - Set fields in an outgoing Homa packet that are needed
+ * for TCP hijacking to work properly. This function doesn't actually cause
+ * the packet to be sent via TCP (that is determined by hsk->sock.sk_protocol,
+ * which is set elsewhere). The modifications made here are safe even if the
+ * packet isn't actually sent via TCP.
+ * @skb:    Packet buffer in which to set fields.
+ * @peer:   Peer that contains source and destination addresses for the packet.
+ * @ipv6:   True means the packet is going to be sent via IPv6; false means
+ *          IPv4.
+ */
+static inline void homa_set_hijack(struct sk_buff *skb, struct homa_peer *peer,
+				   bool ipv6)
+{
+	struct homa_common_hdr *h;
+
+	h = (struct homa_common_hdr *)skb_transport_header(skb);
+	h->flags = HOMA_TCP_FLAGS;
+	h->urgent = htons(HOMA_TCP_URGENT);
+	/* Arrange for proper TCP checksumming. */
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	skb->csum_start = skb_transport_header(skb) - skb->head;
+	skb->csum_offset = offsetof(struct homa_common_hdr, checksum);
+	if (ipv6)
+		h->checksum = ~tcp_v6_check(skb->len, &peer->flow.u.ip6.saddr,
+					    &peer->flow.u.ip6.daddr, 0);
+	else
+		h->checksum = ~tcp_v4_check(skb->len, peer->flow.u.ip4.saddr,
+					    peer->flow.u.ip4.daddr, 0);
+}
+#endif /* See strip.py */
+
 /**
  * homa_message_out_init() - Initialize rpc->msgout.
  * @rpc:       RPC whose output message should be initialized. Must be
@@ -160,8 +193,7 @@ struct sk_buff *homa_tx_data_pkt_alloc(struct homa_rpc *rpc,
 	h->common.dport = htons(rpc->dport);
 	h->common.sequence = htonl(offset);
 	h->common.type = DATA;
-	IF_NO_STRIP(homa_set_hijack(&h->common));
-	homa_set_doff(h, sizeof(struct homa_data_hdr));
+	homa_set_doff(skb, sizeof(struct homa_data_hdr));
 	h->common.checksum = 0;
 	h->common.sender_id = cpu_to_be64(rpc->id);
 	h->message_length = htonl(rpc->msgout.length);
@@ -190,7 +222,7 @@ struct sk_buff *homa_tx_data_pkt_alloc(struct homa_rpc *rpc,
 #else /* See strip.py */
 	if (segs > 1) {
 #endif /* See strip.py */
-		homa_set_doff(h, sizeof(struct homa_data_hdr)  -
+		homa_set_doff(skb, sizeof(struct homa_data_hdr)  -
 				sizeof(struct homa_seg_hdr));
 #ifndef __STRIP__ /* See strip.py */
 		h->seg.offset = htonl(offset);
@@ -429,7 +461,6 @@ int homa_xmit_control(enum homa_packet_type type, void *contents,
 	h->type = type;
 	h->sport = htons(rpc->hsk->port);
 	h->dport = htons(rpc->dport);
-	IF_NO_STRIP(homa_set_hijack(h));
 	h->sender_id = cpu_to_be64(rpc->id);
 	return __homa_xmit_control(contents, length, rpc->peer, rpc->hsk);
 }
@@ -474,12 +505,16 @@ int __homa_xmit_control(void *contents, size_t length, struct homa_peer *peer,
 	priority = hsk->homa->num_priorities - 1;
 #endif /* See strip.py */
 	skb->ooo_okay = 1;
+	homa_set_doff(skb, length);
 #ifndef __STRIP__ /* See strip.py */
 	if (hsk->inet.sk.sk_family == AF_INET6) {
+		homa_set_hijack(skb, peer, true);
 		result = ip6_xmit(&hsk->inet.sk, skb, &peer->flow.u.ip6, 0,
 				  NULL, hsk->homa->priority_map[priority] << 5,
 				  0);
 	} else {
+		homa_set_hijack(skb, peer, false);
+
 		/* This will find its way to the DSCP field in the IPv4 hdr. */
 		hsk->inet.tos = hsk->homa->priority_map[priority] << 5;
 		result = ip_queue_xmit(&hsk->inet.sk, skb, &peer->flow);
@@ -535,7 +570,6 @@ void homa_xmit_unknown(struct sk_buff *skb, struct homa_sock *hsk)
 	unknown.common.sport = h->dport;
 	unknown.common.dport = h->sport;
 	unknown.common.type = RPC_UNKNOWN;
-	IF_NO_STRIP(homa_set_hijack(&unknown.common));
 	unknown.common.sender_id = cpu_to_be64(homa_local_id(h->sender_id));
 	peer = homa_peer_get(hsk, &saddr);
 	if (!IS_ERR(peer)) {
@@ -680,15 +714,13 @@ void __homa_xmit_data(struct sk_buff *skb, struct homa_rpc *rpc)
 	skb_dst_set(skb, homa_get_dst(rpc->peer, rpc->hsk));
 
 	skb->ooo_okay = 1;
-	skb->ip_summed = CHECKSUM_PARTIAL;
-	skb->csum_start = skb_transport_header(skb) - skb->head;
-	skb->csum_offset = offsetof(struct homa_common_hdr, checksum);
 	if (rpc->hsk->inet.sk.sk_family == AF_INET6) {
 		tt_record4("calling ip6_xmit: wire_bytes %d, peer 0x%x, id %d, offset %d",
 			   homa_get_skb_info(skb)->wire_bytes,
 			   tt_addr(rpc->peer->addr), rpc->id,
 			   homa_get_skb_info(skb)->offset);
 #ifndef __STRIP__ /* See strip.py */
+		homa_set_hijack(skb, rpc->peer, true);
 		err = ip6_xmit(&rpc->hsk->inet.sk, skb, &rpc->peer->flow.u.ip6,
 			       0, NULL,
 			       rpc->hsk->homa->priority_map[priority] << 5, 0);
@@ -703,6 +735,7 @@ void __homa_xmit_data(struct sk_buff *skb, struct homa_rpc *rpc)
 			   homa_get_skb_info(skb)->offset);
 
 #ifndef __STRIP__ /* See strip.py */
+		homa_set_hijack(skb, rpc->peer, false);
 		rpc->hsk->inet.tos =
 				rpc->hsk->homa->priority_map[priority] << 5;
 		err = ip_queue_xmit(&rpc->hsk->inet.sk, skb, &rpc->peer->flow);

homa_wire.h

@@ -547,20 +547,6 @@ static inline u64 homa_local_id(__be64 sender_id)
 	return be64_to_cpu(sender_id) ^ 1;
 }
 
-#ifndef __STRIP__ /* See strip.py */
-/**
- * homa_set_hijack() - Set fields in a Homa header that are needed for
- * TCP hijacking to work properly.
- * @common:   Header in which to set fields.
- */
-static inline void homa_set_hijack(struct homa_common_hdr *common)
-{
-	common->flags = HOMA_TCP_FLAGS;
-	common->urgent = htons(HOMA_TCP_URGENT);
-	common->doff = 0x50;
-}
-#endif /* See strip.py */
-
 /**
  * homa_get_offset() - Returns the offset within message of the first byte
  * of data in a Homa DATA packet (the offset is stored in different places

test/mock.c

@@ -70,11 +70,16 @@ int mock_signal_pending;
 /* Used as current task during tests. Also returned by kthread_run. */
 struct task_struct mock_task;
 
-/* If a test sets this variable to nonzero, ip_queue_xmit will log
+/* If a test sets this variable to nonzero, ip*xmit will log
  * outgoing packets using the long format rather than short.
  */
 int mock_xmit_log_verbose;
 
+/* If a test sets this variable to nonzero, ip*xmit will log hijacking
+ * information from outgoing packets.
+ */
+int mock_xmit_log_hijack;
+
 /* If a test sets this variable to nonzero, calls to wake_up and
  * wake_up_all will be logged.
  */
@@ -460,6 +465,13 @@ void __copy_overflow(int size, unsigned long count)
 	abort();
 }
 
+__sum16 csum_ipv6_magic(const struct in6_addr *saddr,
+			const struct in6_addr *daddr,
+			__u32 len, __u8 proto, __wsum csum)
+{
+	return 0;
+}
+
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 int debug_lockdep_rcu_enabled(void)
 {
@@ -775,6 +787,13 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
 	else
 		homa_print_packet_short(skb, buffer, sizeof(buffer));
 	unit_log_printf("; ", "xmit %s", buffer);
+	if (mock_xmit_log_hijack) {
+		struct homa_common_hdr *h;
+
+		h = (struct homa_common_hdr *)skb_transport_header(skb);
+		unit_log_printf("; ", "hijack checksum %d, flags 0x%x",
+			       h->checksum, h->flags);
+	}
 	kfree_skb(skb);
 	return 0;
 }
@@ -802,6 +821,13 @@ int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
 	else
 		homa_print_packet_short(skb, buffer, sizeof(buffer));
 	unit_log_printf("; ", "xmit %s", buffer);
+	if (mock_xmit_log_hijack) {
+		struct homa_common_hdr *h;
+
+		h = (struct homa_common_hdr *)skb_transport_header(skb);
+		unit_log_printf("; ", "hijack checksum %d, flags 0x%x",
+			       h->checksum, h->flags);
+	}
 	kfree_skb(skb);
 	return 0;
 }
@@ -1428,6 +1454,10 @@ void sk_skb_reason_drop(struct sock *sk, struct sk_buff *skb,
 	__kfree_skb(skb);
 #endif
 }
+__wsum skb_checksum(const struct sk_buff *skb, int offset, int len, __wsum csum)
+{
+	return 0;
+}
 
 int skb_copy_datagram_iter(const struct sk_buff *from, int offset,
 		struct iov_iter *iter, int size)
@@ -2428,6 +2458,7 @@ void mock_teardown(void)
 	mock_prepare_to_wait_status = -ERESTARTSYS;
 	mock_signal_pending = 0;
 	mock_xmit_log_verbose = 0;
+	mock_xmit_log_hijack = 0;
 	mock_log_wakeups = 0;
 	mock_mtu = 0;
 	mock_max_skb_frags = MAX_SKB_FRAGS;

test/mock.h

@@ -104,6 +104,12 @@
 
 #define spin_unlock mock_spin_unlock
 
+#undef tcp_v4_check
+#define tcp_v4_check(...) (~(__force __sum16)444U)
+
+#undef tcp_v6_check
+#define tcp_v6_check(...) (~(__force __sum16)666U)
+
 #undef this_cpu_ptr
 #define this_cpu_ptr(name) (&name[cpu_number])
 
@@ -180,6 +186,7 @@ extern int         mock_trylock_errors;
 extern u64         mock_tt_cycles;
 extern int         mock_vmalloc_errors;
 extern int         mock_xmit_log_verbose;
+extern int         mock_xmit_log_hijack;
 
 extern struct task_struct *current_task;