diff --git a/src/datastructure.h b/src/datastructure.h index ff65edbc..18a75ea1 100644 --- a/src/datastructure.h +++ b/src/datastructure.h @@ -87,12 +87,13 @@ typedef struct { typedef struct { // Hot fields ordered first for cache locality; cold overTime[] array at // end. Contains size_t fields -> size differs by architecture (64-bit: - // 684 bytes for OVERTIME_SLOTS=145; 32-bit: ~668 bytes). - // On 64-bit, 4 bytes between hash (offset 48) and groupspos (offset - // 56) are intentional alignment padding: they ensure ippos lands at - // offset 64, the start of cache line 1. Without them, ippos would be - // at offset 60 and straddle the cache line boundary (bytes 60–67), - // causing a split load on every client IP comparison. + // 688 bytes for OVERTIME_SLOTS=145; 32-bit: smaller). + // On 64-bit, the 4 bytes between hash (offset 48) and groupspos (offset + // 56) were alignment padding ensuring ippos lands at offset 64, the start + // of cache line 1; without it ippos would sit at offset 60 and straddle + // the cache line boundary (bytes 60-67), causing a split load on every + // client IP comparison. hwaddr_next_try now occupies that padding, so the + // struct is the same size it always was and every offset is unchanged. unsigned char magic; char hwlen; unsigned char hwaddr[16]; // See DHCP_CHADDR_MAX in dnsmasq/dhcp-protocol.h @@ -111,6 +112,15 @@ typedef struct { unsigned int rate_limit; unsigned int numQueriesARP; uint32_t hash; + // When the next MAC lookup for this client may run, in monotonic seconds. + // A client whose address has no ARP entry - anything behind a router, and + // every loopback client - never yields one, so without this the lookup is + // retried on *every* query for the lifetime of the client. Deliberately + // uint32_t and placed here: it occupies the alignment padding described + // above, so it costs nothing per client and leaves every offset unchanged. + // Monotonic seconds, so it is a deadline rather than a wall-clock stamp; + // 32 bits is 136 years of uptime. + uint32_t hwaddr_next_try; size_t groupspos; // SHM intarray: client's assigned group IDs size_t ippos; size_t namepos; diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 0d1476c5..54e2a7c3 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -176,6 +176,23 @@ static uint64_t ftl_cache_misses = 0; #define PERF_STAT_CDB_REGEX 10 // in_regex(REGEX_DENY) #define PERF_STAT_COUNT 11 +// How long to wait before retrying a MAC lookup that came back empty. Long +// enough that a busy client costs one netlink round trip a minute instead of +// one per query, short enough that a genuinely late ARP entry is still picked +// up quickly. +#define MAC_LOOKUP_BACKOFF 60 + +// Monotonic seconds for the backoff deadline. A wall-clock stamp would let an +// NTP or admin clock step suppress MAC lookups for the size of the step (or +// expire them early), which is why the encrypted-DNS listeners use the same +// clock for their deadlines. +static inline uint32_t mac_lookup_now(void) +{ + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + return (uint32_t)ts.tv_sec; +} + static struct { uint64_t calls; // number of invocations uint64_t total_us; // cumulative microseconds @@ -1226,7 +1243,20 @@ bool _FTL_new_query(const unsigned int flags, const char *name, // Don't do this for internally generated queries (e.g., DNSSEC), if the // MAC address is already known or if the netlink socket is not available // (e.g., when retrying a query using TCP after UDP truncation) - if(!internal_query && client->hwlen < 1 && daemon->netlinkfd > 0) + // + // A client that has no ARP entry never gets one from this lookup, and the + // unresolved hwlen then re-triggers it on every single query. Measured on a + // loopback client: find_mac() itself costs ~67 us and, because the SHM lock + // is dropped around it, re-acquiring the lock costs a further ~108 us - some + // 27% of FTL's per-query work, spent re-learning that the answer is still no. + // Back off after a miss instead; a MAC that appears later is picked up on the + // next attempt. + // A loopback client is a special case of the same waste: 127.0.0.0/8 and ::1 + // never appear in the neighbour table at all, so unlike a client that might + // gain an ARP entry later there is nothing to come back for. Skip it outright + // rather than retrying once per backoff interval forever. + if(!internal_query && !mysockaddr_is_loopback(addr) && client->hwlen < 1 && + daemon->netlinkfd > 0 && mac_lookup_now() >= client->hwaddr_next_try) { // find_mac() may trigger a netlink kernel call // (iface_enumerate) to refresh the ARP table on a cache miss. @@ -1251,6 +1281,9 @@ bool _FTL_new_query(const unsigned int flags, const char *name, client->flags.found_group = false; memcpy(client->hwaddr, hwaddr, sizeof(hwaddr)); client->hwlen = hwlen; + // Nothing found: do not ask again for a while. + if(hwlen < 1) + client->hwaddr_next_try = mac_lookup_now() + MAC_LOOKUP_BACKOFF; } // Re-fetch all SHM pointers as SHM may have been remapped diff --git a/src/shmem.c b/src/shmem.c index 2658ec38..55bfe109 100644 --- a/src/shmem.c +++ b/src/shmem.c @@ -36,7 +36,8 @@ #include "lookup-table.h" /// The version of shared memory used -#define SHARED_MEMORY_VERSION 17 +// 18: clientsData gained hwaddr_next_try (MAC-lookup backoff) +#define SHARED_MEMORY_VERSION 18 /// The name of the shared memory. Use this when connecting to the shared memory. #define SHMEM_PATH "/dev/shm"