[PATCH 0/6] udp: Small cleanups

[PATCH 0/6] udp: Small cleanups

[David Gibson]

Following on from the recent set of small fixes for UDP, here are a
number of small cleanups to the UDP code, which will simplify later
more complex fixes and improvements.

Laurent, I expect this will have some conflicts with part 2 of your
vhost-user work, sorry.

David Gibson (6):
  udp: Refactor udp_sock[46]_iov_init()
  udp: Tweak interface to udp_update_hdr[46]
  udp: Clarify setting of addresses inin udp_update_hdr[46]()
  udp: Consistent port variable names in udp_update_hdr[46]
  udp: Avoid unnecessary pointer in udp_update_hdr4()
  udp: Clean up UDP checksum generation for inbound IPv6 packets

 udp.c | 235 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 108 insertions(+), 127 deletions(-)

-- 
2.44.0

[PATCH 1/6] udp: Refactor udp_sock[46]_iov_init()

[David Gibson]

Each of these functions have 3 essentially identical loops in a row.
Merge the loops into a single common udp_sock_iov_init() function, calling
udp_sock[46]_iov_init_one() helpers to initialize each "slot" in the
various parallel arrays.  This is slightly neater now, and more naturally
allows changes we want to make where more initialization will become common
between IPv4 and IPv6.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 102 ++++++++++++++++++++++++++++------------------------------
 1 file changed, 50 insertions(+), 52 deletions(-)

diff --git a/udp.c b/udp.c
index 5b7778eb..092d343e 100644
--- a/udp.c
+++ b/udp.c
@@ -307,72 +307,74 @@ void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s)
 }
 
 /**
- * udp_sock4_iov_init() - Initialise scatter-gather L2 buffers for IPv4 sockets
+ * udp_sock4_iov_init_one() - Initialise a scatter-gather L2 buffer for IPv4
  * @c:		Execution context
+ * @i:		Index of buffer to initialize
  */
-static void udp_sock4_iov_init(const struct ctx *c)
+static void udp_sock4_iov_init_one(const struct ctx *c, size_t i)
 {
-	struct mmsghdr *h;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(udp4_l2_buf); i++) {
-		udp4_l2_buf[i] = (struct udp4_l2_buf_t) {
-			.taph = TAP_HDR_INIT(ETH_P_IP),
-			.iph = L2_BUF_IP4_INIT(IPPROTO_UDP)
-		};
-	}
-
-	for (i = 0, h = udp4_l2_mh_sock; i < UDP_MAX_FRAMES; i++, h++) {
-		struct msghdr *mh = &h->msg_hdr;
+	struct msghdr *mh = &udp4_l2_mh_sock[i].msg_hdr;
+	struct udp4_l2_buf_t *buf = &udp4_l2_buf[i];
+	struct iovec *siov = &udp4_l2_iov_sock[i];
+	struct iovec *tiov = &udp4_l2_iov_tap[i];
 
-		mh->msg_name			= &udp4_l2_buf[i].s_in;
-		mh->msg_namelen			= sizeof(udp4_l2_buf[i].s_in);
+	*buf = (struct udp4_l2_buf_t) {
+		.taph = TAP_HDR_INIT(ETH_P_IP),
+		.iph = L2_BUF_IP4_INIT(IPPROTO_UDP)
+	};
 
-		udp4_l2_iov_sock[i].iov_base	= udp4_l2_buf[i].data;
-		udp4_l2_iov_sock[i].iov_len	= sizeof(udp4_l2_buf[i].data);
-		mh->msg_iov			= &udp4_l2_iov_sock[i];
-		mh->msg_iovlen			= 1;
-	}
+	siov->iov_base	= buf->data;
+	siov->iov_len	= sizeof(buf->data);
 
-	for (i = 0; i < UDP_MAX_FRAMES; i++) {
-		struct iovec *iov = &udp4_l2_iov_tap[i];
+	mh->msg_name	= &buf->s_in;
+	mh->msg_namelen	= sizeof(buf->s_in);
+	mh->msg_iov	= siov;
+	mh->msg_iovlen	= 1;
 
-		iov->iov_base = tap_iov_base(c, &udp4_l2_buf[i].taph);
-	}
+	tiov->iov_base	= tap_iov_base(c, &buf->taph);
 }
 
 /**
- * udp_sock6_iov_init() - Initialise scatter-gather L2 buffers for IPv6 sockets
+ * udp_sock6_iov_init_one() - Initialise a scatter-gather L2 buffer for IPv6
  * @c:		Execution context
+ * @i:		Index of buffer to initialize
  */
-static void udp_sock6_iov_init(const struct ctx *c)
+static void udp_sock6_iov_init_one(const struct ctx *c, size_t i)
 {
-	struct mmsghdr *h;
-	int i;
+	struct msghdr *mh = &udp6_l2_mh_sock[i].msg_hdr;
+	struct udp6_l2_buf_t *buf = &udp6_l2_buf[i];
+	struct iovec *siov = &udp6_l2_iov_sock[i];
+	struct iovec *tiov = &udp6_l2_iov_tap[i];
 
-	for (i = 0; i < ARRAY_SIZE(udp6_l2_buf); i++) {
-		udp6_l2_buf[i] = (struct udp6_l2_buf_t) {
-			.taph = TAP_HDR_INIT(ETH_P_IPV6),
-			.ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP)
-		};
-	}
+	*buf = (struct udp6_l2_buf_t) {
+		.taph = TAP_HDR_INIT(ETH_P_IPV6),
+		.ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP)
+	};
 
