On Thu, 21 Dec 2023 17:53:23 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:
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(a)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;
This is obviously the same as the existing check. On the other hand,
I've been trying to use packet_get() to validate any packet read length
we need.
Here, the first call sets plen, but the only purpose is to get the
length of the message we need to relay. Given that we need at least
sizeof(*ih) here, I would prefer keep the explicit packet_get()
validation.
Same for the IPv6 path below.
Ok, fair enough. Updated.
--
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_!