[PATCH v2 0/6] Improve IPv4 address endian handling and related bug fixes

David Gibson

4 Nov 2022 4 Nov '22

4:10 a.m.

While working on allowing us to use IPv4/IPv6 combined listening sockets, I made several endian mistakes. So, I took a diversion to make it harder to make such mistakes. Along the way I found some small existing endian and other bugs in IPv4 address handling. Changes since v1: * Added extra patch making more safety conversion * Added extra patch making some tangentially related comment fixes David Gibson (6): Correct some missing endian conversions of IPv4 addresses Minor improvements to IPv4 netmask handling Use IPV4_IS_LOOPBACK more widely Use typing to reduce chances of IPv4 endianness errors Use endian-safer typing in struct tap4_l4_t tcp: Correct function comments for address types checksum.c | 7 ++-- checksum.h | 3 +- conf.c | 118 ++++++++++++++++++++++++++++++----------------------- dhcp.c | 15 ++++--- icmp.c | 5 +-- passt.c | 2 +- passt.h | 16 ++++---- pasta.c | 7 +--- tap.c | 49 +++++++++++----------- tap.h | 8 ++-- tcp.c | 60 +++++++++++++-------------- tcp.h | 2 +- udp.c | 30 +++++++------- udp.h | 2 +- util.h | 12 +++++- 15 files changed, 182 insertions(+), 154 deletions(-) -- 2.38.1

Show replies by date

David Gibson

4 Nov 4 Nov

4:10 a.m.

New subject: [PATCH v2 1/6] Correct some missing endian conversions of IPv4 addresses

The INADDR_LOOPBACK constant is in host endianness, and similarly the IN_MULTICAST macro expects a host endian address. However, there are some places in passt where we use those with network endian values. This means that passt will incorrectly allow you to set 127.0.0.1 or a multicast address as the guest address or DNS forwarding address. Add the necessary conversions to correct this. INADDR_ANY and INADDR_BROADCAST logically behave the same way, although because they're palindromes it doesn't have an effect in practice. Change them to be logically correct while we're there, though. Signed-off-by: David Gibson --- conf.c | 26 +++++++++++++------------- icmp.c | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/conf.c b/conf.c index 598c711..769df97 100644 --- a/conf.c +++ b/conf.c @@ -1164,11 +1164,11 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_LOOPBACK(&c->ip6.dns_fwd)) break; - if (c->ip4.dns_fwd == INADDR_ANY && + if (c->ip4.dns_fwd == htonl(INADDR_ANY) && inet_pton(AF_INET, optarg, &c->ip4.dns_fwd) && - c->ip4.dns_fwd != INADDR_ANY && - c->ip4.dns_fwd != INADDR_BROADCAST && - c->ip4.dns_fwd != INADDR_LOOPBACK) + c->ip4.dns_fwd != htonl(INADDR_ANY) && + c->ip4.dns_fwd != htonl(INADDR_BROADCAST) && + c->ip4.dns_fwd != htonl(INADDR_LOOPBACK)) break; err("Invalid DNS forwarding address: %s", optarg); @@ -1363,12 +1363,12 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_MULTICAST(&c->ip6.addr)) break; - if (c->ip4.addr == INADDR_ANY && + if (c->ip4.addr == htonl(INADDR_ANY) && inet_pton(AF_INET, optarg, &c->ip4.addr) && - c->ip4.addr != INADDR_ANY && - c->ip4.addr != INADDR_BROADCAST && - c->ip4.addr != INADDR_LOOPBACK && - !IN_MULTICAST(c->ip4.addr)) + c->ip4.addr != htonl(INADDR_ANY) && + c->ip4.addr != htonl(INADDR_BROADCAST) && + c->ip4.addr != htonl(INADDR_LOOPBACK) && + !IN_MULTICAST(ntohl(c->ip4.addr))) break; err("Invalid address: %s", optarg); @@ -1406,11 +1406,11 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw)) break; - if (c->ip4.gw == INADDR_ANY && + if (c->ip4.gw == htonl(INADDR_ANY) && inet_pton(AF_INET, optarg, &c->ip4.gw) && - c->ip4.gw != INADDR_ANY && - c->ip4.gw != INADDR_BROADCAST && - c->ip4.gw != INADDR_LOOPBACK) + c->ip4.gw != htonl(INADDR_ANY) && + c->ip4.gw != htonl(INADDR_BROADCAST) && + c->ip4.gw != htonl(INADDR_LOOPBACK)) break; err("Invalid gateway address: %s", optarg); diff --git a/icmp.c b/icmp.c index 233acf9..125f314 100644 --- a/icmp.c +++ b/icmp.c @@ -148,7 +148,7 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, union icmp_epoll_ref iref = { .icmp.v6 = 0 }; struct sockaddr_in sa = { .sin_family = AF_INET, - .sin_addr = { .s_addr = INADDR_ANY }, + .sin_addr = { .s_addr = htonl(INADDR_ANY) }, }; struct icmphdr *ih; int id, s; -- 2.38.1

David Gibson

4:10 a.m.

New subject: [PATCH v2 2/6] Minor improvements to IPv4 netmask handling