-	for (i = 0, h = udp6_l2_mh_sock; i < UDP_MAX_FRAMES; i++, h++) {
-		struct msghdr *mh = &h->msg_hdr;
+	siov->iov_base	= buf->data;
+	siov->iov_len	= sizeof(buf->data);
 
-		mh->msg_name			= &udp6_l2_buf[i].s_in6;
-		mh->msg_namelen			= sizeof(struct sockaddr_in6);
+	mh->msg_name	= &buf->s_in6;
+	mh->msg_namelen	= sizeof(buf->s_in6);
+	mh->msg_iov	= siov;
+	mh->msg_iovlen	= 1;
 
-		udp6_l2_iov_sock[i].iov_base	= udp6_l2_buf[i].data;
-		udp6_l2_iov_sock[i].iov_len	= sizeof(udp6_l2_buf[i].data);
-		mh->msg_iov			= &udp6_l2_iov_sock[i];
-		mh->msg_iovlen			= 1;
-	}
+	tiov->iov_base	= tap_iov_base(c, &buf->taph);
+}
 
-	for (i = 0; i < UDP_MAX_FRAMES; i++) {
-		struct iovec *iov = &udp6_l2_iov_tap[i];
+/**
+ * udp_sock_iov_init() - Initialise scatter-gather L2 buffers
+ * @c:		Execution context
+ */
+static void udp_sock_iov_init(const struct ctx *c)
+{
+	size_t i;
 
-		iov->iov_base = tap_iov_base(c, &udp6_l2_buf[i].taph);
+	for (i = 0; i < UDP_MAX_FRAMES; i++) {
+		if (c->ifi4)
+			udp_sock4_iov_init_one(c, i);
+		if (c->ifi6)
+			udp_sock6_iov_init_one(c, i);
 	}
 }
 
@@ -1240,11 +1242,7 @@ v6:
  */
 int udp_init(struct ctx *c)
 {
-	if (c->ifi4)
-		udp_sock4_iov_init(c);
-
-	if (c->ifi6)
-		udp_sock6_iov_init(c);
+	udp_sock_iov_init(c);
 
 	udp_invert_portmap(&c->udp.fwd_in);
 	udp_invert_portmap(&c->udp.fwd_out);
-- 
@@ -307,72 +307,74 @@ void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s)
 }
 
 /**
- * udp_sock4_iov_init() - Initialise scatter-gather L2 buffers for IPv4 sockets
+ * udp_sock4_iov_init_one() - Initialise a scatter-gather L2 buffer for IPv4
  * @c:		Execution context
+ * @i:		Index of buffer to initialize
  */
-static void udp_sock4_iov_init(const struct ctx *c)
+static void udp_sock4_iov_init_one(const struct ctx *c, size_t i)
 {
-	struct mmsghdr *h;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(udp4_l2_buf); i++) {
-		udp4_l2_buf[i] = (struct udp4_l2_buf_t) {
-			.taph = TAP_HDR_INIT(ETH_P_IP),
-			.iph = L2_BUF_IP4_INIT(IPPROTO_UDP)
-		};
-	}
-
-	for (i = 0, h = udp4_l2_mh_sock; i < UDP_MAX_FRAMES; i++, h++) {
-		struct msghdr *mh = &h->msg_hdr;
+	struct msghdr *mh = &udp4_l2_mh_sock[i].msg_hdr;
+	struct udp4_l2_buf_t *buf = &udp4_l2_buf[i];
+	struct iovec *siov = &udp4_l2_iov_sock[i];
+	struct iovec *tiov = &udp4_l2_iov_tap[i];
 
-		mh->msg_name			= &udp4_l2_buf[i].s_in;
-		mh->msg_namelen			= sizeof(udp4_l2_buf[i].s_in);
+	*buf = (struct udp4_l2_buf_t) {
+		.taph = TAP_HDR_INIT(ETH_P_IP),
+		.iph = L2_BUF_IP4_INIT(IPPROTO_UDP)
+	};
 
-		udp4_l2_iov_sock[i].iov_base	= udp4_l2_buf[i].data;
-		udp4_l2_iov_sock[i].iov_len	= sizeof(udp4_l2_buf[i].data);
-		mh->msg_iov			= &udp4_l2_iov_sock[i];
-		mh->msg_iovlen			= 1;
-	}
+	siov->iov_base	= buf->data;
+	siov->iov_len	= sizeof(buf->data);
 
-	for (i = 0; i < UDP_MAX_FRAMES; i++) {
-		struct iovec *iov = &udp4_l2_iov_tap[i];
+	mh->msg_name	= &buf->s_in;
+	mh->msg_namelen	= sizeof(buf->s_in);
+	mh->msg_iov	= siov;
+	mh->msg_iovlen	= 1;
 
-		iov->iov_base = tap_iov_base(c, &udp4_l2_buf[i].taph);
-	}
+	tiov->iov_base	= tap_iov_base(c, &buf->taph);
 }
 
 /**
- * udp_sock6_iov_init() - Initialise scatter-gather L2 buffers for IPv6 sockets
+ * udp_sock6_iov_init_one() - Initialise a scatter-gather L2 buffer for IPv6
  * @c:		Execution context
+ * @i:		Index of buffer to initialize
  */
