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 08/12] icmp: Share more between IPv4 and IPv6 paths in icmp_tap_handler()
Date: Thu, 21 Dec 2023 17:53:23 +1100 [thread overview]
Message-ID: <20231221065327.1307827-9-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20231221065327.1307827-1-david@gibson.dropbear.id.au>
Currently icmp_tap_handler() consists of two almost disjoint paths for the
IPv4 and IPv6 cases. The only thing they share is an error message.
We can use some intermediate variables to refactor this to share some more
code between those paths.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
icmp.c | 140 ++++++++++++++++++++++++++++-----------------------------
1 file changed, 69 insertions(+), 71 deletions(-)
diff --git a/icmp.c b/icmp.c
index 02739b9..f6c744a 100644
--- a/icmp.c
+++ b/icmp.c
@@ -154,102 +154,100 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
const void *saddr, const void *daddr,
const struct pool *p, const struct timespec *now)
{
+ uint8_t proto = af == AF_INET ? IPPROTO_ICMP : IPPROTO_ICMPV6;
+ const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
+ union {
+ struct sockaddr sa;
+ struct sockaddr_in sa4;
+ struct sockaddr_in6 sa6;
+ } sa = { .sa.sa_family = af };
+ const socklen_t sl = af == AF_INET ? sizeof(sa.sa4) : sizeof(sa.sa6);
+ struct icmp_id_sock *id_map;
+ uint16_t id, seq;
size_t plen;
+ void *pkt;
+ int s;
(void)saddr;
(void)pif;
+ pkt = packet_get(p, 0, 0, 0, &plen);
+ if (!pkt)
+ return 1;
+
if (af == AF_INET) {
- struct sockaddr_in sa = {
- .sin_family = AF_INET,
- };
- union icmp_epoll_ref iref;
- struct icmphdr *ih;
- int id, s;
-
- ih = packet_get(p, 0, 0, sizeof(*ih), &plen);
- if (!ih)
+ struct icmphdr *ih = (struct icmphdr *)pkt;
+
+ if (plen < sizeof(*ih))
return 1;
if (ih->type != ICMP_ECHO)
return 1;
- iref.id = id = ntohs(ih->un.echo.id);
+ id = ntohs(ih->un.echo.id);
+ id_map = &icmp_id_map[V4][id];
+ seq = ntohs(ih->un.echo.sequence);
+ sa.sa4.sin_addr = *(struct in_addr *)daddr;
+ } else if (af == AF_INET6) {
+ struct icmp6hdr *ih = (struct icmp6hdr *)pkt;
- if ((s = icmp_id_map[V4][id].sock) < 0) {
- s = sock_l4(c, AF_INET, IPPROTO_ICMP, &c->ip4.addr_out,
- c->ip4.ifname_out, 0, iref.u32);
- if (s < 0)
- goto fail_sock;
- if (s > FD_REF_MAX) {
- close(s);
- return 1;
- }
+ if (plen < sizeof(*ih))
+ return 1;
- icmp_id_map[V4][id].sock = s;
+ if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
+ return 1;
- debug("ICMP: new socket %i for echo ID %i", s, id);
- }
- icmp_id_map[V4][id].ts = now->tv_sec;
+ id = ntohs(ih->icmp6_identifier);
+ id_map = &icmp_id_map[V6][id];
+ seq = ntohs(ih->icmp6_sequence);
+ sa.sa6.sin6_addr = *(struct in6_addr *)daddr;
+ sa.sa6.sin6_scope_id = c->ifi6;
+ } else {
+ ASSERT(0);
+ }
+
+ if ((s = id_map->sock) < 0) {
+ union icmp_epoll_ref iref = { .id = id };
+ const void *bind_addr;
+ const char *bind_if;
- sa.sin_addr = *(struct in_addr *)daddr;
- if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL,
- (struct sockaddr *)&sa, sizeof(sa)) < 0) {
- debug("ICMP: failed to relay request to socket");
+ if (af == AF_INET) {
+ bind_addr = &c->ip4.addr_out;
+ bind_if = c->ip4.ifname_out;
} else {
- debug("ICMP: echo request to socket, ID: %i, seq: %i",
- id, ntohs(ih->un.echo.sequence));
+ bind_addr = &c->ip6.addr_out;
+ bind_if = c->ip6.ifname_out;
}
- } else if (af == AF_INET6) {
- struct sockaddr_in6 sa = {
- .sin6_family = AF_INET6,
- .sin6_scope_id = c->ifi6,
- };
- union icmp_epoll_ref iref;
- struct icmp6hdr *ih;
- int id, s;
-
- ih = packet_get(p, 0, 0, sizeof(struct icmp6hdr), &plen);
- if (!ih)
- return 1;
- if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
- return 1;
+ s = sock_l4(c, af, proto, bind_addr, bind_if, 0, iref.u32);
- iref.id = id = ntohs(ih->icmp6_identifier);
- if ((s = icmp_id_map[V6][id].sock) < 0) {
- s = sock_l4(c, AF_INET6, IPPROTO_ICMPV6,
- &c->ip6.addr_out,
- c->ip6.ifname_out, 0, iref.u32);
- if (s < 0)
- goto fail_sock;
- if (s > FD_REF_MAX) {
- close(s);
- return 1;
- }
-
- icmp_id_map[V6][id].sock = s;
-
- debug("ICMPv6: new socket %i for echo ID %i", s, id);
+ if (s < 0) {
+ warn("Cannot open \"ping\" socket. You might need to:");
+ warn(" sysctl -w net.ipv4.ping_group_range=\"0 2147483647\"");
+ warn("...echo requests/replies will fail.");
+ return 1;
}
- icmp_id_map[V6][id].ts = now->tv_sec;
- sa.sin6_addr = *(struct in6_addr *)daddr;
- if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL,
- (struct sockaddr *)&sa, sizeof(sa)) < 1) {
- debug("ICMPv6: failed to relay request to socket");
- } else {
- debug("ICMPv6: echo request to socket, ID: %i, seq: %i",
- id, ntohs(ih->icmp6_sequence));
+ if (s > FD_REF_MAX) {
+ close(s);
+ return 1;
}
+
+ id_map->sock = s;
+
+ debug("%s: new socket %i for echo ID %"PRIu16, pname, s, id);
}
- return 1;
+ id_map->ts = now->tv_sec;
+
+ if (sendto(s, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
+ debug("%s: failed to relay request to socket: %s",
+ pname, strerror(errno));
+ } else {
+ debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
+ pname, id, seq);
+ }
-fail_sock:
- warn("Cannot open \"ping\" socket. You might need to:");
- warn(" sysctl -w net.ipv4.ping_group_range=\"0 2147483647\"");
- warn("...echo requests/replies will fail.");
return 1;
}
--
@@ -154,102 +154,100 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
const void *saddr, const void *daddr,
const struct pool *p, const struct timespec *now)
{
+ uint8_t proto = af == AF_INET ? IPPROTO_ICMP : IPPROTO_ICMPV6;
+ const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
+ union {
+ struct sockaddr sa;
+ struct sockaddr_in sa4;
+ struct sockaddr_in6 sa6;
+ } sa = { .sa.sa_family = af };
+ const socklen_t sl = af == AF_INET ? sizeof(sa.sa4) : sizeof(sa.sa6);
+ struct icmp_id_sock *id_map;
+ uint16_t id, seq;
size_t plen;
+ void *pkt;
+ int s;
(void)saddr;
(void)pif;
+ pkt = packet_get(p, 0, 0, 0, &plen);
+ if (!pkt)
+ return 1;
+
if (af == AF_INET) {
- struct sockaddr_in sa = {
- .sin_family = AF_INET,
- };
- union icmp_epoll_ref iref;
- struct icmphdr *ih;
- int id, s;
-
- ih = packet_get(p, 0, 0, sizeof(*ih), &plen);
- if (!ih)
+ struct icmphdr *ih = (struct icmphdr *)pkt;
+
+ if (plen < sizeof(*ih))
return 1;
if (ih->type != ICMP_ECHO)
return 1;
- iref.id = id = ntohs(ih->un.echo.id);
+ id = ntohs(ih->un.echo.id);
+ id_map = &icmp_id_map[V4][id];
+ seq = ntohs(ih->un.echo.sequence);
+ sa.sa4.sin_addr = *(struct in_addr *)daddr;
+ } else if (af == AF_INET6) {
+ struct icmp6hdr *ih = (struct icmp6hdr *)pkt;
- if ((s = icmp_id_map[V4][id].sock) < 0) {
- s = sock_l4(c, AF_INET, IPPROTO_ICMP, &c->ip4.addr_out,
- c->ip4.ifname_out, 0, iref.u32);
- if (s < 0)
- goto fail_sock;
- if (s > FD_REF_MAX) {
- close(s);
- return 1;
- }
+ if (plen < sizeof(*ih))
+ return 1;
- icmp_id_map[V4][id].sock = s;
+ if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
+ return 1;
- debug("ICMP: new socket %i for echo ID %i", s, id);
- }
- icmp_id_map[V4][id].ts = now->tv_sec;
+ id = ntohs(ih->icmp6_identifier);
+ id_map = &icmp_id_map[V6][id];
+ seq = ntohs(ih->icmp6_sequence);
+ sa.sa6.sin6_addr = *(struct in6_addr *)daddr;
+ sa.sa6.sin6_scope_id = c->ifi6;
+ } else {
+ ASSERT(0);
+ }
+
+ if ((s = id_map->sock) < 0) {
+ union icmp_epoll_ref iref = { .id = id };
+ const void *bind_addr;
+ const char *bind_if;
- sa.sin_addr = *(struct in_addr *)daddr;
- if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL,
- (struct sockaddr *)&sa, sizeof(sa)) < 0) {
- debug("ICMP: failed to relay request to socket");
+ if (af == AF_INET) {
+ bind_addr = &c->ip4.addr_out;
+ bind_if = c->ip4.ifname_out;
} else {
- debug("ICMP: echo request to socket, ID: %i, seq: %i",
- id, ntohs(ih->un.echo.sequence));
+ bind_addr = &c->ip6.addr_out;
+ bind_if = c->ip6.ifname_out;
}
- } else if (af == AF_INET6) {
- struct sockaddr_in6 sa = {
- .sin6_family = AF_INET6,
- .sin6_scope_id = c->ifi6,
- };
- union icmp_epoll_ref iref;
- struct icmp6hdr *ih;
- int id, s;
-
- ih = packet_get(p, 0, 0, sizeof(struct icmp6hdr), &plen);
- if (!ih)
- return 1;
- if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
- return 1;
+ s = sock_l4(c, af, proto, bind_addr, bind_if, 0, iref.u32);
- iref.id = id = ntohs(ih->icmp6_identifier);
- if ((s = icmp_id_map[V6][id].sock) < 0) {
- s = sock_l4(c, AF_INET6, IPPROTO_ICMPV6,
- &c->ip6.addr_out,
- c->ip6.ifname_out, 0, iref.u32);
- if (s < 0)
- goto fail_sock;
- if (s > FD_REF_MAX) {
- close(s);
- return 1;
- }
-
- icmp_id_map[V6][id].sock = s;
-
- debug("ICMPv6: new socket %i for echo ID %i", s, id);
+ if (s < 0) {
+ warn("Cannot open \"ping\" socket. You might need to:");
+ warn(" sysctl -w net.ipv4.ping_group_range=\"0 2147483647\"");
+ warn("...echo requests/replies will fail.");
+ return 1;
}
- icmp_id_map[V6][id].ts = now->tv_sec;
- sa.sin6_addr = *(struct in6_addr *)daddr;
- if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL,
- (struct sockaddr *)&sa, sizeof(sa)) < 1) {
- debug("ICMPv6: failed to relay request to socket");
- } else {
- debug("ICMPv6: echo request to socket, ID: %i, seq: %i",
- id, ntohs(ih->icmp6_sequence));
+ if (s > FD_REF_MAX) {
+ close(s);
+ return 1;
}
+
+ id_map->sock = s;
+
+ debug("%s: new socket %i for echo ID %"PRIu16, pname, s, id);
}
- return 1;
+ id_map->ts = now->tv_sec;
+
+ if (sendto(s, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
+ debug("%s: failed to relay request to socket: %s",
+ pname, strerror(errno));
+ } else {
+ debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
+ pname, id, seq);
+ }
-fail_sock:
- warn("Cannot open \"ping\" socket. You might need to:");
- warn(" sysctl -w net.ipv4.ping_group_range=\"0 2147483647\"");
- warn("...echo requests/replies will fail.");
return 1;
}
--
2.43.0
next prev parent reply other threads:[~2023-12-21 6:53 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-21 6:53 [PATCH v2 00/12] RFC: ICMP reworks preliminary to flow table integration David Gibson
2023-12-21 6:53 ` [PATCH v2 01/12] checksum: Don't use linux/icmp.h when netinet/ip_icmp.h will do David Gibson
2023-12-21 6:53 ` [PATCH v2 02/12] icmp: Don't set "port" on destination sockaddr for ping sockets David Gibson
2023-12-21 6:53 ` [PATCH v2 03/12] icmp: Remove redundant initialisation of sendto() address David Gibson
2023-12-21 6:53 ` [PATCH v2 04/12] icmp: Don't attempt to handle "wrong direction" ping socket traffic David Gibson
2024-01-06 15:59 ` Stefano Brivio
2024-01-07 0:37 ` David Gibson
2024-01-07 14:59 ` Stefano Brivio
2023-12-21 6:53 ` [PATCH v2 05/12] icmp: Don't attempt to match host IDs to guest IDs David Gibson
2023-12-21 6:53 ` [PATCH v2 06/12] icmp: Use -1 to represent "missing" sockets David Gibson
2024-01-06 15:59 ` Stefano Brivio
2024-01-07 0:38 ` David Gibson
2023-12-21 6:53 ` [PATCH v2 07/12] icmp: Simplify socket expiry scanning David Gibson
2024-01-06 15:59 ` Stefano Brivio
2024-01-07 0:41 ` David Gibson
2023-12-21 6:53 ` David Gibson [this message]
2024-01-06 16:00 ` [PATCH v2 08/12] icmp: Share more between IPv4 and IPv6 paths in icmp_tap_handler() Stefano Brivio
2024-01-07 4:41 ` David Gibson
2023-12-21 6:53 ` [PATCH v2 09/12] icmp: Consolidate icmp_sock_handler() with icmpv6_sock_handler() David Gibson
2024-01-06 16:00 ` Stefano Brivio
2024-01-07 0:45 ` David Gibson
2023-12-21 6:53 ` [PATCH v2 10/12] icmp: Warn on receive errors from ping sockets David Gibson
2023-12-21 6:53 ` [PATCH v2 11/12] icmp: Validate packets received on " David Gibson
2023-12-21 6:53 ` [PATCH v2 12/12] icmp: Dedicated functions for starting and closing ping sequences David Gibson
2024-01-06 16:01 ` Stefano Brivio
2024-01-07 4:30 ` David Gibson
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=20231221065327.1307827-9-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).