On Tue, Oct 18, 2022 at 05:06:11AM +0200, Stefano Brivio wrote:On Mon, 17 Oct 2022 19:58:02 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Makes sense. Realized that comment isn't quite correct, because it was DHCPv6 rather than ICMPv6 traffic that got the flow labels.The IPv4 and IPv6 paths in tap_ip_send() have very little in common, and it turns out that every caller (statically) knows if it is using IPv4 or IPv6. So split into separate tap_ip4_send() and tap_ip6_send() functions. Use a new tap_l2_hdr() function for the very small common part. While we're there, make some minor cleanups: - We were double writing some fields in the IPv6 header, so that it temporary matched the pseudo-header for checksum calculation. With recent checksum reworks, this isn't neccessary any more. - We don't use any IPv4 header options, so use some sizeof() constructs instead of some open coded values for header length. - The comment used to say that the flow label was for TCP over IPv6, but in fact the only thing we used it for was ICMPv6...right, this used to be the data path.Done. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibsonSigned-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- dhcpv6.c | 6 +- icmp.c | 10 +--- tap.c | 176 +++++++++++++++++++++++++++++-------------------------- tap.h | 6 +- 4 files changed, 102 insertions(+), 96 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index e7640ce..7829968 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -531,8 +531,8 @@ int dhcpv6(struct ctx *c, const struct pool *p, resp_not_on_link.hdr.xid = mh->xid; - tap_ip_send(c, src, IPPROTO_UDP, - (char *)&resp_not_on_link, n, mh->xid); + tap_ip6_send(c, src, IPPROTO_UDP, + (char *)&resp_not_on_link, n, mh->xid); return 1; } @@ -580,7 +580,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, resp.hdr.xid = mh->xid; - tap_ip_send(c, src, IPPROTO_UDP, (char *)&resp, n, mh->xid); + tap_ip6_send(c, src, IPPROTO_UDP, (char *)&resp, n, mh->xid); c->ip6.addr_seen = c->ip6.addr; return 1; diff --git a/icmp.c b/icmp.c index 21ea2d7..61c2d90 100644 --- a/icmp.c +++ b/icmp.c @@ -69,10 +69,6 @@ static uint8_t icmp_act[IP_VERSIONS][DIV_ROUND_UP(ICMP_NUM_IDS, 8)]; void icmp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now) { - struct in6_addr a6 = { .s6_addr = { 0, 0, 0, 0, - 0, 0, 0, 0, - 0, 0, 0xff, 0xff, - 0, 0, 0, 0 } }; union icmp_epoll_ref *iref = &ref.r.p.icmp; struct sockaddr_storage sr; socklen_t sl = sizeof(sr); @@ -109,7 +105,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref, icmp_id_map[V6][id].seq = seq; } - tap_ip_send(c, &sr6->sin6_addr, IPPROTO_ICMPV6, buf, n, 0); + tap_ip6_send(c, &sr6->sin6_addr, IPPROTO_ICMPV6, buf, n, 0); } else { struct sockaddr_in *sr4 = (struct sockaddr_in *)&sr; struct icmphdr *ih = (struct icmphdr *)buf; @@ -127,9 +123,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref, icmp_id_map[V4][id].seq = seq; } - memcpy(&a6.s6_addr[12], &sr4->sin_addr, sizeof(sr4->sin_addr)); - - tap_ip_send(c, &a6, IPPROTO_ICMP, buf, n, 0); + tap_ip4_send(c, sr4->sin_addr.s_addr, IPPROTO_ICMP, buf, n); } } diff --git a/tap.c b/tap.c index ae75fac..45547ac 100644 --- a/tap.c +++ b/tap.c @@ -109,100 +109,110 @@ const struct in6_addr *tap_ip6_daddr(const struct ctx *c, } /** - * tap_ip_send() - Send IP packet, with L2 headers, calculating L3/L4 checksums + * tap_l2_hdr() - Build an L2 header for an inbound packet * @c: Execution context - * @src: IPv6 source address, IPv4-mapped for IPv4 sources - * @proto: L4 protocol number - * @in: Payload - * @len: L4 payload length - * @flow: Flow label for TCP over IPv6 + * @buf: Buffer address at which to generate header + * @proto: Ethernet protocol number for L3 + * + * Returns a pointer at which to write the packet's payload* Return: ...