-static void udp_sock6_iov_init(const struct ctx *c)
+static void udp_sock6_iov_init_one(const struct ctx *c, size_t i)
 {
-	struct mmsghdr *h;
-	int i;
+	struct msghdr *mh = &udp6_l2_mh_sock[i].msg_hdr;
+	struct udp6_l2_buf_t *buf = &udp6_l2_buf[i];
+	struct iovec *siov = &udp6_l2_iov_sock[i];
+	struct iovec *tiov = &udp6_l2_iov_tap[i];
 
-	for (i = 0; i < ARRAY_SIZE(udp6_l2_buf); i++) {
-		udp6_l2_buf[i] = (struct udp6_l2_buf_t) {
-			.taph = TAP_HDR_INIT(ETH_P_IPV6),
-			.ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP)
-		};
-	}
+	*buf = (struct udp6_l2_buf_t) {
+		.taph = TAP_HDR_INIT(ETH_P_IPV6),
+		.ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP)
+	};
 
-	for (i = 0, h = udp6_l2_mh_sock; i < UDP_MAX_FRAMES; i++, h++) {
-		struct msghdr *mh = &h->msg_hdr;
+	siov->iov_base	= buf->data;
+	siov->iov_len	= sizeof(buf->data);
 
-		mh->msg_name			= &udp6_l2_buf[i].s_in6;
-		mh->msg_namelen			= sizeof(struct sockaddr_in6);
+	mh->msg_name	= &buf->s_in6;
+	mh->msg_namelen	= sizeof(buf->s_in6);
+	mh->msg_iov	= siov;
+	mh->msg_iovlen	= 1;
 
-		udp6_l2_iov_sock[i].iov_base	= udp6_l2_buf[i].data;
-		udp6_l2_iov_sock[i].iov_len	= sizeof(udp6_l2_buf[i].data);
-		mh->msg_iov			= &udp6_l2_iov_sock[i];
-		mh->msg_iovlen			= 1;
-	}
+	tiov->iov_base	= tap_iov_base(c, &buf->taph);
+}
 
-	for (i = 0; i < UDP_MAX_FRAMES; i++) {
-		struct iovec *iov = &udp6_l2_iov_tap[i];
+/**
+ * udp_sock_iov_init() - Initialise scatter-gather L2 buffers
+ * @c:		Execution context
+ */
+static void udp_sock_iov_init(const struct ctx *c)
+{
+	size_t i;
 
-		iov->iov_base = tap_iov_base(c, &udp6_l2_buf[i].taph);
+	for (i = 0; i < UDP_MAX_FRAMES; i++) {
+		if (c->ifi4)
+			udp_sock4_iov_init_one(c, i);
+		if (c->ifi6)
+			udp_sock6_iov_init_one(c, i);
 	}
 }
 
@@ -1240,11 +1242,7 @@ v6:
  */
 int udp_init(struct ctx *c)
 {
-	if (c->ifi4)
-		udp_sock4_iov_init(c);
-
-	if (c->ifi6)
-		udp_sock6_iov_init(c);
+	udp_sock_iov_init(c);
 
 	udp_invert_portmap(&c->udp.fwd_in);
 	udp_invert_portmap(&c->udp.fwd_out);
-- 
2.44.0

[PATCH 2/6] udp: Tweak interface to udp_update_hdr[46]

[David Gibson]

These functions take an index to the L2 buffer whose header information to
update.  They use that for two things: to locate the buffer pointer itself,
and to retrieve the length of the received message from the paralllel
udp[46]_l2_mh_sock array.  The latter is arguably a failure to separate
concerns.  Change these functions to explicitly take a buffer pointer and
payload length as parameters.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 46 ++++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/udp.c b/udp.c
index 092d343e..e3c51bae 100644
--- a/udp.c
+++ b/udp.c
@@ -576,28 +576,24 @@ static void udp_splice_sendfrom(const struct ctx *c, unsigned start, unsigned n,
 /**
  * udp_update_hdr4() - Update headers for one IPv4 datagram
  * @c:		Execution context
- * @n:		Index of buffer in udp4_l2_buf pool
+ * @b:		Pointer to udp4_l2_buf to update
  * @dstport:	Destination port number
+ * @datalen:	Length of UDP payload
  * @now:	Current timestamp
  *
  * Return: size of tap frame with headers
  */
-static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
+static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
+			      in_port_t dstport, size_t datalen,
 			      const struct timespec *now)
 {
-	struct udp4_l2_buf_t *b = &udp4_l2_buf[n];
-	struct in_addr *src;
-	in_port_t src_port;
-	size_t ip_len;
-
-	ip_len = udp4_l2_mh_sock[n].msg_len + sizeof(b->iph) + sizeof(b->uh);
+	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
+	in_port_t src_port = ntohs(b->s_in.sin_port);
+	struct in_addr *src = &b->s_in.sin_addr;
 
 	b->iph.tot_len = htons(ip_len);
 	b->iph.daddr = c->ip4.addr_seen.s_addr;
 
-	src = &b->s_in.sin_addr;
-	src_port = ntohs(b->s_in.sin_port);
-
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
 	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && src_port == 53) {
 		b->iph.saddr = c->ip4.dns_match.s_addr;
@@ -620,7 +616,7 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
 	udp_update_check4(b);
 	b->uh.source = b->s_in.sin_port;
 	b->uh.dest = htons(dstport);
-	b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh));
+	b->uh.len = htons(datalen + sizeof(b->uh));
 
 	return tap_iov_len(c, &b->taph, ip_len);
 }
