From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v2 4/6] udp: Re-order udp_update_hdr[46] for clarity and brevity
Date: Wed, 6 Mar 2024 16:34:26 +1100 [thread overview]
Message-ID: <20240306053428.1176129-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240306053428.1176129-1-david@gibson.dropbear.id.au>
The order of things in these functions is a bit odd for historical reasons.
We initialise some IP header fields early, the more later after making
some tests. Likewise we declare some variables without initialisation,
but then unconditionally set them to values we could calculate at the
start of the function.
Previous cleanups have removed the reasons for some of these choices, so
reorder for clarity, and where possible move the first assignment into an
initialiser.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
udp.c | 40 ++++++++++++++--------------------------
1 file changed, 14 insertions(+), 26 deletions(-)
diff --git a/udp.c b/udp.c
index e20f537a..d3b9f5f9 100644
--- a/udp.c
+++ b/udp.c
@@ -574,17 +574,9 @@ 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)
{
- const struct in_addr *src;
- in_port_t srcport;
- size_t ip_len;
-
- ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
-
- b->iph.tot_len = htons(ip_len);
- b->iph.daddr = c->ip4.addr_seen.s_addr;
-
- src = &b->s_in.sin_addr;
- srcport = ntohs(b->s_in.sin_port);
+ 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);
if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && srcport == 53) {
@@ -603,10 +595,13 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
src = &c->ip4.gw;
}
- b->iph.saddr = src->s_addr;
+ b->iph.tot_len = htons(ip_len);
+ b->iph.daddr = c->ip4.addr_seen.s_addr;
+ b->iph.saddr = src->s_addr;
b->iph.check = csum_ip4_header(b->iph.tot_len, IPPROTO_UDP,
*src, c->ip4.addr_seen);
+
b->uh.source = b->s_in.sin_port;
b->uh.dest = htons(dstport);
b->uh.len = htons(datalen + sizeof(b->uh));
@@ -628,19 +623,10 @@ 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)
{
- const struct in6_addr *src, *dst;
- uint16_t payload_len;
- in_port_t srcport;
- size_t ip_len;
-
- dst = &c->ip6.addr_seen;
- src = &b->s_in6.sin6_addr;
- srcport = ntohs(b->s_in6.sin6_port);
-
- ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
-
- payload_len = datalen + sizeof(b->uh);
- b->ip6h.payload_len = htons(payload_len);
+ const struct in6_addr *src = &b->s_in6.sin6_addr;
+ const struct in6_addr *dst = &c->ip6.addr_seen;
+ uint16_t payload_len = datalen + sizeof(b->uh);
+ in_port_t srcport = ntohs(b->s_in6.sin6_port);
if (IN6_IS_ADDR_LINKLOCAL(src)) {
dst = &c->ip6.addr_ll_seen;
@@ -674,6 +660,8 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
src = &c->ip6.addr_ll;
}
+
+ b->ip6h.payload_len = htons(payload_len);
b->ip6h.daddr = *dst;
b->ip6h.saddr = *src;
b->ip6h.version = 6;
@@ -688,7 +676,7 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
proto_ipv6_header_psum(payload_len, IPPROTO_UDP,
src, dst));
- return tap_iov_len(c, &b->taph, ip_len);
+ return tap_iov_len(c, &b->taph, payload_len + sizeof(b->ip6h));
}
/**
--
@@ -574,17 +574,9 @@ 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)
{
- const struct in_addr *src;
- in_port_t srcport;
- size_t ip_len;
-
- ip_len = datalen + sizeof(b->iph) + sizeof(b->uh);
-
- b->iph.tot_len = htons(ip_len);
- b->iph.daddr = c->ip4.addr_seen.s_addr;
-
- src = &b->s_in.sin_addr;
- srcport = ntohs(b->s_in.sin_port);
+ 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);
if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
IN4_ARE_ADDR_EQUAL(src, &c->ip4.dns_host) && srcport == 53) {
@@ -603,10 +595,13 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
src = &c->ip4.gw;
}
- b->iph.saddr = src->s_addr;
+ b->iph.tot_len = htons(ip_len);
+ b->iph.daddr = c->ip4.addr_seen.s_addr;
+ b->iph.saddr = src->s_addr;
b->iph.check = csum_ip4_header(b->iph.tot_len, IPPROTO_UDP,
*src, c->ip4.addr_seen);
+
b->uh.source = b->s_in.sin_port;
b->uh.dest = htons(dstport);
b->uh.len = htons(datalen + sizeof(b->uh));
@@ -628,19 +623,10 @@ 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)
{
- const struct in6_addr *src, *dst;
- uint16_t payload_len;
- in_port_t srcport;
- size_t ip_len;
-
- dst = &c->ip6.addr_seen;
- src = &b->s_in6.sin6_addr;
- srcport = ntohs(b->s_in6.sin6_port);
-
- ip_len = datalen + sizeof(b->ip6h) + sizeof(b->uh);
-
- payload_len = datalen + sizeof(b->uh);
- b->ip6h.payload_len = htons(payload_len);
+ const struct in6_addr *src = &b->s_in6.sin6_addr;
+ const struct in6_addr *dst = &c->ip6.addr_seen;
+ uint16_t payload_len = datalen + sizeof(b->uh);
+ in_port_t srcport = ntohs(b->s_in6.sin6_port);
if (IN6_IS_ADDR_LINKLOCAL(src)) {
dst = &c->ip6.addr_ll_seen;
@@ -674,6 +660,8 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
src = &c->ip6.addr_ll;
}
+
+ b->ip6h.payload_len = htons(payload_len);
b->ip6h.daddr = *dst;
b->ip6h.saddr = *src;
b->ip6h.version = 6;
@@ -688,7 +676,7 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
proto_ipv6_header_psum(payload_len, IPPROTO_UDP,
src, dst));
- return tap_iov_len(c, &b->taph, ip_len);
+ return tap_iov_len(c, &b->taph, payload_len + sizeof(b->ip6h));
}
/**
--
2.44.0
next prev parent reply other threads:[~2024-03-06 5:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-06 5:34 [PATCH v2 0/6] udp: Small cleanups David Gibson
2024-03-06 5:34 ` [PATCH v2 1/6] udp: Refactor udp_sock[46]_iov_init() David Gibson
2024-03-06 5:34 ` [PATCH v2 2/6] udp: Consistent port variable names in udp_update_hdr[46] David Gibson
2024-03-06 5:34 ` [PATCH v2 3/6] udp: Pass data length explicitly to to udp_update_hdr[46] David Gibson
2024-03-06 5:34 ` David Gibson [this message]
2024-03-06 5:34 ` [PATCH v2 5/6] udp: Avoid unnecessary pointer in udp_update_hdr4() David Gibson
2024-03-06 5:34 ` [PATCH v2 6/6] udp: Use existing helper for UDP checksum on inbound IPv6 packets David Gibson
2024-03-13 14:21 ` [PATCH v2 0/6] udp: Small cleanups Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240306053428.1176129-5-david@gibson.dropbear.id.au \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).