There are several minor problems with our parsing of IPv4 netmasks (-n). First, we don't reject nonsensical netmasks like 0.255.0.255. Address this structurally by using prefix length instead of netmask as the primary variable, only converting (and validating) when we need to. This has the added benefit of making some things more uniform with the IPv6 path. Second, when the user specifies a prefix length, we truncate the output from strtol() to an integer, which means we would treat -n 4294967320 as valid (equivalent to 24). Fix types to check for this. Signed-off-by: David Gibson --- conf.c | 63 +++++++++++++++++++++++++++++++++++---------------------- dhcp.c | 6 ++++-- passt.h | 4 ++-- pasta.c | 7 ++----- 4 files changed, 47 insertions(+), 33 deletions(-) diff --git a/conf.c b/conf.c index 769df97..6c2a9ad 100644 --- a/conf.c +++ b/conf.c @@ -522,6 +522,31 @@ static int conf_pasta_ns(int *netns_only, char *userns, char *netns, return 0; } +/** conf_ip4_prefix() - Parse an IPv4 prefix length or netmask + * @arg: Netmask in dotted decimal or prefix length + * + * Return: Validated prefix length on success, -1 on failure + */ +static int conf_ip4_prefix(const char *arg) +{ + struct in_addr mask; + unsigned long len; + + if (inet_pton(AF_INET, arg, &mask)) { + in_addr_t hmask = ntohl(mask.s_addr); + len = __builtin_popcount(hmask); + if ((hmask << len) != 0) + return -1; + } else { + errno = 0; + len = strtoul(optarg, NULL, 0); + if (len > 32 || errno) + return -1; + } + + return len; +} + /** * conf_ip4() - Verify or detect IPv4 support, get relevant addresses * @ifi: Host interface to attempt (0 to determine one) @@ -544,22 +569,18 @@ static unsigned int conf_ip4(unsigned int ifi, if (!ip4->gw) nl_route(0, ifi, AF_INET, &ip4->gw); - if (!ip4->addr) { - int mask_len = 0; + if (!ip4->addr) + nl_addr(0, ifi, AF_INET, &ip4->addr, &ip4->prefix_len, NULL); - nl_addr(0, ifi, AF_INET, &ip4->addr, &mask_len, NULL); - ip4->mask = htonl(0xffffffff << (32 - mask_len)); - } - - if (!ip4->mask) { + if (!ip4->prefix_len) { if (IN_CLASSA(ntohl(ip4->addr))) - ip4->mask = htonl(IN_CLASSA_NET); + ip4->prefix_len = (32 - IN_CLASSA_NSHIFT); else if (IN_CLASSB(ntohl(ip4->addr))) - ip4->mask = htonl(IN_CLASSB_NET); + ip4->prefix_len = (32 - IN_CLASSB_NSHIFT); else if (IN_CLASSC(ntohl(ip4->addr))) - ip4->mask = htonl(IN_CLASSC_NET); + ip4->prefix_len = (32 - IN_CLASSC_NSHIFT); else - ip4->mask = 0xffffffff; + ip4->prefix_len = 32; } memcpy(&ip4->addr_seen, &ip4->addr, sizeof(ip4->addr_seen)); @@ -819,11 +840,12 @@ static void conf_print(const struct ctx *c) if (c->ifi4) { if (!c->no_dhcp) { + uint32_t mask = htonl(0xffffffff << c->ip4.prefix_len); info("DHCP:"); info(" assign: %s", inet_ntop(AF_INET, &c->ip4.addr, buf4, sizeof(buf4))); info(" mask: %s", - inet_ntop(AF_INET, &c->ip4.mask, buf4, sizeof(buf4))); + inet_ntop(AF_INET, &mask, buf4, sizeof(buf4))); info(" router: %s", inet_ntop(AF_INET, &c->ip4.gw, buf4, sizeof(buf4))); } @@ -1067,9 +1089,9 @@ void conf(struct ctx *c, int argc, char **argv) struct in6_addr *dns6 = c->ip6.dns; struct fqdn *dnss = c->dns_search; uint32_t *dns4 = c->ip4.dns; - int name, ret, mask, b, i; const char *optstring; unsigned int ifi = 0; + int name, ret, b, i; size_t logsize = 0; uid_t uid; gid_t gid; @@ -1375,18 +1397,11 @@ void conf(struct ctx *c, int argc, char **argv) usage(argv[0]); break; case 'n': - if (inet_pton(AF_INET, optarg, &c->ip4.mask)) - break; - - errno = 0; - mask = strtol(optarg, NULL, 0); - if (mask > 0 && mask <= 32 && !errno) { - c->ip4.mask = htonl(0xffffffff << (32 - mask)); - break; + c->ip4.prefix_len = conf_ip4_prefix(optarg); + if (c->ip4.prefix_len < 0) { + err("Invalid netmask: %s", optarg); + usage(argv[0]); } - - err("Invalid netmask: %s", optarg); - usage(argv[0]); break; case 'M': for (i = 0; i < ETH_ALEN; i++) { diff --git a/dhcp.c b/dhcp.c index d22698a..076e9b6 100644 --- a/dhcp.c +++ b/dhcp.c @@ -268,6 +268,7 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) int dhcp(const struct ctx *c, const struct pool *p) { size_t mlen, len, offset = 0, opt_len, opt_off = 0; + struct in_addr mask; struct ethhdr *eh; struct iphdr *iph; struct udphdr *uh; @@ -334,14 +335,15 @@ int dhcp(const struct ctx *c, const struct pool *p) m->chaddr[3], m->chaddr[4], m->chaddr[5]); m->yiaddr = c->ip4.addr; - memcpy(opts[1].s, &c->ip4.mask, sizeof(c->ip4.mask)); + mask.s_addr = htonl(0xffffffff << c->ip4.prefix_len); + memcpy(opts[1].s, &mask, sizeof(mask)); memcpy(opts[3].s, &c->ip4.gw, sizeof(c->ip4.gw)); memcpy(opts[54].s, &c->ip4.gw, sizeof(c->ip4.gw)); /* If the gateway is not on the assigned subnet, send an option 121 * (Classless Static Routing) adding a dummy route to it. */ - if ((c->ip4.addr & c->ip4.mask) != (c->ip4.gw & c->ip4.mask)) { + if ((c->ip4.addr & mask.s_addr) != (c->ip4.gw & mask.s_addr)) { /* a.b.c.d/32:0.0.0.0, 0:a.b.c.d */ opts[121].slen = 14; opts[121].s[0] = 32; diff --git a/passt.h b/passt.h index 67281db..4cf6078 100644 --- a/passt.h +++ b/passt.h @@ -99,7 +99,7 @@ enum passt_modes { * struct ip4_ctx - IPv4 execution context * @addr: IPv4 address for external, routable interface * @addr_seen: Latest IPv4 address seen as source from tap - * @mask: IPv4 netmask, network order + * @prefixlen: IPv4 prefix length (netmask) * @gw: Default IPv4 gateway, network order * @dns: IPv4 DNS addresses, zero-terminated, network order * @dns_fwd: Address forwarded (UDP) to first IPv4 DNS, network order @@ -107,7 +107,7 @@ enum passt_modes { struct ip4_ctx { uint32_t addr; uint32_t addr_seen; - uint32_t mask; + int prefix_len; uint32_t gw; uint32_t dns[MAXNS + 1]; uint32_t dns_fwd; diff --git a/pasta.c b/pasta.c index ae695e2..db86317 100644 --- a/pasta.c +++ b/pasta.c @@ -249,19 +249,16 @@ void pasta_ns_conf(struct ctx *c) nl_link(1, 1 /* lo */, MAC_ZERO, 1, 0); if (c->pasta_conf_ns) { - int prefix_len; - nl_link(1, c->pasta_ifi, c->mac_guest, 1, c->mtu); if (c->ifi4) { - prefix_len = __builtin_popcount(c->ip4.mask); nl_addr(1, c->pasta_ifi, AF_INET, &c->ip4.addr, - &prefix_len, NULL); + &c->ip4.prefix_len, NULL); nl_route(1, c->pasta_ifi, AF_INET, &c->ip4.gw); } if (c->ifi6) { - prefix_len = 64; + int prefix_len = 64; nl_addr(1, c->pasta_ifi, AF_INET6, &c->ip6.addr, &prefix_len, NULL); nl_route(1, c->pasta_ifi, AF_INET6, &c->ip6.gw); -- 2.38.1