@@ -628,26 +624,22 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
 /**
  * udp_update_hdr6() - Update headers for one IPv6 datagram
  * @c:		Execution context
- * @n:		Index of buffer in udp6_l2_buf pool
+ * @b:		Pointer to udp6_l2_buf to update
  * @dstport:	Destination port number
+ * @datalen:	Length of UDP payload
  * @now:	Current timestamp
  *
  * Return: size of tap frame with headers
  */
-static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
+static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
+			      in_port_t dstport, size_t datalen,
 			      const struct timespec *now)
 {
-	struct udp6_l2_buf_t *b = &udp6_l2_buf[n];
-	struct in6_addr *src;
-	in_port_t src_port;
-	size_t ip_len;
-
-	src = &b->s_in6.sin6_addr;
-	src_port = ntohs(b->s_in6.sin6_port);
-
-	ip_len = udp6_l2_mh_sock[n].msg_len + sizeof(b->ip6h) + sizeof(b->uh);
+	size_t ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
+	in_port_t src_port = ntohs(b->s_in6.sin6_port);
+	struct in6_addr *src = &b->s_in6.sin6_addr;
 
-	b->ip6h.payload_len = htons(udp6_l2_mh_sock[n].msg_len + sizeof(b->uh));
+	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
 
 	if (IN6_IS_ADDR_LINKLOCAL(src)) {
 		b->ip6h.daddr = c->ip6.addr_ll_seen;
@@ -727,9 +719,11 @@ static void udp_tap_send(const struct ctx *c,
 		size_t buf_len;
 
 		if (v6)
-			buf_len = udp_update_hdr6(c, i, dstport, now);
+			buf_len = udp_update_hdr6(c, &udp6_l2_buf[i], dstport,
+						  udp6_l2_mh_sock[i].msg_len, now);
 		else
-			buf_len = udp_update_hdr4(c, i, dstport, now);
+			buf_len = udp_update_hdr4(c, &udp4_l2_buf[i], dstport,
+						  udp4_l2_mh_sock[i].msg_len, now);
 
 		tap_iov[i].iov_len = buf_len;
 	}
-- 
@@ -576,28 +576,24 @@ static void udp_splice_sendfrom(const struct ctx *c, unsigned start, unsigned n,
 /**
  * udp_update_hdr4() - Update headers for one IPv4 datagram
  * @c:		Execution context
- * @n:		Index of buffer in udp4_l2_buf pool
+ * @b:		Pointer to udp4_l2_buf to update
  * @dstport:	Destination port number
+ * @datalen:	Length of UDP payload
  * @now:	Current timestamp
  *
  * Return: size of tap frame with headers
  */
-static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
+static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
+			      in_port_t dstport, size_t datalen,
 			      const struct timespec *now)
 {
-	struct udp4_l2_buf_t *b = &udp4_l2_buf[n];
-	struct in_addr *src;
-	in_port_t src_port;
-	size_t ip_len;
-
-	ip_len = udp4_l2_mh_sock[n].msg_len + sizeof(b->iph) + sizeof(b->uh);
+	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
+	in_port_t src_port = ntohs(b->s_in.sin_port);
+	struct in_addr *src = &b->s_in.sin_addr;
 
 	b->iph.tot_len = htons(ip_len);
 	b->iph.daddr = c->ip4.addr_seen.s_addr;
 
-	src = &b->s_in.sin_addr;
-	src_port = ntohs(b->s_in.sin_port);
-
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
 	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && src_port == 53) {
 		b->iph.saddr = c->ip4.dns_match.s_addr;
@@ -620,7 +616,7 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
 	udp_update_check4(b);
 	b->uh.source = b->s_in.sin_port;
 	b->uh.dest = htons(dstport);
-	b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh));
+	b->uh.len = htons(datalen + sizeof(b->uh));
 
 	return tap_iov_len(c, &b->taph, ip_len);
 }
@@ -628,26 +624,22 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport,
 /**
  * udp_update_hdr6() - Update headers for one IPv6 datagram
  * @c:		Execution context
- * @n:		Index of buffer in udp6_l2_buf pool
+ * @b:		Pointer to udp6_l2_buf to update
  * @dstport:	Destination port number
+ * @datalen:	Length of UDP payload
  * @now:	Current timestamp
  *
  * Return: size of tap frame with headers
  */
