From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id E4FB15A027B for ; Mon, 7 Aug 2023 15:46:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1691415994; bh=qKJm7rm10Et8qe75K6Ny+Zks+jcORQkk0KP+fDcrIxA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VSA+PUGOIWond/UnOuFNdu7lZy6rPitgrz3a8moVQU6Z7OjS9JSfZkGu6mO6oHNwm FwWDzOBT5MWmiiJWkKWMIVF3vFlBPo8O+EjJmzEI7RsMVtQ8XtBklA8/yzAvXMWWXf 3DaHji8vOJkd30QyjPA8xdXCro6dnc8xGff79+98= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4RKHff0sFSz4wyP; Mon, 7 Aug 2023 23:46:34 +1000 (AEST) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH 4/9] epoll: Split handling of ICMP and ICMPv6 sockets Date: Mon, 7 Aug 2023 23:46:26 +1000 Message-ID: <20230807134631.1400119-5-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230807134631.1400119-1-david@gibson.dropbear.id.au> References: <20230807134631.1400119-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: 35STRZZEWLJVERWXL6RIHSWCGKT7Y2ON X-Message-ID-Hash: 35STRZZEWLJVERWXL6RIHSWCGKT7Y2ON X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: We have different epoll type values for ICMP and ICMPv6 sockets, but they both call the same handler function, icmp_sock_handler(). However that function does essentially nothing in common for the two cases. So, split it into icmp_sock_handler() and icmpv6_sock_handler() and dispatch them separately from the top level. While we're there remove some parameters that the function was never using anyway. Also move the test for c->no_icmp into the functions, so that all the logic specific to ICMP is within the handler, rather than in the top level dispatch code. Signed-off-by: David Gibson --- icmp.c | 112 ++++++++++++++++++++++++++++++++------------------------ icmp.h | 9 ++--- passt.c | 5 ++- 3 files changed, 70 insertions(+), 56 deletions(-) diff --git a/icmp.c b/icmp.c index a4b6a47..b676a1a 100644 --- a/icmp.c +++ b/icmp.c @@ -60,78 +60,94 @@ static struct icmp_id_sock icmp_id_map[IP_VERSIONS][ICMP_NUM_IDS]; static uint8_t icmp_act[IP_VERSIONS][DIV_ROUND_UP(ICMP_NUM_IDS, 8)]; /** - * icmp_sock_handler() - Handle new data from socket + * icmp_sock_handler() - Handle new data from IPv4 ICMP socket * @c: Execution context * @ref: epoll reference - * @events: epoll events bitmap - * @now: Current timestamp, unused */ -void icmp_sock_handler(const struct ctx *c, union epoll_ref ref, - uint32_t events, const struct timespec *now) +void icmp_sock_handler(const struct ctx *c, union epoll_ref ref) { - union icmp_epoll_ref *iref = &ref.icmp; - struct sockaddr_storage sr; - socklen_t sl = sizeof(sr); char buf[USHRT_MAX]; + struct icmphdr *ih = (struct icmphdr *)buf; + struct sockaddr_in sr; + socklen_t sl = sizeof(sr); uint16_t seq, id; ssize_t n; - (void)events; - (void)now; + if (c->no_icmp) + return; + /* FIXME: Workaround clang-tidy not realizing that recvfrom() + * writes the socket address. See + * https://github.com/llvm/llvm-project/issues/58992 + */ + memset(&sr, 0, sizeof(sr)); n = recvfrom(ref.fd, buf, sizeof(buf), 0, (struct sockaddr *)&sr, &sl); if (n < 0) return; - if (iref->v6) { - struct sockaddr_in6 *sr6 = (struct sockaddr_in6 *)&sr; - struct icmp6hdr *ih = (struct icmp6hdr *)buf; + id = ntohs(ih->un.echo.id); + seq = ntohs(ih->un.echo.sequence); - id = ntohs(ih->icmp6_identifier); - seq = ntohs(ih->icmp6_sequence); + if (id != ref.icmp.id) + ih->un.echo.id = htons(ref.icmp.id); - /* If bind() fails e.g. because of a broken SELinux policy, this - * might happen. Fix up the identifier to match the sent one. - */ - if (id != iref->id) - ih->icmp6_identifier = htons(iref->id); + if (c->mode == MODE_PASTA) { + if (icmp_id_map[V4][id].seq == seq) + return; - /* In PASTA mode, we'll get any reply we send, discard them. */ - if (c->mode == MODE_PASTA) { - if (icmp_id_map[V6][id].seq == seq) - return; - - icmp_id_map[V6][id].seq = seq; - } + icmp_id_map[V4][id].seq = seq; + } - debug("ICMPv6: echo %s to tap, ID: %i, seq: %i", - (ih->icmp6_type == 128) ? "request" : "reply", id, seq); + debug("ICMP: echo %s to tap, ID: %i, seq: %i", + (ih->type == ICMP_ECHO) ? "request" : "reply", id, seq); - 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; + tap_icmp4_send(c, sr.sin_addr, tap_ip4_daddr(c), buf, n); +} - id = ntohs(ih->un.echo.id); - seq = ntohs(ih->un.echo.sequence); +/** + * icmpv6_sock_handler() - Handle new data from ICMPv6 socket + * @c: Execution context + * @ref: epoll reference + */ +void icmpv6_sock_handler(const struct ctx *c, union epoll_ref ref) +{ + char buf[USHRT_MAX]; + struct icmp6hdr *ih = (struct icmp6hdr *)buf; + struct sockaddr_in6 sr; + socklen_t sl = sizeof(sr); + uint16_t seq, id; + ssize_t n; - if (id != iref->id) - ih->un.echo.id = htons(iref->id); + if (c->no_icmp) + return; - if (c->mode == MODE_PASTA) { + n = recvfrom(ref.fd, buf, sizeof(buf), 0, (struct sockaddr *)&sr, &sl); + if (n < 0) + return; - if (icmp_id_map[V4][id].seq == seq) - return; + id = ntohs(ih->icmp6_identifier); + seq = ntohs(ih->icmp6_sequence); - icmp_id_map[V4][id].seq = seq; - } + /* If bind() fails e.g. because of a broken SELinux policy, + * this might happen. Fix up the identifier to match the sent + * one. + */ + if (id != ref.icmp.id) + ih->icmp6_identifier = htons(ref.icmp.id); - debug("ICMP: echo %s to tap, ID: %i, seq: %i", - (ih->type == ICMP_ECHO) ? "request" : "reply", id, seq); + /* In PASTA mode, we'll get any reply we send, discard them. */ + if (c->mode == MODE_PASTA) { + if (icmp_id_map[V6][id].seq == seq) + return; - tap_icmp4_send(c, sr4->sin_addr, tap_ip4_daddr(c), buf, n); + icmp_id_map[V6][id].seq = seq; } + + debug("ICMPv6: echo %s to tap, ID: %i, seq: %i", + (ih->icmp6_type == 128) ? "request" : "reply", id, seq); + + tap_icmp6_send(c, &sr.sin6_addr, + tap_ip6_daddr(c, &sr.sin6_addr), buf, n); } /** @@ -150,11 +166,11 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, size_t plen; if (af == AF_INET) { - union icmp_epoll_ref iref = { .v6 = 0 }; struct sockaddr_in sa = { .sin_family = AF_INET, .sin_addr = { .s_addr = htonl(INADDR_ANY) }, }; + union icmp_epoll_ref iref; struct icmphdr *ih; int id, s; @@ -204,12 +220,12 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, id, ntohs(ih->un.echo.sequence)); } } else if (af == AF_INET6) { - union icmp_epoll_ref iref = { .v6 = 1 }; struct sockaddr_in6 sa = { .sin6_family = AF_INET6, .sin6_addr = IN6ADDR_ANY_INIT, .sin6_scope_id = c->ifi6, }; + union icmp_epoll_ref iref; struct icmp6hdr *ih; int id, s; diff --git a/icmp.h b/icmp.h index 29c7829..32f0c47 100644 --- a/icmp.h +++ b/icmp.h @@ -10,8 +10,8 @@ struct ctx; -void icmp_sock_handler(const struct ctx *c, union epoll_ref ref, - uint32_t events, const struct timespec *now); +void icmp_sock_handler(const struct ctx *c, union epoll_ref ref); +void icmpv6_sock_handler(const struct ctx *c, union epoll_ref ref); int icmp_tap_handler(const struct ctx *c, int af, const void *addr, const struct pool *p, const struct timespec *now); void icmp_timer(const struct ctx *c, const struct timespec *ts); @@ -24,10 +24,7 @@ void icmp_init(void); * @id: Associated echo identifier, needed if bind() fails */ union icmp_epoll_ref { - struct { - uint32_t v6:1, - id:16; - }; + uint16_t id; uint32_t u32; }; diff --git a/passt.c b/passt.c index 32ea2ca..33c2162 100644 --- a/passt.c +++ b/passt.c @@ -329,9 +329,10 @@ loop: udp_sock_handler(&c, ref, eventmask, &now); break; case EPOLL_TYPE_ICMP: + icmp_sock_handler(&c, ref); + break; case EPOLL_TYPE_ICMPV6: - if (!c.no_icmp) - icmp_sock_handler(&c, ref, eventmask, &now); + icmpv6_sock_handler(&c, ref); break; default: /* Can't happen */ -- 2.41.0