[PATCH v2 10/14] tap: Split tap_ip6_send() into UDP and ICMP variants

[David Gibson <david@gibson.dropbear.id.au>]

tap_ip6_send() has special case logic to compute the checksums for UDP
and ICMP packets, which is a mild layering violation.  By using a suitable
helper we can split it into tap_udp6_send() and tap_icmp6_send() functions
without greatly increasing the code size, this removing that layering
violation.

We make some small changes to the interface while there.  In both cases
we make the destination IPv6 address a parameter, which will be useful
later.  For the UDP variant we make it take just the UDP payload, and it
will generate the UDP header.  For the ICMP variant we pass in the ICMP
header as before.  The inconsistency is because that's what seems to be
the more natural way to invoke the function in the callers in each case.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 dhcpv6.c | 21 +++------------
 icmp.c   |  3 ++-
 tap.c    | 82 ++++++++++++++++++++++++++++++++++++++++++--------------
 tap.h    |  9 +++++--
 4 files changed, 75 insertions(+), 40 deletions(-)

diff --git a/dhcpv6.c b/dhcpv6.c
index 7829968..e763aed 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -208,15 +208,8 @@ struct msg_hdr {
 	uint32_t xid:24;
 } __attribute__((__packed__));
 
-#if __BYTE_ORDER == __BIG_ENDIAN
-#define UH_RESP {{{ 547, 546, 0, 0, }}}
-#else
-#define UH_RESP {{{ __bswap_constant_16(547), __bswap_constant_16(546), 0, 0 }}}
-#endif
-
 /**
  * struct resp_t - Normal advertise and reply message
- * @uh:			UDP header
  * @hdr:		DHCP message header
  * @server_id:		Server Identifier option
  * @ia_na:		Non-temporary Address option
@@ -226,7 +219,6 @@ struct msg_hdr {
  * @dns_search:		Domain Search List, here just for storage size
  */
 static struct resp_t {
-	struct udphdr  uh;
 	struct msg_hdr hdr;
 
 	struct opt_server_id server_id;
@@ -236,7 +228,6 @@ static struct resp_t {
 	struct opt_dns_servers dns_servers;
 	struct opt_dns_search dns_search;
 } __attribute__((__packed__)) resp = {
-	UH_RESP,
 	{ 0 },
 	SERVER_ID,
 
@@ -270,13 +261,11 @@ static const struct opt_status_code sc_not_on_link = {
 
 /**
  * struct resp_not_on_link_t - NotOnLink error (mandated by RFC 8415, 18.3.2.)
- * @uh:		UDP header
  * @hdr:	DHCP message header
  * @server_id:	Server Identifier option
  * @var:	Payload: IA_NA from client, status code, client ID
  */
 static struct resp_not_on_link_t {
-	struct udphdr  uh;
 	struct msg_hdr hdr;
 
 	struct opt_server_id server_id;
@@ -284,7 +273,6 @@ static struct resp_not_on_link_t {
 	uint8_t var[sizeof(struct opt_ia_na) + sizeof(struct opt_status_code) +
 		    sizeof(struct opt_client_id)];
 } __attribute__((__packed__)) resp_not_on_link = {
-	UH_RESP,
 	{ TYPE_REPLY, 0 },
 	SERVER_ID,
 	{ 0, },
@@ -527,12 +515,11 @@ int dhcpv6(struct ctx *c, const struct pool *p,
 			n += sizeof(struct opt_hdr) + ntohs(client_id->l);
 
 			n = offsetof(struct resp_not_on_link_t, var) + n;
-			resp_not_on_link.uh.len = htons(n);
 
 			resp_not_on_link.hdr.xid = mh->xid;
 
-			tap_ip6_send(c, src, IPPROTO_UDP,
-				     (char *)&resp_not_on_link, n, mh->xid);
+			tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546,
+				      mh->xid, &resp_not_on_link, n);
 
 			return 1;
 		}
@@ -576,11 +563,11 @@ int dhcpv6(struct ctx *c, const struct pool *p,
 	n = offsetof(struct resp_t, client_id) +
 	    sizeof(struct opt_hdr) + ntohs(client_id->l);
 	n = dhcpv6_dns_fill(c, (char *)&resp, n);
-	resp.uh.len = htons(n);
 
 	resp.hdr.xid = mh->xid;
 
-	tap_ip6_send(c, src, IPPROTO_UDP, (char *)&resp, n, mh->xid);
+	tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546,
+		      mh->xid, &resp, n);
 	c->ip6.addr_seen = c->ip6.addr;
 
 	return 1;
diff --git a/icmp.c b/icmp.c
index 61c2d90..6493ea9 100644
--- a/icmp.c
+++ b/icmp.c
@@ -105,7 +105,8 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref,
 			icmp_id_map[V6][id].seq = seq;
 		}
 
-		tap_ip6_send(c, &sr6->sin6_addr, IPPROTO_ICMPV6, buf, n, 0);
+		tap_icmp6_send(c, &sr6->sin6_addr,
+			       tap_ip6_daddr(c, &sr6->sin6_addr), buf, n);
 	} else {
 		struct sockaddr_in *sr4 = (struct sockaddr_in *)&sr;
 		struct icmphdr *ih = (struct icmphdr *)buf;
diff --git a/tap.c b/tap.c
index 0e8c99b..135d799 100644
--- a/tap.c
+++ b/tap.c
@@ -175,21 +175,22 @@ void tap_ip4_send(const struct ctx *c, in_addr_t src, uint8_t proto,
 }
 
 /**
- * tap_ip6_send() - Send IPv6 packet, with L2 headers, calculating L3/L4 checksums
+ * tap_push_ip6h() - Build IPv6 header for inbound packet
  * @c:		Execution context
  * @src:	IPv6 source address
- * @proto:	L4 protocol number
- * @in:		Payload
+ * @dst:	IPv6 destination address
  * @len:	L4 payload length
- * @flow:	Flow label
+ * @proto:	L4 protocol number
+ * @flow:	IPv6 flow identifier
+ *
+ * Return: pointer at which to write the packet's payload
  */
-void tap_ip6_send(const struct ctx *c, const struct in6_addr *src,
-		  uint8_t proto, const char *in, size_t len, uint32_t flow)
+static void *tap_push_ip6h(char *buf,
+			   const struct in6_addr *src,
+			   const struct in6_addr *dst,
+			   size_t len, uint8_t proto, uint32_t flow)
 {
-	char buf[USHRT_MAX];
-	struct ipv6hdr *ip6h =
-		(struct ipv6hdr *)tap_push_l2h(c, buf, ETH_P_IPV6);
-	char *data = (char *)(ip6h + 1);
+	struct ipv6hdr *ip6h = (struct ipv6hdr *)buf;
 
 	ip6h->payload_len = htons(len);
 	ip6h->priority = 0;
@@ -197,24 +198,65 @@ void tap_ip6_send(const struct ctx *c, const struct in6_addr *src,
 	ip6h->nexthdr = proto;
 	ip6h->hop_limit = 255;
 	ip6h->saddr = *src;
-	ip6h->daddr = *tap_ip6_daddr(c, src);
+	ip6h->daddr = *dst;
 	ip6h->flow_lbl[0] = (flow >> 16) & 0xf;
 	ip6h->flow_lbl[1] = (flow >> 8) & 0xff;
 	ip6h->flow_lbl[2] = (flow >> 0) & 0xff;
+	return ip6h + 1;
+}
 
+/**
+ * tap_udp6_send() - Send UDP over IPv6 packet
+ * @c:		Execution context
+ * @src:	IPv6 source address
+ * @sport:	UDP source port
+ * @dst:	IPv6 destination address
+ * @dport:	UDP destination port
+ * @flow:	Flow label
+ * @in:		UDP payload contents (not including UDP header)
+ * @len:	UDP payload length (not including UDP header)
+ */
+void tap_udp6_send(const struct ctx *c,
+		   const struct in6_addr *src, in_port_t sport,
+		   const struct in6_addr *dst, in_port_t dport,
+		   uint32_t flow, const void *in, size_t len)
+{
+	size_t udplen = len + sizeof(struct udphdr);
+	char buf[USHRT_MAX];
+	void *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
+	void *uhp = tap_push_ip6h(ip6h, src, dst, udplen, IPPROTO_UDP, flow);
+	struct udphdr *uh = (struct udphdr *)uhp;
+	char *data = (char *)(uh + 1);
+
+	uh->source = htons(sport);
+	uh->dest = htons(dport);
+	uh->len = htons(udplen);
+	csum_udp6(uh, src, dst, in, len);
 	memcpy(data, in, len);
 
-	if (proto == IPPROTO_UDP) {
-		struct udphdr *uh = (struct udphdr *)(ip6h + 1);
+	if (tap_send(c, buf, len + (data - buf)) < 1)
+		debug("tap: failed to send %lu bytes (IPv6)", len);
+}
 
-		csum_udp6(uh, &ip6h->saddr, &ip6h->daddr,
-			  uh + 1, len - sizeof(*uh));
-	} else if (proto == IPPROTO_ICMPV6) {
-		struct icmp6hdr *ih = (struct icmp6hdr *)(ip6h + 1);
+/**
+ * tap_icmp6_send() - Send ICMPv6 packet
+ * @c:		Execution context
+ * @src:	IPv6 source address
+ * @dst:	IPv6 destination address
+ * @in:		ICMP packet, including ICMP header
+ * @len:	ICMP packet length, including ICMP header
+ */
+void tap_icmp6_send(const struct ctx *c,
+		    const struct in6_addr *src, const struct in6_addr *dst,
+		    void *in, size_t len)
+{
+	char buf[USHRT_MAX];
+	void *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
+	char *data = tap_push_ip6h(ip6h, src, dst, len, IPPROTO_ICMPV6, 0);
+	struct icmp6hdr *icmp6h = (struct icmp6hdr *)data;
 
-		csum_icmp6(ih, &ip6h->saddr, &ip6h->daddr,
-			   ih + 1, len - sizeof(*ih));
-	}
+	memcpy(data, in, len);
+	csum_icmp6(icmp6h, src, dst, icmp6h + 1, len - sizeof(*icmp6h));
 
 	if (tap_send(c, buf, len + (data - buf)) < 1)
 		debug("tap: failed to send %lu bytes (IPv6)", len);
diff --git a/tap.h b/tap.h
index 011ba8e..d43c7a0 100644
--- a/tap.h
+++ b/tap.h
@@ -11,8 +11,13 @@ const struct in6_addr *tap_ip6_daddr(const struct ctx *c,
 				     const struct in6_addr *src);
 void tap_ip4_send(const struct ctx *c, in_addr_t src, uint8_t proto,
 		  const char *in, size_t len);
-void tap_ip6_send(const struct ctx *c, const struct in6_addr *src,
-		  uint8_t proto, const char *in, size_t len, uint32_t flow);
+void tap_udp6_send(const struct ctx *c,
+		   const struct in6_addr *src, in_port_t sport,
+		   const struct in6_addr *dst, in_port_t dport,
+		   uint32_t flow, const void *in, size_t len);
+void tap_icmp6_send(const struct ctx *c,
+		    const struct in6_addr *src, const struct in6_addr *dst,
+		    void *in, size_t len);
 int tap_send(const struct ctx *c, const void *data, size_t len);
 void tap_handler(struct ctx *c, int fd, uint32_t events,
 		 const struct timespec *now);
-- 
@@ -11,8 +11,13 @@ const struct in6_addr *tap_ip6_daddr(const struct ctx *c,
 				     const struct in6_addr *src);
 void tap_ip4_send(const struct ctx *c, in_addr_t src, uint8_t proto,
 		  const char *in, size_t len);
-void tap_ip6_send(const struct ctx *c, const struct in6_addr *src,
-		  uint8_t proto, const char *in, size_t len, uint32_t flow);
+void tap_udp6_send(const struct ctx *c,
+		   const struct in6_addr *src, in_port_t sport,
+		   const struct in6_addr *dst, in_port_t dport,
+		   uint32_t flow, const void *in, size_t len);
+void tap_icmp6_send(const struct ctx *c,
+		    const struct in6_addr *src, const struct in6_addr *dst,
+		    void *in, size_t len);
 int tap_send(const struct ctx *c, const void *data, size_t len);
 void tap_handler(struct ctx *c, int fd, uint32_t events,
 		 const struct timespec *now);
-- 
2.37.3