-static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport,
+static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
+			      in_port_t dstport, size_t datalen,
 			      const struct timespec *now)
 {
-	struct udp6_l2_buf_t *b = &udp6_l2_buf[n];
-	struct in6_addr *src;
-	in_port_t src_port;
-	size_t ip_len;
-
-	src = &b->s_in6.sin6_addr;
-	src_port = ntohs(b->s_in6.sin6_port);
-
-	ip_len = udp6_l2_mh_sock[n].msg_len + sizeof(b->ip6h) + sizeof(b->uh);
+	size_t ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
+	in_port_t src_port = ntohs(b->s_in6.sin6_port);
+	struct in6_addr *src = &b->s_in6.sin6_addr;
 
-	b->ip6h.payload_len = htons(udp6_l2_mh_sock[n].msg_len + sizeof(b->uh));
+	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
 
 	if (IN6_IS_ADDR_LINKLOCAL(src)) {
 		b->ip6h.daddr = c->ip6.addr_ll_seen;
@@ -727,9 +719,11 @@ static void udp_tap_send(const struct ctx *c,
 		size_t buf_len;
 
 		if (v6)
-			buf_len = udp_update_hdr6(c, i, dstport, now);
+			buf_len = udp_update_hdr6(c, &udp6_l2_buf[i], dstport,
+						  udp6_l2_mh_sock[i].msg_len, now);
 		else
-			buf_len = udp_update_hdr4(c, i, dstport, now);
+			buf_len = udp_update_hdr4(c, &udp4_l2_buf[i], dstport,
+						  udp4_l2_mh_sock[i].msg_len, now);
 
 		tap_iov[i].iov_len = buf_len;
 	}
-- 
2.44.0

[PATCH 3/6] udp: Clarify setting of addresses inin udp_update_hdr[46]()

[David Gibson]

Both of these functions can set the final values of the addresses in the
header in several places, which can be hard to follow.  Change it to use
temporary locals, 'src' and 'dst' to track the addresses we're going to
want then write it to the actual header in one place.  This will make some
subsequent changes easier.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 49 ++++++++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/udp.c b/udp.c
index e3c51bae..a07c1a62 100644
--- a/udp.c
+++ b/udp.c
@@ -588,18 +588,14 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 			      const struct timespec *now)
 {
 	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
+	const struct in_addr *src = &b->s_in.sin_addr;
 	in_port_t src_port = ntohs(b->s_in.sin_port);
-	struct in_addr *src = &b->s_in.sin_addr;
-
-	b->iph.tot_len = htons(ip_len);
-	b->iph.daddr = c->ip4.addr_seen.s_addr;
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
 	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && src_port == 53) {
-		b->iph.saddr = c->ip4.dns_match.s_addr;
+		src = &c->ip4.dns_match;
 	} else if (IN4_IS_ADDR_LOOPBACK(src) ||
 		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr_seen)) {
-		b->iph.saddr = c->ip4.gw.s_addr;
 		udp_tap_map[V4][src_port].ts = now->tv_sec;
 		udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
 
@@ -609,12 +605,15 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 			udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
 
 		bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
-	} else {
-		b->iph.saddr = src->s_addr;
+
+		src = &c->ip4.gw;
 	}
 
+	b->iph.tot_len = htons(ip_len);
+	b->iph.daddr = c->ip4.addr_seen.s_addr;
+	b->iph.saddr = src->s_addr;
 	udp_update_check4(b);
-	b->uh.source = b->s_in.sin_port;
+	b->uh.source = htons(src_port);
 	b->uh.dest = htons(dstport);
 	b->uh.len = htons(datalen + sizeof(b->uh));
 
@@ -636,29 +635,19 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 			      const struct timespec *now)
 {
 	size_t ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
+	const struct in6_addr *src = &b->s_in6.sin6_addr;
+	const struct in6_addr *dst = &c->ip6.addr_seen;
 	in_port_t src_port = ntohs(b->s_in6.sin6_port);
-	struct in6_addr *src = &b->s_in6.sin6_addr;
-
-	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
 
 	if (IN6_IS_ADDR_LINKLOCAL(src)) {
-		b->ip6h.daddr = c->ip6.addr_ll_seen;
-		b->ip6h.saddr = b->s_in6.sin6_addr;
+		dst = &c->ip6.addr_ll_seen;
 	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) &&
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) &&
 		   src_port == 53) {
-		b->ip6h.daddr = c->ip6.addr_seen;
-		b->ip6h.saddr = c->ip6.dns_match;
+		src = &c->ip6.dns_match;
 	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) {
-		b->ip6h.daddr = c->ip6.addr_ll_seen;
-
-		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
-			b->ip6h.saddr = c->ip6.gw;
-		else
-			b->ip6h.saddr = c->ip6.addr_ll;
-
 		udp_tap_map[V6][src_port].ts = now->tv_sec;
 		udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
 
@@ -673,12 +662,18 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 			udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
 
 		bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
-	} else {
-		b->ip6h.daddr = c->ip6.addr_seen;
-		b->ip6h.saddr = b->s_in6.sin6_addr;
+
+		dst = &c->ip6.addr_ll_seen;
+		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+			src = &c->ip6.gw;
+		else
+			src = &c->ip6.addr_ll;
 	}
 
-	b->uh.source = b->s_in6.sin6_port;
+	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
+	b->ip6h.saddr = *src;
+	b->ip6h.daddr = *dst;
+	b->uh.source = htons(src_port);
 	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
 
-- 
@@ -588,18 +588,14 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 			      const struct timespec *now)
 {
 	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
+	const struct in_addr *src = &b->s_in.sin_addr;
 	in_port_t src_port = ntohs(b->s_in.sin_port);
-	struct in_addr *src = &b->s_in.sin_addr;
-
-	b->iph.tot_len = htons(ip_len);
-	b->iph.daddr = c->ip4.addr_seen.s_addr;
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
 	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && src_port == 53) {
-		b->iph.saddr = c->ip4.dns_match.s_addr;
+		src = &c->ip4.dns_match;
 	} else if (IN4_IS_ADDR_LOOPBACK(src) ||
 		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr_seen)) {
-		b->iph.saddr = c->ip4.gw.s_addr;
 		udp_tap_map[V4][src_port].ts = now->tv_sec;
 		udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
 
@@ -609,12 +605,15 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 			udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
 
 		bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
-	} else {
-		b->iph.saddr = src->s_addr;
+
+		src = &c->ip4.gw;
 	}
 