David Gibson

4:10 a.m.

New subject: [PATCH v2 3/6] Use IPV4_IS_LOOPBACK more widely

This macro checks if an IPv4 address is in the loopback network (127.0.0.0/8). There are two places where we open code an identical check, use the macro instead. There are also a number of places we specifically exclude the loopback address (127.0.0.1), but we should actually be excluding anything in the loopback network. Change those sites to use the macro as well. Signed-off-by: David Gibson --- conf.c | 8 ++++---- udp.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/conf.c b/conf.c index 6c2a9ad..c36403d 100644 --- a/conf.c +++ b/conf.c @@ -389,7 +389,7 @@ static void get_dns(struct ctx *c) dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1 && inet_pton(AF_INET, p + 1, dns4)) { /* We can only access local addresses via the gw redirect */ - if (ntohl(*dns4) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET) { + if (IPV4_IS_LOOPBACK(ntohl(*dns4))) { if (c->no_map_gw) { *dns4 = 0; continue; @@ -1190,7 +1190,7 @@ void conf(struct ctx *c, int argc, char **argv) inet_pton(AF_INET, optarg, &c->ip4.dns_fwd) && c->ip4.dns_fwd != htonl(INADDR_ANY) && c->ip4.dns_fwd != htonl(INADDR_BROADCAST) && - c->ip4.dns_fwd != htonl(INADDR_LOOPBACK)) + !IPV4_IS_LOOPBACK(ntohl(c->ip4.dns_fwd))) break; err("Invalid DNS forwarding address: %s", optarg); @@ -1389,7 +1389,7 @@ void conf(struct ctx *c, int argc, char **argv) inet_pton(AF_INET, optarg, &c->ip4.addr) && c->ip4.addr != htonl(INADDR_ANY) && c->ip4.addr != htonl(INADDR_BROADCAST) && - c->ip4.addr != htonl(INADDR_LOOPBACK) && + !IPV4_IS_LOOPBACK(ntohl(c->ip4.addr)) && !IN_MULTICAST(ntohl(c->ip4.addr))) break; @@ -1425,7 +1425,7 @@ void conf(struct ctx *c, int argc, char **argv) inet_pton(AF_INET, optarg, &c->ip4.gw) && c->ip4.gw != htonl(INADDR_ANY) && c->ip4.gw != htonl(INADDR_BROADCAST) && - c->ip4.gw != htonl(INADDR_LOOPBACK)) + !IPV4_IS_LOOPBACK(ntohl(c->ip4.gw))) break; err("Invalid gateway address: %s", optarg); diff --git a/udp.c b/udp.c index 4b201d3..7ce533d 100644 --- a/udp.c +++ b/udp.c @@ -680,7 +680,7 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n, src = ntohl(b->s_in.sin_addr.s_addr); src_port = ntohs(b->s_in.sin_port); - if (src >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET || + if (IPV4_IS_LOOPBACK(src) || src == INADDR_ANY || src == ntohl(c->ip4.addr_seen)) { b->iph.saddr = c->ip4.gw; udp_tap_map[V4][src_port].ts = now->tv_sec; -- 2.38.1

David Gibson

4:10 a.m.

New subject: [PATCH v2 4/6] Use typing to reduce chances of IPv4 endianness errors

We recently corrected some errors handling the endianness of IPv4 addresses. These are very easy errors to make since although we mostly store them in network endianness, we sometimes need to manipulate them in host endianness. To reduce the chances of making such mistakes again, change to always using a (struct in_addr) instead of a bare in_addr_t or uint32_t to store network endian addresses. This makes it harder to accidentally do arithmetic or comparisons on such addresses as if they were host endian. We introduce a number of IN4_IS_ADDR_*() helpers to make it easier to directly work with struct in_addr values. This has the additional benefit of making the IPv4 and IPv6 paths more visually similar. Signed-off-by: David Gibson --- checksum.c | 7 ++++--- checksum.h | 3 ++- conf.c | 55 ++++++++++++++++++++++++++++-------------------------- dhcp.c | 11 +++++++---- icmp.c | 3 +-- passt.c | 2 +- passt.h | 12 ++++++------ tap.c | 18 +++++++++--------- tap.h | 8 ++++---- tcp.c | 48 +++++++++++++++++++++++------------------------ tcp.h | 2 +- udp.c | 30 ++++++++++++++--------------- udp.h | 2 +- util.h | 12 ++++++++++-- 14 files changed, 113 insertions(+), 100 deletions(-) diff --git a/checksum.c b/checksum.c index c59869c..29769d9 100644 --- a/checksum.c +++ b/checksum.c @@ -133,7 +133,8 @@ void csum_ip4_header(struct iphdr *ip4h) * @payload: ICMPv4 packet payload * @len: Length of @payload (not including UDP) */ -void csum_udp4(struct udphdr *udp4hr, in_addr_t saddr, in_addr_t daddr, +void csum_udp4(struct udphdr *udp4hr, + struct in_addr saddr, struct in_addr daddr, const void *payload, size_t len) { /* UDP checksums are optional, so don't bother */ @@ -142,8 +143,8 @@ void csum_udp4(struct udphdr *udp4hr, in_addr_t saddr, in_addr_t daddr, if (UDP4_REAL_CHECKSUMS) { /* UNTESTED: if we did want real UDPv4 checksums, this * is roughly what we'd need */ - uint32_t psum = csum_fold(htonl(saddr)) - + csum_fold(htonl(daddr)) + uint32_t psum = csum_fold(saddr.s_addr) + + csum_fold(daddr.s_addr) + htons(len + sizeof(*udp4hr)) + htons(IPPROTO_UDP); /* Add in partial checksum for the UDP header alone */ diff --git a/checksum.h b/checksum.h index b87b0d6..c6b6fb4 100644 --- a/checksum.h +++ b/checksum.h @@ -14,7 +14,8 @@ uint32_t sum_16b(const void *buf, size_t len); uint16_t csum_fold(uint32_t sum); uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init); void csum_ip4_header(struct iphdr *ip4h); -void csum_udp4(struct udphdr *udp4hr, in_addr_t saddr, in_addr_t daddr, +void csum_udp4(struct udphdr *udp4hr, + struct in_addr saddr, struct in_addr daddr, const void *payload, size_t len); void csum_icmp4(struct icmphdr *ih, const void *payload, size_t len); void csum_udp6(struct udphdr *udp6hr, diff --git a/conf.c b/conf.c index c36403d..b1d694b 100644 --- a/conf.c +++ b/conf.c @@ -358,12 +358,12 @@ static void get_dns(struct ctx *c) int dns4_set, dns6_set, dnss_set, dns_set, fd; struct in6_addr *dns6 = &c->ip6.dns[0]; struct fqdn *s = c->dns_search; - uint32_t *dns4 = &c->ip4.dns[0]; + struct in_addr *dns4 = &c->ip4.dns[0]; struct lineread resolvconf; int line_len; char *line, *p, *end; - dns4_set = !c->ifi4 || !!*dns4; + dns4_set = !c->ifi4 || !IN4_IS_ADDR_UNSPECIFIED(dns4); dns6_set = !c->ifi6 || !IN6_IS_ADDR_UNSPECIFIED(dns6); dnss_set = !!*s->n || c->no_dns_search; dns_set = (dns4_set && dns6_set) || c->no_dns; @@ -389,15 +389,15 @@ static void get_dns(struct ctx *c) dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1 && inet_pton(AF_INET, p + 1, dns4)) { /* We can only access local addresses via the gw redirect */ - if (IPV4_IS_LOOPBACK(ntohl(*dns4))) { + if (IN4_IS_ADDR_LOOPBACK(dns4)) { if (c->no_map_gw) { - *dns4 = 0; + dns4->s_addr = htonl(INADDR_ANY); continue; } *dns4 = c->ip4.gw; } dns4++; - *dns4 = 0; + dns4->s_addr = htonl(INADDR_ANY); } if (!dns6_set && @@ -566,18 +566,19 @@ static unsigned int conf_ip4(unsigned int ifi, return 0; } - if (!ip4->gw) + if (IN4_IS_ADDR_UNSPECIFIED(&ip4->gw)) nl_route(0, ifi, AF_INET, &ip4->gw); - if (!ip4->addr) + if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) nl_addr(0, ifi, AF_INET, &ip4->addr, &ip4->prefix_len, NULL); if (!ip4->prefix_len) { - if (IN_CLASSA(ntohl(ip4->addr))) + in_addr_t addr = ntohl(ip4->addr.s_addr); + if (IN_CLASSA(addr)) ip4->prefix_len = (32 - IN_CLASSA_NSHIFT); - else if (IN_CLASSB(ntohl(ip4->addr))) + else if (IN_CLASSB(addr)) ip4->prefix_len = (32 - IN_CLASSB_NSHIFT); - else if (IN_CLASSC(ntohl(ip4->addr))) + else if (IN_CLASSC(addr)) ip4->prefix_len = (32 - IN_CLASSC_NSHIFT); else ip4->prefix_len = 32; @@ -588,7 +589,9 @@ static unsigned int conf_ip4(unsigned int ifi, if (MAC_IS_ZERO(mac)) nl_link(0, ifi, mac, 0, 0); - if (!ip4->gw || !ip4->addr || MAC_IS_ZERO(mac)) + if (IN4_IS_ADDR_UNSPECIFIED(&ip4->gw) || + IN4_IS_ADDR_UNSPECIFIED(&ip4->addr) || + MAC_IS_ZERO(mac)) return 0; return ifi; @@ -850,7 +853,7 @@ static void conf_print(const struct ctx *c) inet_ntop(AF_INET, &c->ip4.gw, buf4, sizeof(buf4))); } - for (i = 0; c->ip4.dns[i]; i++) { + for (i = 0; !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) { if (!i) info("DNS:"); inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4)); @@ -1088,7 +1091,7 @@ void conf(struct ctx *c, int argc, char **argv) char *runas = NULL, *logfile = NULL; struct in6_addr *dns6 = c->ip6.dns; struct fqdn *dnss = c->dns_search; - uint32_t *dns4 = c->ip4.dns; + struct in_addr *dns4 = c->ip4.dns; const char *optstring; unsigned int ifi = 0; int name, ret, b, i; @@ -1186,11 +1189,11 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_LOOPBACK(&c->ip6.dns_fwd)) break; - if (c->ip4.dns_fwd == htonl(INADDR_ANY) && + if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_fwd) && inet_pton(AF_INET, optarg, &c->ip4.dns_fwd) && - c->ip4.dns_fwd != htonl(INADDR_ANY) && - c->ip4.dns_fwd != htonl(INADDR_BROADCAST) && - !IPV4_IS_LOOPBACK(ntohl(c->ip4.dns_fwd))) + !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_fwd) && + !IN4_IS_ADDR_BROADCAST(&c->ip4.dns_fwd) && + !IN4_IS_ADDR_LOOPBACK(&c->ip4.dns_fwd)) break; err("Invalid DNS forwarding address: %s", optarg); @@ -1385,12 +1388,12 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_MULTICAST(&c->ip6.addr)) break; - if (c->ip4.addr == htonl(INADDR_ANY) && + if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr) && inet_pton(AF_INET, optarg, &c->ip4.addr) && - c->ip4.addr != htonl(INADDR_ANY) && - c->ip4.addr != htonl(INADDR_BROADCAST) && - !IPV4_IS_LOOPBACK(ntohl(c->ip4.addr)) && - !IN_MULTICAST(ntohl(c->ip4.addr))) + !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr) && + !IN4_IS_ADDR_BROADCAST(&c->ip4.addr) && + !IN4_IS_ADDR_LOOPBACK(&c->ip4.addr) && + !IN4_IS_ADDR_MULTICAST(&c->ip4.addr)) break; err("Invalid address: %s", optarg); @@ -1421,11 +1424,11 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw)) break; - if (c->ip4.gw == htonl(INADDR_ANY) && + if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw) && inet_pton(AF_INET, optarg, &c->ip4.gw) && - c->ip4.gw != htonl(INADDR_ANY) && - c->ip4.gw != htonl(INADDR_BROADCAST) && - !IPV4_IS_LOOPBACK(ntohl(c->ip4.gw))) + !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw) && + !IN4_IS_ADDR_BROADCAST(&c->ip4.gw) && + !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw)) break; err("Invalid gateway address: %s", optarg); diff --git a/dhcp.c b/dhcp.c index 076e9b6..0c6f712 100644 --- a/dhcp.c +++ b/dhcp.c @@ -107,7 +107,7 @@ struct msg { uint16_t secs; uint16_t flags; uint32_t ciaddr; - uint32_t yiaddr; + struct in_addr yiaddr; uint32_t siaddr; uint32_t giaddr; uint8_t chaddr[16]; @@ -343,7 +343,8 @@ int dhcp(const struct ctx *c, const struct pool *p) /* If the gateway is not on the assigned subnet, send an option 121 * (Classless Static Routing) adding a dummy route to it. */ - if ((c->ip4.addr & mask.s_addr) != (c->ip4.gw & mask.s_addr)) { + if ((c->ip4.addr.s_addr & mask.s_addr) + != (c->ip4.gw.s_addr & mask.s_addr)) { /* a.b.c.d/32:0.0.0.0, 0:a.b.c.d */ opts[121].slen = 14; opts[121].s[0] = 32; @@ -357,8 +358,10 @@ int dhcp(const struct ctx *c, const struct pool *p) opts[26].s[1] = c->mtu % 256; } - for (i = 0, opts[6].slen = 0; !c->no_dhcp_dns && c->ip4.dns[i]; i++) { - ((uint32_t *)opts[6].s)[i] = c->ip4.dns[i]; + for (i = 0, opts[6].slen = 0; + !c->no_dhcp_dns && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); + i++) { + ((struct in_addr *)opts[6].s)[i] = c->ip4.dns[i]; opts[6].slen += sizeof(uint32_t); } diff --git a/icmp.c b/icmp.c index 125f314..82a6f97 100644 --- a/icmp.c +++ b/icmp.c @@ -124,8 +124,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref, icmp_id_map[V4][id].seq = seq; } - tap_icmp4_send(c, sr4->sin_addr.s_addr, tap_ip4_daddr(c), - buf, n); + tap_icmp4_send(c, sr4->sin_addr, tap_ip4_daddr(c), buf, n); } } diff --git a/passt.c b/passt.c index ff4ee5d..7be998b 100644 --- a/passt.c +++ b/passt.c @@ -136,7 +136,7 @@ static void timer_init(struct ctx *c, const struct timespec *now) * @ip_da: Pointer to IPv4 destination address, NULL if unchanged */ void proto_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, - const uint32_t *ip_da) + const struct in_addr *ip_da) { tcp_update_l2_buf(eth_d, eth_s, ip_da); udp_update_l2_buf(eth_d, eth_s, ip_da); diff --git a/passt.h b/passt.h index 4cf6078..1a8d74b 100644 --- a/passt.h +++ b/passt.h @@ -105,12 +105,12 @@ enum passt_modes { * @dns_fwd: Address forwarded (UDP) to first IPv4 DNS, network order */ struct ip4_ctx { - uint32_t addr; - uint32_t addr_seen; + struct in_addr addr; + struct in_addr addr_seen; int prefix_len; - uint32_t gw; - uint32_t dns[MAXNS + 1]; - uint32_t dns_fwd; + struct in_addr gw; + struct in_addr dns[MAXNS + 1]; + struct in_addr dns_fwd; }; /** @@ -248,6 +248,6 @@ struct ctx { }; void proto_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, - const uint32_t *ip_da); + const struct in_addr *ip_da); #endif /* PASST_H */ diff --git a/tap.c b/tap.c index 3f78c99..bb57568 100644 --- a/tap.c +++ b/tap.c @@ -92,7 +92,7 @@ int tap_send(const struct ctx *c, const void *data, size_t len) * * Returns: IPv4 address, network order */ -in_addr_t tap_ip4_daddr(const struct ctx *c) +struct in_addr tap_ip4_daddr(const struct ctx *c) { return c->ip4.addr_seen; } @@ -141,7 +141,7 @@ static void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto) * * Return: pointer at which to write the packet's payload */ -static void *tap_push_ip4h(char *buf, in_addr_t src, in_addr_t dst, +static void *tap_push_ip4h(char *buf, struct in_addr src, struct in_addr dst, size_t len, uint8_t proto) { struct iphdr *ip4h = (struct iphdr *)buf; @@ -154,8 +154,8 @@ static void *tap_push_ip4h(char *buf, in_addr_t src, in_addr_t dst, ip4h->frag_off = 0; ip4h->ttl = 255; ip4h->protocol = proto; - ip4h->saddr = src; - ip4h->daddr = dst; + ip4h->saddr = src.s_addr; + ip4h->daddr = dst.s_addr; csum_ip4_header(ip4h); return ip4h + 1; } @@ -170,8 +170,8 @@ static void *tap_push_ip4h(char *buf, in_addr_t src, in_addr_t dst, * @in: UDP payload contents (not including UDP header) * @len: UDP payload length (not including UDP header) */ -void tap_udp4_send(const struct ctx *c, in_addr_t src, in_port_t sport, - in_addr_t dst, in_port_t dport, +void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport, + struct in_addr dst, in_port_t dport, const void *in, size_t len) { size_t udplen = len + sizeof(struct udphdr); @@ -199,7 +199,7 @@ void tap_udp4_send(const struct ctx *c, in_addr_t src, in_port_t sport, * @in: ICMP packet, including ICMP header * @len: ICMP packet length, including ICMP header */ -void tap_icmp4_send(const struct ctx *c, in_addr_t src, in_addr_t dst, +void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_addr dst, void *in, size_t len) { char buf[USHRT_MAX]; @@ -448,8 +448,8 @@ resume: l4_len = l3_len - hlen; - if (iph->saddr && c->ip4.addr_seen != iph->saddr) { - c->ip4.addr_seen = iph->saddr; + if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr) { + c->ip4.addr_seen.s_addr = iph->saddr; proto_update_l2_buf(NULL, NULL, &c->ip4.addr_seen); } diff --git a/tap.h b/tap.h index 743bc58..674ab5c 100644 --- a/tap.h +++ b/tap.h @@ -6,11 +6,11 @@ #ifndef TAP_H #define TAP_H -in_addr_t tap_ip4_daddr(const struct ctx *c); -void tap_udp4_send(const struct ctx *c, in_addr_t src, in_port_t sport, - in_addr_t dst, in_port_t dport, +struct in_addr tap_ip4_daddr(const struct ctx *c); +void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport, + struct in_addr dst, in_port_t dport, const void *in, size_t len); -void tap_icmp4_send(const struct ctx *c, in_addr_t src, in_addr_t dst, +void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_addr dst, void *in, size_t len); const struct in6_addr *tap_ip6_daddr(const struct ctx *c, const struct in6_addr *src); diff --git a/tcp.c b/tcp.c index e6979a9..7405ba9 100644 --- a/tcp.c +++ b/tcp.c @@ -1107,7 +1107,7 @@ static void tcp_update_check_tcp6(struct tcp6_l2_buf_t *buf) * @ip_da: Pointer to IPv4 destination address, NULL if unchanged */ void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, - const uint32_t *ip_da) + const struct in_addr *ip_da) { int i; @@ -1134,16 +1134,16 @@ void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, } if (ip_da) { - b4f->iph.daddr = b4->iph.daddr = *ip_da; + b4f->iph.daddr = b4->iph.daddr = ip_da->s_addr; if (!i) { b4f->iph.saddr = b4->iph.saddr = 0; b4f->iph.tot_len = b4->iph.tot_len = 0; b4f->iph.check = b4->iph.check = 0; b4f->psum = b4->psum = sum_16b(&b4->iph, 20); - b4->tsum = ((*ip_da >> 16) & 0xffff) + - (*ip_da & 0xffff) + - htons(IPPROTO_TCP); + b4->tsum = ((ip_da->s_addr >> 16) & 0xffff) + + (ip_da->s_addr & 0xffff) + + htons(IPPROTO_TCP); b4f->tsum = b4->tsum; } else { b4f->psum = b4->psum = tcp4_l2_buf[0].psum; @@ -1701,7 +1701,7 @@ do { \ ip_len = plen + sizeof(struct iphdr) + sizeof(struct tcphdr); b->iph.tot_len = htons(ip_len); b->iph.saddr = conn->a.a4.a.s_addr; - b->iph.daddr = c->ip4.addr_seen; + b->iph.daddr = c->ip4.addr_seen.s_addr; if (check) b->iph.check = *check; @@ -2048,7 +2048,7 @@ static uint32_t tcp_seq_init(const struct ctx *c, int af, const void *addr, } __attribute__((__packed__)) in = { .src = *(struct in_addr *)addr, .srcport = srcport, - .dst = { c->ip4.addr }, + .dst = c->ip4.addr, .dstport = dstport, }; @@ -2176,10 +2176,10 @@ static void tcp_conn_from_tap(struct ctx *c, int af, const void *addr, return; if (!c->no_map_gw) { - if (af == AF_INET && addr4.sin_addr.s_addr == c->ip4.gw) - addr4.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + if (af == AF_INET && IN4_ARE_ADDR_EQUAL(addr, &c->ip4.gw)) + addr4.sin_addr.s_addr = htonl(INADDR_LOOPBACK); if (af == AF_INET6 && IN6_ARE_ADDR_EQUAL(addr, &c->ip6.gw)) - addr6.sin6_addr = in6addr_loopback; + addr6.sin6_addr = in6addr_loopback; } if (af == AF_INET6 && IN6_IS_ADDR_LINKLOCAL(&addr6.sin6_addr)) { @@ -2899,30 +2899,28 @@ static void tcp_conn_from_sock(struct ctx *c, union epoll_ref ref, tcp_hash_insert(c, conn, AF_INET6, &sa6.sin6_addr); } else { struct sockaddr_in sa4; - in_addr_t s_addr; memcpy(&sa4, &sa, sizeof(sa4)); - s_addr = ntohl(sa4.sin_addr.s_addr); memset(&conn->a.a4.zero, 0, sizeof(conn->a.a4.zero)); memset(&conn->a.a4.one, 0xff, sizeof(conn->a.a4.one)); - if (IPV4_IS_LOOPBACK(s_addr) || s_addr == INADDR_ANY || - htonl(s_addr) == c->ip4.addr_seen) - s_addr = ntohl(c->ip4.gw); + if (IN4_IS_ADDR_LOOPBACK(&sa4.sin_addr) || + IN4_IS_ADDR_UNSPECIFIED(&sa4.sin_addr) || + IN4_ARE_ADDR_EQUAL(&sa4.sin_addr, &c->ip4.addr_seen)) + sa4.sin_addr = c->ip4.gw; - s_addr = htonl(s_addr); - memcpy(&conn->a.a4.a, &s_addr, sizeof(conn->a.a4.a)); + conn->a.a4.a = sa4.sin_addr; conn->sock_port = ntohs(sa4.sin_port); conn->tap_port = ref.r.p.tcp.tcp.index; - conn->seq_to_tap = tcp_seq_init(c, AF_INET, &s_addr, + conn->seq_to_tap = tcp_seq_init(c, AF_INET, &sa4.sin_addr, conn->sock_port, conn->tap_port, now); - tcp_hash_insert(c, conn, AF_INET, &s_addr); + tcp_hash_insert(c, conn, AF_INET, &sa4.sin_addr); } conn->seq_ack_from_tap = conn->seq_to_tap + 1; @@ -3081,7 +3079,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order */ -static void tcp_sock_init4(const struct ctx *c, int ns, const in_addr_t *addr, +static void tcp_sock_init4(const struct ctx *c, int ns, const struct in_addr *addr, const char *ifname, in_port_t port) { union tcp_epoll_ref tref = { .tcp.listen = 1, .tcp.outbound = ns }; @@ -3089,14 +3087,13 @@ static void tcp_sock_init4(const struct ctx *c, int ns, const in_addr_t *addr, int s; if (c->mode == MODE_PASTA) { - spliced = !addr || - ntohl(*addr) == INADDR_ANY || - IPV4_IS_LOOPBACK(ntohl(*addr)); + spliced = !addr || IN4_IS_ADDR_UNSPECIFIED(addr) || + IN4_IS_ADDR_LOOPBACK(addr); if (!addr) addr = &c->ip4.addr; - tap = !ns && !IPV4_IS_LOOPBACK(ntohl(*addr)); + tap = !ns && !IN4_IS_ADDR_LOOPBACK(addr); } if (ns) @@ -3117,9 +3114,10 @@ static void tcp_sock_init4(const struct ctx *c, int ns, const in_addr_t *addr, } if (spliced) { + struct in_addr loopback = { htonl(INADDR_LOOPBACK) }; tref.tcp.splice = 1; - addr = &(uint32_t){ htonl(INADDR_LOOPBACK) }; + addr = &loopback; s = sock_l4(c, AF_INET, IPPROTO_TCP, addr, ifname, port, tref.u32); diff --git a/tcp.h b/tcp.h index 72e7815..3fabb5a 100644 --- a/tcp.h +++ b/tcp.h @@ -28,7 +28,7 @@ void tcp_defer_handler(struct ctx *c); void tcp_sock_set_bufsize(const struct ctx *c, int s); void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, - const uint32_t *ip_da); + const struct in_addr *ip_da); /** * union tcp_epoll_ref - epoll reference portion for TCP connections diff --git a/udp.c b/udp.c index 7ce533d..fca418d 100644 --- a/udp.c +++ b/udp.c @@ -298,7 +298,7 @@ static void udp_update_check4(struct udp4_l2_buf_t *buf) * @ip_da: Pointer to IPv4 destination address, NULL if unchanged */ void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, - const uint32_t *ip_da) + const struct in_addr *ip_da) { int i; @@ -317,7 +317,7 @@ void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, } if (ip_da) { - b4->iph.daddr = *ip_da; + b4->iph.daddr = ip_da->s_addr; if (!i) { b4->iph.saddr = 0; b4->iph.tot_len = 0; @@ -671,30 +671,30 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n, struct udp4_l2_buf_t *b = &udp4_l2_buf[n]; size_t ip_len, buf_len; in_port_t src_port; - in_addr_t src; ip_len = udp4_l2_mh_sock[n].msg_len + sizeof(b->iph) + sizeof(b->uh); b->iph.tot_len = htons(ip_len); - src = ntohl(b->s_in.sin_addr.s_addr); src_port = ntohs(b->s_in.sin_port); - if (IPV4_IS_LOOPBACK(src) || - src == INADDR_ANY || src == ntohl(c->ip4.addr_seen)) { - b->iph.saddr = c->ip4.gw; + if (IN4_IS_ADDR_LOOPBACK(&b->s_in.sin_addr) || + IN4_IS_ADDR_UNSPECIFIED(&b->s_in.sin_addr)|| + IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr, &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; - if (b->s_in.sin_addr.s_addr == c->ip4.addr_seen) + if (IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr.s_addr, &c->ip4.addr_seen)) udp_tap_map[V4][src_port].flags &= ~PORT_LOOPBACK; else udp_tap_map[V4][src_port].flags |= PORT_LOOPBACK; bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port); - } else if (c->ip4.dns_fwd && - src == htonl(c->ip4.dns[0]) && src_port == 53) { - b->iph.saddr = c->ip4.dns_fwd; + } else if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_fwd) && + IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr, &c->ip4.dns[0]) && + src_port == 53) { + b->iph.saddr = c->ip4.dns_fwd.s_addr; } else { b->iph.saddr = b->s_in.sin_addr.s_addr; } @@ -1016,15 +1016,15 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, udp_tap_map[V4][src].ts = now->tv_sec; - if (s_in.sin_addr.s_addr == c->ip4.gw && !c->no_map_gw) { + if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) && !c->no_map_gw) { if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) || (udp_tap_map[V4][dst].flags & PORT_LOOPBACK)) s_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK); else - s_in.sin_addr.s_addr = c->ip4.addr_seen; - } else if (s_in.sin_addr.s_addr == c->ip4.dns_fwd && + s_in.sin_addr = c->ip4.addr_seen; + } else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_fwd) && ntohs(s_in.sin_port) == 53) { - s_in.sin_addr.s_addr = c->ip4.dns[0]; + s_in.sin_addr = c->ip4.dns[0]; } } else { s_in6 = (struct sockaddr_in6) { diff --git a/udp.h b/udp.h index b4ee8b7..2ac8610 100644 --- a/udp.h +++ b/udp.h @@ -17,7 +17,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, int udp_init(struct ctx *c); void udp_timer(struct ctx *c, const struct timespec *ts); void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, - const uint32_t *ip_da); + const struct in_addr *ip_da); /** * union udp_epoll_ref - epoll reference portion for TCP connections diff --git a/util.h b/util.h index c498a80..2d4e1ff 100644 --- a/util.h +++ b/util.h @@ -69,8 +69,16 @@ #define MAC_ZERO ((uint8_t [ETH_ALEN]){ 0 }) #define MAC_IS_ZERO(addr) (!memcmp((addr), MAC_ZERO, ETH_ALEN)) -#define IPV4_IS_LOOPBACK(addr) \ - ((addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET) +#define IN4_IS_ADDR_UNSPECIFIED(a) \ + ((a)->s_addr == htonl(INADDR_ANY)) +#define IN4_IS_ADDR_BROADCAST(a) \ + ((a)->s_addr == htonl(INADDR_BROADCAST)) +#define IN4_IS_ADDR_LOOPBACK(a) \ + (ntohl((a)->s_addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET) +#define IN4_IS_ADDR_MULTICAST(a) \ + (IN_MULTICAST(ntohl((a)->s_addr))) +#define IN4_ARE_ADDR_EQUAL(a, b) \ + (((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr) #define NS_FN_STACK_SIZE (RLIMIT_STACK_VAL * 1024 / 8) #define NS_CALL(fn, arg) \ -- 2.38.1

David Gibson

4:10 a.m.

New subject: [PATCH v2 5/6] Use endian-safer typing in struct tap4_l4_t

We recently converted to using struct in_addr rather than bare in_addr_t or uint32_t to represent IPv4 addresses in network order. This makes it harder forget to apply the correct endian conversions. We omitted the IPv4 addresses stored in struct tap4_l4_t, however. Convert those as well. Signed-off-by: David Gibson --- tap.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tap.c b/tap.c index bb57568..e9cfab2 100644 --- a/tap.c +++ b/tap.c @@ -320,8 +320,8 @@ static struct tap4_l4_t { uint16_t source; uint16_t dest; - uint32_t saddr; - uint32_t daddr; + struct in_addr saddr; + struct in_addr daddr; struct pool_l4_t p; } tap4_l4[TAP_SEQS /* Arbitrary: TAP_MSGS in theory, so limit in users */]; @@ -367,14 +367,15 @@ static void tap_packet_debug(const struct iphdr *iph, uint8_t proto = 0; if (iph || seq4) { - inet_ntop(AF_INET, iph ? &iph->saddr : &seq4->saddr, - buf4s, sizeof(buf4s)); - inet_ntop(AF_INET, iph ? &iph->daddr : &seq4->daddr, - buf4d, sizeof(buf4d)); - if (iph) + if (iph) { + inet_ntop(AF_INET, &iph->saddr, buf4s, sizeof(buf4s)); + inet_ntop(AF_INET, &iph->daddr, buf4d, sizeof(buf4d)); proto = iph->protocol; - else if (seq4) + } else { + inet_ntop(AF_INET, &seq4->saddr, buf4s, sizeof(buf4s)); + inet_ntop(AF_INET, &seq4->daddr, buf4d, sizeof(buf4d)); proto = seq4->protocol; + } } else { inet_ntop(AF_INET6, ip6h ? &ip6h->saddr : &seq6->saddr, buf6s, sizeof(buf6s)); @@ -489,15 +490,15 @@ resume: #define L4_MATCH(iph, uh, seq) \ (seq->protocol == iph->protocol && \ seq->source == uh->source && seq->dest == uh->dest && \ - seq->saddr == iph->saddr && seq->daddr == iph->daddr) + seq->saddr.s_addr == iph->saddr && seq->daddr.s_addr == iph->daddr) #define L4_SET(iph, uh, seq) \ do { \ - seq->protocol = iph->protocol; \ - seq->source = uh->source; \ - seq->dest = uh->dest; \ - seq->saddr = iph->saddr; \ - seq->daddr = iph->daddr; \ + seq->protocol = iph->protocol; \ + seq->source = uh->source; \ + seq->dest = uh->dest; \ + seq->saddr.s_addr = iph->saddr; \ + seq->daddr.s_addr = iph->daddr; \ } while (0) if (seq && L4_MATCH(iph, uh, seq) && seq->p.count < TAP_SEQS) @@ -529,7 +530,7 @@ append: for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) { struct pool *p = (struct pool *)&seq->p; - uint32_t *da = &seq->daddr; + struct in_addr *da = &seq->daddr; size_t n = p->count; tap_packet_debug(NULL, NULL, seq, 0, NULL, n); -- 2.38.1

David Gibson

4:10 a.m.

New subject: [PATCH v2 6/6] tcp: Correct function comments for address types

A number of functions describe themselves as taking a pointer to 'sin_addr or sin6_addr'. Those are field names, not type names. Replace them with the correct type names, in_addr or in6_addr. Signed-off-by: David Gibson --- tcp.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tcp.c b/tcp.c index 7405ba9..713248f 100644 --- a/tcp.c +++ b/tcp.c @@ -1277,7 +1277,7 @@ static int tcp_opt_get(const char *opts, size_t len, uint8_t type_find, * tcp_hash_match() - Check if a connection entry matches address and ports * @conn: Connection entry to match against * @af: Address family, AF_INET or AF_INET6 - * @addr: Remote address, pointer to sin_addr or sin6_addr + * @addr: Remote address, pointer to in_addr or in6_addr * @tap_port: tap-facing port * @sock_port: Socket-facing port * @@ -1303,7 +1303,7 @@ static int tcp_hash_match(const struct tcp_conn *conn, int af, const void *addr, * tcp_hash() - Calculate hash value for connection given address and ports * @c: Execution context * @af: Address family, AF_INET or AF_INET6 - * @addr: Remote address, pointer to sin_addr or sin6_addr + * @addr: Remote address, pointer to in_addr or in6_addr * @tap_port: tap-facing port * @sock_port: Socket-facing port * @@ -1347,7 +1347,7 @@ static unsigned int tcp_hash(const struct ctx *c, int af, const void *addr, * @c: Execution context * @conn: Connection pointer * @af: Address family, AF_INET or AF_INET6 - * @addr: Remote address, pointer to sin_addr or sin6_addr + * @addr: Remote address, pointer to in_addr or in6_addr */ static void tcp_hash_insert(const struct ctx *c, struct tcp_conn *conn, int af, const void *addr) @@ -1418,7 +1418,7 @@ static void tcp_hash_update(struct tcp_conn *old, struct tcp_conn *new) * tcp_hash_lookup() - Look up connection given remote address and ports * @c: Execution context * @af: Address family, AF_INET or AF_INET6 - * @addr: Remote address, pointer to sin_addr or sin6_addr + * @addr: Remote address, pointer to in_addr or in6_addr * @tap_port: tap-facing port * @sock_port: Socket-facing port * @@ -2026,7 +2026,7 @@ static void tcp_clamp_window(const struct ctx *c, struct tcp_conn *conn, * tcp_seq_init() - Calculate initial sequence number according to RFC 6528 * @c: Execution context * @af: Address family, AF_INET or AF_INET6 - * @addr: Remote address, pointer to sin_addr or sin6_addr + * @addr: Remote address, pointer to in_addr or in6_addr * @dstport: Destination port, connection-wise, network order * @srcport: Source port, connection-wise, network order * @now: Current timestamp @@ -2144,7 +2144,7 @@ static uint16_t tcp_conn_tap_mss(const struct ctx *c, * tcp_conn_from_tap() - Handle connection request (SYN segment) from tap * @c: Execution context * @af: Address family, AF_INET or AF_INET6 - * @addr: Remote address, pointer to sin_addr or sin6_addr + * @addr: Remote address, pointer to in_addr or in6_addr * @th: TCP header from tap: caller MUST ensure it's there * @opts: Pointer to start of options * @optlen: Bytes in options: caller MUST ensure available length -- 2.38.1

922

Age (days ago)

922

Last active (days ago)

List overview

Download

6 comments

1 participants

participants (1)

David Gibson