icmp: Simplify socket expiry scanning

Currently we use icmp_act[] to scan for ICMP ids which might have an open
socket which could time out.  However icmp_act[] contains no information
that's not already in icmp_id_map[] - it's just an "index" which allows
scanning for relevant entries with less cache footprint.

We only scan for ICMP socket expiry every 1s, though, so it's not clear
that cache footprint really matters.  Furthermore, there's no strong reason
we need to scan even that often - the timeout is fairly arbitrary and
approximate.

So, eliminate icmp_act[] in favour of directly scanning icmp_id_map[] and
compensate for the cache impact by reducing the scan frequency to once
every 10s.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
David Gibson 2024-01-16 16:16:13 +11:00 committed by Stefano Brivio
parent 24badd0acf
commit 15be1bfd81
2 changed files with 11 additions and 33 deletions

42
icmp.c
View file

@ -56,9 +56,6 @@ struct icmp_id_sock {
/* Indexed by ICMP echo identifier */ /* Indexed by ICMP echo identifier */
static struct icmp_id_sock icmp_id_map[IP_VERSIONS][ICMP_NUM_IDS]; static struct icmp_id_sock icmp_id_map[IP_VERSIONS][ICMP_NUM_IDS];
/* Bitmaps, activity monitoring needed for identifier */
static uint8_t icmp_act[IP_VERSIONS][DIV_ROUND_UP(ICMP_NUM_IDS, 8)];
/** /**
* icmp_sock_handler() - Handle new data from IPv4 ICMP socket * icmp_sock_handler() - Handle new data from IPv4 ICMP socket
* @c: Execution context * @c: Execution context
@ -194,7 +191,6 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
debug("ICMP: new socket %i for echo ID %i", s, id); debug("ICMP: new socket %i for echo ID %i", s, id);
} }
icmp_id_map[V4][id].ts = now->tv_sec; icmp_id_map[V4][id].ts = now->tv_sec;
bitmap_set(icmp_act[V4], id);
sa.sin_addr = *(struct in_addr *)daddr; sa.sin_addr = *(struct in_addr *)daddr;
if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL, if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL,
@ -237,7 +233,6 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
debug("ICMPv6: new socket %i for echo ID %i", s, id); debug("ICMPv6: new socket %i for echo ID %i", s, id);
} }
icmp_id_map[V6][id].ts = now->tv_sec; icmp_id_map[V6][id].ts = now->tv_sec;
bitmap_set(icmp_act[V6], id);
sa.sin6_addr = *(struct in6_addr *)daddr; sa.sin6_addr = *(struct in6_addr *)daddr;
if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL, if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL,
@ -261,24 +256,19 @@ fail_sock:
/** /**
* icmp_timer_one() - Handler for timed events related to a given identifier * icmp_timer_one() - Handler for timed events related to a given identifier
* @c: Execution context * @c: Execution context
* @v6: Set for IPv6 echo identifier bindings * @id_sock: Socket fd and activity timestamp
* @id: Echo identifier, host order
* @now: Current timestamp * @now: Current timestamp
*/ */
static void icmp_timer_one(const struct ctx *c, int v6, uint16_t id, static void icmp_timer_one(const struct ctx *c, struct icmp_id_sock *id_sock,
const struct timespec *now) const struct timespec *now)
{ {
struct icmp_id_sock *id_map = &icmp_id_map[v6 ? V6 : V4][id]; if (id_sock->sock < 0 || now->tv_sec - id_sock->ts <= ICMP_ECHO_TIMEOUT)
if (now->tv_sec - id_map->ts <= ICMP_ECHO_TIMEOUT)
return; return;
bitmap_clear(icmp_act[v6 ? V6 : V4], id); epoll_ctl(c->epollfd, EPOLL_CTL_DEL, id_sock->sock, NULL);
close(id_sock->sock);
epoll_ctl(c->epollfd, EPOLL_CTL_DEL, id_map->sock, NULL); id_sock->sock = -1;
close(id_map->sock); id_sock->seq = -1;
id_map->sock = -1;
id_map->seq = -1;
} }
/** /**
@ -288,23 +278,11 @@ static void icmp_timer_one(const struct ctx *c, int v6, uint16_t id,
*/ */
void icmp_timer(const struct ctx *c, const struct timespec *now) void icmp_timer(const struct ctx *c, const struct timespec *now)
{ {
long *word, tmp;
unsigned int i; unsigned int i;
int n, v6 = 0;
v6: for (i = 0; i < ICMP_NUM_IDS; i++) {
word = (long *)icmp_act[v6 ? V6 : V4]; icmp_timer_one(c, &icmp_id_map[V4][i], now);
for (i = 0; i < ARRAY_SIZE(icmp_act); i += sizeof(long), word++) { icmp_timer_one(c, &icmp_id_map[V6][i], now);
tmp = *word;
while ((n = ffsl(tmp))) {
tmp &= ~(1UL << (n - 1));
icmp_timer_one(c, v6, i * 8 + n - 1, now);
}
}
if (!v6) {
v6 = 1;
goto v6;
} }
} }

2
icmp.h
View file

@ -6,7 +6,7 @@
#ifndef ICMP_H #ifndef ICMP_H
#define ICMP_H #define ICMP_H
#define ICMP_TIMER_INTERVAL 1000 /* ms */ #define ICMP_TIMER_INTERVAL 10000 /* ms */
struct ctx; struct ctx;