+	b->iph.tot_len = htons(ip_len);
+	b->iph.daddr = c->ip4.addr_seen.s_addr;
+	b->iph.saddr = src->s_addr;
 	udp_update_check4(b);
-	b->uh.source = b->s_in.sin_port;
+	b->uh.source = htons(src_port);
 	b->uh.dest = htons(dstport);
 	b->uh.len = htons(datalen + sizeof(b->uh));
 
@@ -636,29 +635,19 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 			      const struct timespec *now)
 {
 	size_t ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
+	const struct in6_addr *src = &b->s_in6.sin6_addr;
+	const struct in6_addr *dst = &c->ip6.addr_seen;
 	in_port_t src_port = ntohs(b->s_in6.sin6_port);
-	struct in6_addr *src = &b->s_in6.sin6_addr;
-
-	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
 
 	if (IN6_IS_ADDR_LINKLOCAL(src)) {
-		b->ip6h.daddr = c->ip6.addr_ll_seen;
-		b->ip6h.saddr = b->s_in6.sin6_addr;
+		dst = &c->ip6.addr_ll_seen;
 	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) &&
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) &&
 		   src_port == 53) {
-		b->ip6h.daddr = c->ip6.addr_seen;
-		b->ip6h.saddr = c->ip6.dns_match;
+		src = &c->ip6.dns_match;
 	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) {
-		b->ip6h.daddr = c->ip6.addr_ll_seen;
-
-		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
-			b->ip6h.saddr = c->ip6.gw;
-		else
-			b->ip6h.saddr = c->ip6.addr_ll;
-
 		udp_tap_map[V6][src_port].ts = now->tv_sec;
 		udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
 
@@ -673,12 +662,18 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 			udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
 
 		bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
-	} else {
-		b->ip6h.daddr = c->ip6.addr_seen;
-		b->ip6h.saddr = b->s_in6.sin6_addr;
+
+		dst = &c->ip6.addr_ll_seen;
+		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+			src = &c->ip6.gw;
+		else
+			src = &c->ip6.addr_ll;
 	}
 
-	b->uh.source = b->s_in6.sin6_port;
+	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
+	b->ip6h.saddr = *src;
+	b->ip6h.daddr = *dst;
+	b->uh.source = htons(src_port);
 	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
 
-- 
2.44.0

[PATCH 4/6] udp: Consistent port variable names in udp_update_hdr[46]

[David Gibson]

In these functions we have 'dstport' for the destination port, but
'src_port' for the source port.  Change the latter to 'srcport' for
consistency.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/udp.c b/udp.c
index a07c1a62..0cca9531 100644
--- a/udp.c
+++ b/udp.c
@@ -589,22 +589,22 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 {
 	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
 	const struct in_addr *src = &b->s_in.sin_addr;
-	in_port_t src_port = ntohs(b->s_in.sin_port);
+	in_port_t srcport = ntohs(b->s_in.sin_port);
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
-	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && src_port == 53) {
+	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && srcport == 53) {
 		src = &c->ip4.dns_match;
 	} else if (IN4_IS_ADDR_LOOPBACK(src) ||
 		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr_seen)) {
-		udp_tap_map[V4][src_port].ts = now->tv_sec;
-		udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
+		udp_tap_map[V4][srcport].ts = now->tv_sec;
+		udp_tap_map[V4][srcport].flags |= PORT_LOCAL;
 
 		if (IN4_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V4][src_port].flags |= PORT_LOOPBACK;
+			udp_tap_map[V4][srcport].flags |= PORT_LOOPBACK;
 		else
-			udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
+			udp_tap_map[V4][srcport].flags &= ~PORT_LOOPBACK;
 
-		bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
+		bitmap_set(udp_act[V4][UDP_ACT_TAP], srcport);
 
 		src = &c->ip4.gw;
 	}
@@ -613,7 +613,7 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 	b->iph.daddr = c->ip4.addr_seen.s_addr;
 	b->iph.saddr = src->s_addr;
 	udp_update_check4(b);
-	b->uh.source = htons(src_port);
+	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
 	b->uh.len = htons(datalen + sizeof(b->uh));
 
@@ -637,31 +637,31 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 	size_t ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
 	const struct in6_addr *src = &b->s_in6.sin6_addr;
 	const struct in6_addr *dst = &c->ip6.addr_seen;
-	in_port_t src_port = ntohs(b->s_in6.sin6_port);
+	in_port_t srcport = ntohs(b->s_in6.sin6_port);
 
 	if (IN6_IS_ADDR_LINKLOCAL(src)) {
 		dst = &c->ip6.addr_ll_seen;
 	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) &&
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) &&
-		   src_port == 53) {
+		   srcport == 53) {
 		src = &c->ip6.dns_match;
 	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) {
-		udp_tap_map[V6][src_port].ts = now->tv_sec;
-		udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
+		udp_tap_map[V6][srcport].ts = now->tv_sec;
+		udp_tap_map[V6][srcport].flags |= PORT_LOCAL;
 
 		if (IN6_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V6][src_port].flags |= PORT_LOOPBACK;
+			udp_tap_map[V6][srcport].flags |= PORT_LOOPBACK;
 		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_LOOPBACK;
+			udp_tap_map[V6][srcport].flags &= ~PORT_LOOPBACK;
 
 		if (IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr))
-			udp_tap_map[V6][src_port].flags |= PORT_GUA;
+			udp_tap_map[V6][srcport].flags |= PORT_GUA;
 		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
+			udp_tap_map[V6][srcport].flags &= ~PORT_GUA;
 
-		bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
+		bitmap_set(udp_act[V6][UDP_ACT_TAP], srcport);
 
 		dst = &c->ip6.addr_ll_seen;
 		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
@@ -673,7 +673,7 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
 	b->ip6h.saddr = *src;
 	b->ip6h.daddr = *dst;
-	b->uh.source = htons(src_port);
+	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
 
-- 
@@ -589,22 +589,22 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 {
 	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
 	const struct in_addr *src = &b->s_in.sin_addr;
-	in_port_t src_port = ntohs(b->s_in.sin_port);
+	in_port_t srcport = ntohs(b->s_in.sin_port);
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
-	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && src_port == 53) {
+	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && srcport == 53) {
 		src = &c->ip4.dns_match;
 	} else if (IN4_IS_ADDR_LOOPBACK(src) ||
 		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr_seen)) {
-		udp_tap_map[V4][src_port].ts = now->tv_sec;
-		udp_tap_map[V4][src_port].flags |= PORT_LOCAL;
+		udp_tap_map[V4][srcport].ts = now->tv_sec;
+		udp_tap_map[V4][srcport].flags |= PORT_LOCAL;
 
 		if (IN4_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V4][src_port].flags |= PORT_LOOPBACK;
+			udp_tap_map[V4][srcport].flags |= PORT_LOOPBACK;
 		else
-			udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK;
+			udp_tap_map[V4][srcport].flags &= ~PORT_LOOPBACK;
 
-		bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
+		bitmap_set(udp_act[V4][UDP_ACT_TAP], srcport);
 
 		src = &c->ip4.gw;
 	}
@@ -613,7 +613,7 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 	b->iph.daddr = c->ip4.addr_seen.s_addr;
 	b->iph.saddr = src->s_addr;
 	udp_update_check4(b);
-	b->uh.source = htons(src_port);
+	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
 	b->uh.len = htons(datalen + sizeof(b->uh));
 
@@ -637,31 +637,31 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 	size_t ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
 	const struct in6_addr *src = &b->s_in6.sin6_addr;
 	const struct in6_addr *dst = &c->ip6.addr_seen;
-	in_port_t src_port = ntohs(b->s_in6.sin6_port);
+	in_port_t srcport = ntohs(b->s_in6.sin6_port);
 
 	if (IN6_IS_ADDR_LINKLOCAL(src)) {
 		dst = &c->ip6.addr_ll_seen;
 	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) &&
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) &&
-		   src_port == 53) {
+		   srcport == 53) {
 		src = &c->ip6.dns_match;
 	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr)) {
-		udp_tap_map[V6][src_port].ts = now->tv_sec;
-		udp_tap_map[V6][src_port].flags |= PORT_LOCAL;
+		udp_tap_map[V6][srcport].ts = now->tv_sec;
+		udp_tap_map[V6][srcport].flags |= PORT_LOCAL;
 
 		if (IN6_IS_ADDR_LOOPBACK(src))
-			udp_tap_map[V6][src_port].flags |= PORT_LOOPBACK;
+			udp_tap_map[V6][srcport].flags |= PORT_LOOPBACK;
 		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_LOOPBACK;
+			udp_tap_map[V6][srcport].flags &= ~PORT_LOOPBACK;
 
 		if (IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr))
-			udp_tap_map[V6][src_port].flags |= PORT_GUA;
+			udp_tap_map[V6][srcport].flags |= PORT_GUA;
 		else
-			udp_tap_map[V6][src_port].flags &= ~PORT_GUA;
+			udp_tap_map[V6][srcport].flags &= ~PORT_GUA;
 
-		bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
+		bitmap_set(udp_act[V6][UDP_ACT_TAP], srcport);
 
 		dst = &c->ip6.addr_ll_seen;
 		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
@@ -673,7 +673,7 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 	b->ip6h.payload_len = htons(datalen + sizeof(b->uh));
 	b->ip6h.saddr = *src;
 	b->ip6h.daddr = *dst;
-	b->uh.source = htons(src_port);
+	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
 
-- 
2.44.0

[PATCH 5/6] udp: Avoid unnecessary pointer in udp_update_hdr4()

[David Gibson]

We carry around the source address as a pointer to a constant struct
in_addr.  But it's silly to carry around a 4 or 8 byte pointer to a 4 byte
IPv4 address.  Just copy the IPv4 address around by value.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/udp.c b/udp.c
index 0cca9531..a0080b1a 100644
--- a/udp.c
+++ b/udp.c
@@ -588,30 +588,30 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 			      const struct timespec *now)
 {
 	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
-	const struct in_addr *src = &b->s_in.sin_addr;
 	in_port_t srcport = ntohs(b->s_in.sin_port);
+	struct in_addr src = b->s_in.sin_addr;
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
-	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && srcport == 53) {
-		src = &c->ip4.dns_match;
-	} else if (IN4_IS_ADDR_LOOPBACK(src) ||
-		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr_seen)) {
+	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53) {
+		src = c->ip4.dns_match;
+	} else if (IN4_IS_ADDR_LOOPBACK(&src) ||
+		   IN4_ARE_ADDR_EQUAL(&src, &c->ip4.addr_seen)) {
 		udp_tap_map[V4][srcport].ts = now->tv_sec;
 		udp_tap_map[V4][srcport].flags |= PORT_LOCAL;
 
-		if (IN4_IS_ADDR_LOOPBACK(src))
+		if (IN4_IS_ADDR_LOOPBACK(&src))
 			udp_tap_map[V4][srcport].flags |= PORT_LOOPBACK;
 		else
 			udp_tap_map[V4][srcport].flags &= ~PORT_LOOPBACK;
 
 		bitmap_set(udp_act[V4][UDP_ACT_TAP], srcport);
 
-		src = &c->ip4.gw;
+		src = c->ip4.gw;
 	}
 
 	b->iph.tot_len = htons(ip_len);
 	b->iph.daddr = c->ip4.addr_seen.s_addr;
-	b->iph.saddr = src->s_addr;
+	b->iph.saddr = src.s_addr;
 	udp_update_check4(b);
 	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
-- 
@@ -588,30 +588,30 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 			      const struct timespec *now)
 {
 	size_t ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
-	const struct in_addr *src = &b->s_in.sin_addr;
 	in_port_t srcport = ntohs(b->s_in.sin_port);
+	struct in_addr src = b->s_in.sin_addr;
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
-	    IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && srcport == 53) {
-		src = &c->ip4.dns_match;
-	} else if (IN4_IS_ADDR_LOOPBACK(src) ||
-		   IN4_ARE_ADDR_EQUAL(src, &c->ip4.addr_seen)) {
+	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53) {
+		src = c->ip4.dns_match;
+	} else if (IN4_IS_ADDR_LOOPBACK(&src) ||
+		   IN4_ARE_ADDR_EQUAL(&src, &c->ip4.addr_seen)) {
 		udp_tap_map[V4][srcport].ts = now->tv_sec;
 		udp_tap_map[V4][srcport].flags |= PORT_LOCAL;
 
-		if (IN4_IS_ADDR_LOOPBACK(src))
+		if (IN4_IS_ADDR_LOOPBACK(&src))
 			udp_tap_map[V4][srcport].flags |= PORT_LOOPBACK;
 		else
 			udp_tap_map[V4][srcport].flags &= ~PORT_LOOPBACK;
 
 		bitmap_set(udp_act[V4][UDP_ACT_TAP], srcport);
 
-		src = &c->ip4.gw;
+		src = c->ip4.gw;
 	}
 
 	b->iph.tot_len = htons(ip_len);
 	b->iph.daddr = c->ip4.addr_seen.s_addr;
-	b->iph.saddr = src->s_addr;
+	b->iph.saddr = src.s_addr;
 	udp_update_check4(b);
 	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
-- 
2.44.0

[PATCH 6/6] udp: Clean up UDP checksum generation for inbound IPv6 packets

[David Gibson]

Currently we open code the calculation of the UDP checksum, which involves
temporarily mangling the IPv6 header to match the UDP checksum
pseudo-header.  It also assumes that the payload is contiguous with the
headers, which is true for now, but we want to change in future.

We already have a helper which correcly calculates UDP over IPv6 checksums,
which doesn't require temporarily modifying the headers and which handles
a non-contiguous payload, so use it.  It turns out we were already
initializing the IPv6 version, nexthdr and hop_limit fields, even though
we overwrote them for each packet here, so we can just leave those in
place now.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/udp.c b/udp.c
index a0080b1a..ee1481f9 100644
--- a/udp.c
+++ b/udp.c
@@ -676,13 +676,7 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
-
-	b->ip6h.hop_limit = IPPROTO_UDP;
-	b->ip6h.version = b->ip6h.nexthdr = b->uh.check = 0;
-	b->uh.check = csum(&b->ip6h, ip_len, 0);
-	b->ip6h.version = 6;
-	b->ip6h.nexthdr = IPPROTO_UDP;
-	b->ip6h.hop_limit = 255;
+	csum_udp6(&b->uh, src, dst, b->data, datalen);
 
 	return tap_iov_len(c, &b->taph, ip_len);
 }
-- 
@@ -676,13 +676,7 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 	b->uh.source = htons(srcport);
 	b->uh.dest = htons(dstport);
 	b->uh.len = b->ip6h.payload_len;
-
-	b->ip6h.hop_limit = IPPROTO_UDP;
-	b->ip6h.version = b->ip6h.nexthdr = b->uh.check = 0;
-	b->uh.check = csum(&b->ip6h, ip_len, 0);
-	b->ip6h.version = 6;
-	b->ip6h.nexthdr = IPPROTO_UDP;
-	b->ip6h.hop_limit = 255;
+	csum_udp6(&b->uh, src, dst, b->data, datalen);
 
 	return tap_iov_len(c, &b->taph, ip_len);
 }
-- 
2.44.0