We add a cache table to keep track of the contents of the kernel ARP and NDP tables. The table is fed from the just introduced netlink based neigbour subscription function. The new table eliminates the need for explicit netlink calls to find a host's MAC address. Signed-off-by: Jon Maloy --- v5: - Moved to earlier in series to reduce rebase conflicts v6: - Sqashed the hash list commit and the FIFO/LRU queue commit - Removed hash lookup. We now only use linear lookup in a linked list - Eliminated dynamic memory allocation. - Ensured there is only one call to clock_gettime() - Using MAC_ZERO instead of the previously dedicated definitions v7: - NOW using MAC_ZERO where needed - I am still using linear back-off for empty cache entries. Even an incoming, flow-creating packet from a local host gives no guarantee that its MAC address is in the ARP table, so we must allow for a few new attempts at first possible occasions. Only after several failed lookups can we conclude that we probably never will succeed. Hence the back-off. - Fixed a bug that David inadvertently made me aware of: I only intended to set the initial expiry value to MAC_CACHE_RENEWAL when an ARP/NDP table lookup was successful. - Improved struct and function description comments. v8: - Total re-design of table, adapting to the new, subscription based way of updating it. v9: - Catering for MAC address change for an existing host. v10: - Changes according to feedback from David Gibson v12: - Changes according to feedback from David and Stefano - Added dummy entries for loopback and default GW addresses --- conf.c | 1 + fwd.c | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ fwd.h | 7 +++ netlink.c | 7 ++- 4 files changed, 195 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index 66b9e63..3224ee6 100644 --- a/conf.c +++ b/conf.c @@ -2133,6 +2133,7 @@ void conf(struct ctx *c, int argc, char **argv) c->udp.fwd_out.mode = fwd_default; fwd_scan_ports_init(c); + fwd_neigh_table_init(c); if (!c->quiet) conf_print(c); diff --git a/fwd.c b/fwd.c index 250cf56..c34bb1c 100644 --- a/fwd.c +++ b/fwd.c @@ -33,6 +33,188 @@ static in_port_t fwd_ephemeral_max = NUM_PORTS - 1; #define PORT_RANGE_SYSCTL "/proc/sys/net/ipv4/ip_local_port_range" +#define NEIGH_TABLE_SLOTS 1024 /* Must be power of two */ +#define NEIGH_TABLE_SIZE (NEIGH_TABLE_SLOTS / 2) +static_assert((NEIGH_TABLE_SLOTS & (NEIGH_TABLE_SLOTS - 1)) == 0, + "NEIGH_TABLE_SLOTS must be a power of two"); + +/** + * struct neigh_table_entry - Entry in the ARP/NDP table + * @next: Next entry in slot or free list + * @addr: IP address of represented host + * @mac: MAC address of represented host + */ +struct neigh_table_entry { + struct neigh_table_entry *next; + union inany_addr addr; + uint8_t mac[ETH_ALEN]; +}; + +/** + * struct neigh_table - Cache of ARP/NDP table contents + * @entries: Entries to be plugged into the hash slots when allocated + * @slots: Hash table slots + * @free: Linked list of unused entries + */ +struct neigh_table { + struct neigh_table_entry entries[NEIGH_TABLE_SIZE]; + struct neigh_table_entry *slots[NEIGH_TABLE_SLOTS]; + struct neigh_table_entry *free; +}; + +static struct neigh_table neigh_table; + +/** + * neigh_table_slot() - Hash key to a number within the table range + * @c: Execution context + * @key: The key to be used for the hash + * + * Return: The resulting hash value + */ +static size_t neigh_table_slot(const struct ctx *c, + const union inany_addr *key) +{ + struct siphash_state st = SIPHASH_INIT(c->hash_secret); + uint32_t i; + + inany_siphash_feed(&st, key); + i = siphash_final(&st, sizeof(*key), 0); + + return ((size_t)i) & (NEIGH_TABLE_SIZE - 1); +} + +/** + * fwd_neigh_table_find() - Find a MAC table entry + * @c: Execution context + * @addr: Neighbour address to be used as key for the lookup + * + * Return: The matching entry, if found. Otherwise NULL. + */ +static struct neigh_table_entry *fwd_neigh_table_find(const struct ctx *c, + const union inany_addr *addr) +{ + size_t slot = neigh_table_slot(c, addr); + struct neigh_table_entry *e = neigh_table.slots[slot]; + + while (e && !inany_equals(&e->addr, addr)) + e = e->next; + + return e; +} + +/** + * fwd_neigh_table_update() - Allocate or update neighbour table entry + * @c: Execution context + * @addr: IP address used to determine insertion slot and store in entry + * @mac: The MAC address associated with the neighbour address + */ +void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr, + const uint8_t *mac) +{ + struct neigh_table *t = &neigh_table; + struct neigh_table_entry *e; + ssize_t slot; + + /* MAC address might change sometimes */ + e = fwd_neigh_table_find(c, addr); + if (e) { + if (inany_equals(addr, &inany_from_v4(c->ip4.guest_gw))) + return; + if (inany_equals(addr, (union inany_addr *)&c->ip6.guest_gw)) + return; + + memcpy(e->mac, mac, ETH_ALEN); + return; + } + + e = t->free; + if (!e) { + debug("Failed to allocate neighbour table entry"); + return; + } + t->free = e->next; + + slot = neigh_table_slot(c, addr); + e->next = t->slots[slot]; + t->slots[slot] = e; + + memcpy(&e->addr, addr, sizeof(*addr)); + memcpy(e->mac, mac, ETH_ALEN); +} + +/** + * fwd_neigh_table_free() - Remove an entry from a slot and add it to free list + * @c: Execution context + * @addr: IP address used to find the slot for the entry + */ +void fwd_neigh_table_free(const struct ctx *c, const union inany_addr *addr) +{ + ssize_t slot = neigh_table_slot(c, addr); + struct neigh_table *t = &neigh_table; + struct neigh_table_entry *e, **prev; + + prev = &t->slots[slot]; + e = t->slots[slot]; + while (e && !inany_equals(&e->addr, addr)) { + prev = &e->next; + e = e->next; + } + if (!e) + return; + + *prev = e->next; + e->next = t->free; + t->free = e; + memset(&e->addr, 0, sizeof(*addr)); + memset(e->mac, 0, ETH_ALEN); +} + +/** + * fwd_neigh_mac_get() - Look up MAC address in the ARP/NDP table + * @c: Execution context + * @addr: Neighbour IP address used as lookup key + * @mac: Buffer for Ethernet MAC to return, found or default value. + * + * Return: true if real MAC found, false if not found or if failure + */ +bool fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, + uint8_t *mac) +{ + const struct neigh_table_entry *e = fwd_neigh_table_find(c, addr); + + if (e) + memcpy(mac, e->mac, ETH_ALEN); + else + memcpy(mac, c->our_tap_mac, ETH_ALEN); + + return !!e; +} + +/** + * fwd_neigh_table_init() - Initialize the neighbour table + * @c: Execution context + */ +void fwd_neigh_table_init(const struct ctx *c) +{ + struct neigh_table *t = &neigh_table; + const uint8_t *omac = c->our_tap_mac; + struct neigh_table_entry *e; + int i; + + memset(t, 0, sizeof(*t)); + for (i = 0; i < NEIGH_TABLE_SIZE; i++) { + e = &t->entries[i]; + e->next = t->free; + t->free = e; + } + + /* These addresses must always map to our own MAC address */ + fwd_neigh_table_update(c, &inany_loopback4, omac); + fwd_neigh_table_update(c, &inany_loopback6, omac); + fwd_neigh_table_update(c, &inany_from_v4(c->ip4.guest_gw), omac); + fwd_neigh_table_update(c, (union inany_addr *)&c->ip6.guest_gw, omac); +} + /** fwd_probe_ephemeral() - Determine what ports this host considers ephemeral * * Work out what ports the host thinks are emphemeral and record it for later diff --git a/fwd.h b/fwd.h index 65c7c96..6ca743c 100644 --- a/fwd.h +++ b/fwd.h @@ -56,5 +56,12 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); +void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr, + const uint8_t *mac); +void fwd_neigh_table_free(const struct ctx *c, + const union inany_addr *addr); +bool fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, + uint8_t *mac); +void fwd_neigh_table_init(const struct ctx *c); #endif /* FWD_H */ diff --git a/netlink.c b/netlink.c index 3fe2fdd..4be5fcf 100644 --- a/netlink.c +++ b/netlink.c @@ -1192,10 +1192,13 @@ static void nl_neigh_msg_read(const struct ctx *c, struct nlmsghdr *nh) inany_from_af(&addr, ndm->ndm_family, dst); inany_ntop(dst, ip_str, sizeof(ip_str)); - if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) + if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) { trace("neigh table update: %s / %s", ip_str, mac_str); - else + fwd_neigh_table_update(c, &addr, mac); + } else { trace("neigh table delete: %s / %s", ip_str, mac_str); + fwd_neigh_table_free(c, &addr); + } } /** -- 2.50.1

When we receive an ARP request or NDP neigbour solicitation over the tap interface for a host on the local network segment attached to the template interface, we respond with that host's real MAC address, if available. Signed-off-by: Jon Maloy Reviewed-by: David Gibson --- v3: - Added helper function to find out if a remote ip address is subject to NAT. This filters out local host addresses which should be presented with the passt/pasta local MAC address 9a:55:9a:55:9a:55 even though it is on the local segment. - Adapted to the change in nl_mac_get() function, so that we now consider only the template interface when checking the ARP/NDP table. v4: - Moved NAT check into the function nat_outbound() to obtain more precise criteria for when NAT is used. We may in theory have NAT even if original and translated addresses are equal, and we want to catch this case. - I chose to keep the wrapper funtion inany_nat(), but moved it to fwd.h/fwd.c and renamed it to fwd_inany_nat(). v5: - Simplified criteria for when we do ARP/NDP lookup. Now, we just try with the potentially translated address after an attempted NAT check. - Using the new ARP/NDP cache table instead of using netlink directly. v6: - Fixes after feedback from David: - Renamed nat_outbound() to fwd_nat_outbound() - Eliminated unnecessary temporary variable in arp.c::arp() v12: - Some minor changes after feedback from Stefano --- arp.c | 9 +++++++-- fwd.c | 8 ++++---- fwd.h | 2 ++ inany.c | 1 + ndp.c | 8 ++++++++ 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/arp.c b/arp.c index b08780f..a95c377 100644 --- a/arp.c +++ b/arp.c @@ -69,6 +69,7 @@ static bool ignore_arp(const struct ctx *c, */ int arp(const struct ctx *c, struct iov_tail *data) { + union inany_addr tgt, tgt_nat; struct { struct ethhdr eh; struct arphdr ah; @@ -102,8 +103,12 @@ int arp(const struct ctx *c, struct iov_tail *data) resp.ah.ar_hln = ah->ar_hln; resp.ah.ar_pln = ah->ar_pln; - /* ARP message */ - memcpy(resp.am.sha, c->our_tap_mac, sizeof(resp.am.sha)); + /* MAC address to return in ARP message */ + inany_from_af(&tgt, AF_INET, am->tip); + fwd_nat_outbound(c, &tgt, &tgt_nat); + fwd_neigh_mac_get(c, &tgt_nat, resp.am.sha); + + /* Rest of ARP message */ memcpy(resp.am.sip, am->tip, sizeof(resp.am.sip)); memcpy(resp.am.tha, am->sha, sizeof(resp.am.tha)); memcpy(resp.am.tip, am->sip, sizeof(resp.am.tip)); diff --git a/fwd.c b/fwd.c index ade97c8..a69d84d 100644 --- a/fwd.c +++ b/fwd.c @@ -522,7 +522,7 @@ static bool fwd_guest_accessible(const struct ctx *c, } /** - * nat_outbound() - Apply address translation for outbound (TAP to HOST) + * fwd_nat_outbound() - Apply address translation for outbound (TAP to HOST) * @c: Execution context * @addr: Input address (as seen on TAP interface) * @translated: Output address (as seen on HOST interface) @@ -530,8 +530,8 @@ static bool fwd_guest_accessible(const struct ctx *c, * Only handles translations that depend *only* on the address. Anything * related to specific ports or flows is handled elsewhere. */ -static void nat_outbound(const struct ctx *c, const union inany_addr *addr, - union inany_addr *translated) +void fwd_nat_outbound(const struct ctx *c, const union inany_addr *addr, + union inany_addr *translated) { if (inany_equals4(addr, &c->ip4.map_host_loopback)) *translated = inany_loopback4; @@ -565,7 +565,7 @@ uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, inany_equals6(&ini->oaddr, &c->ip6.dns_match)) tgt->eaddr.a6 = c->ip6.dns_host; else - nat_outbound(c, &ini->oaddr, &tgt->eaddr); + fwd_nat_outbound(c, &ini->oaddr, &tgt->eaddr); tgt->eport = ini->oport; diff --git a/fwd.h b/fwd.h index 6ca743c..f91fa55 100644 --- a/fwd.h +++ b/fwd.h @@ -50,6 +50,8 @@ void fwd_scan_ports_init(struct ctx *c); bool nat_inbound(const struct ctx *c, const union inany_addr *addr, union inany_addr *translated); +void fwd_nat_outbound(const struct ctx *c, const union inany_addr *addr, + union inany_addr *translated); uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto, diff --git a/inany.c b/inany.c index 65a39f9..7680439 100644 --- a/inany.c +++ b/inany.c @@ -16,6 +16,7 @@ #include "ip.h" #include "siphash.h" #include "inany.h" +#include "fwd.h" const union inany_addr inany_loopback4 = INANY_INIT4(IN4ADDR_LOOPBACK_INIT); const union inany_addr inany_any4 = INANY_INIT4(IN4ADDR_ANY_INIT); diff --git a/ndp.c b/ndp.c index d7f64a3..6bba0a8 100644 --- a/ndp.c +++ b/ndp.c @@ -196,6 +196,7 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst, static void ndp_na(const struct ctx *c, const struct in6_addr *dst, const struct in6_addr *addr) { + union inany_addr tgt, tgt_nat; struct ndp_na na = { .ih = { .icmp6_type = NA, @@ -215,6 +216,13 @@ static void ndp_na(const struct ctx *c, const struct in6_addr *dst, memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN); + /* Respond with recorded MAC address if remote host's address + * or NAT translated address can be found in NDP table. + */ + inany_from_af(&tgt, AF_INET6, addr); + fwd_nat_outbound(c, &tgt, &tgt_nat); + fwd_neigh_mac_get(c, &tgt_nat, na.target_l2_addr.mac); + ndp_send(c, dst, &na, sizeof(na)); } -- 2.50.1

When communicating with remote hosts on the local network, some guest applications want to see the real MAC address of that host instead of PASST/PASTA's own tap address. The flow_common structure is a convenient location for storing that address, so we do that in this commit. Note that we don´t add actual usage of this address here, that will be done in later commits. Signed-off-by: Jon Maloy Reviewed-by: David Gibson --- v3: - Moved the remote host macaddress from struct flowside to struct flow_common. I chose to call it 'omac' as suggested by David, although in my understanding the correct name would be 'emac'. (In general I find the address naming scheme confusing.) - Adapted to new signature of function nl_mac_get(), now passing it the index of the template interface. v4: - Renamed flow_commeon->omac to flow_common->tap_omac to make is role in the code clearer v5: - Modified the criteria for ARP/NDP table lookup like in the previous commits. - Removed the PIF_TAP lookup case, as David suggested, and did instead give the flow->tap_omac field a value marking it as non-initialized. - Calling the cache table instead of netlink for ARP/NDP lookup. - Unconditionally using the potentially translated IP address in the lookup, instead of only if NAT really was applied. v6: - Using MAC_ZERO instead of own definitions v12:- Using MAC_UNDEF (==ff:ff:ff:ff:ff:ff) instead of MAC_ZERO, which is a legal MAC address. --- flow.c | 2 ++ flow.h | 2 ++ util.h | 2 ++ 3 files changed, 6 insertions(+) diff --git a/flow.c b/flow.c index feefda3..df3ce2a 100644 --- a/flow.c +++ b/flow.c @@ -449,6 +449,7 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow, switch (f->pif[INISIDE]) { case PIF_TAP: + memcpy(f->tap_omac, MAC_UNDEF, ETH_ALEN); tgtpif = fwd_nat_from_tap(c, proto, ini, tgt); break; @@ -458,6 +459,7 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow, case PIF_HOST: tgtpif = fwd_nat_from_host(c, proto, ini, tgt); + fwd_neigh_mac_get(c, &ini->eaddr, f->tap_omac); break; default: diff --git a/flow.h b/flow.h index cac618a..f342895 100644 --- a/flow.h +++ b/flow.h @@ -177,6 +177,7 @@ int flowside_connect(const struct ctx *c, int s, * @type: Type of packet flow * @pif[]: Interface for each side of the flow * @side[]: Information for each side of the flow + * @tap_omac: MAC address of remote endpoint as seen from the guest */ struct flow_common { #ifdef __GNUC__ @@ -192,6 +193,7 @@ struct flow_common { #endif uint8_t pif[SIDES]; struct flowside side[SIDES]; + uint8_t tap_omac[6]; }; #define FLOW_INDEX_BITS 17 /* 128k - 1 */ diff --git a/util.h b/util.h index 22eaac5..6fc8f5d 100644 --- a/util.h +++ b/util.h @@ -101,6 +101,8 @@ void abort_with_msg(const char *fmt, ...) ((uint8_t [ETH_ALEN]){ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }) #define MAC_ZERO ((uint8_t [ETH_ALEN]){ 0 }) #define MAC_IS_ZERO(addr) (!memcmp((addr), MAC_ZERO, ETH_ALEN)) +#define MAC_UNDEF MAC_BROADCAST +#define MAC_IS_UNDEF(addr) (!memcmp((addr), MAC_UNDEF, ETH_ALEN)) #ifndef __bswap_constant_16 #define __bswap_constant_16(x) \ -- 2.50.1

In the next commit it must be possible for the callers of function tap_push_l2h() to specify which source MAC address should be added to the ethernet header sent over the tap interface. As a preparation, we now add a new argument to that function, still without any logical changes. Signed-off-by: Jon Maloy Reviewed-by: David Gibson --- v3: -Improved comment for src_mac argument, as suggested by Stefano. v4: -Re-added comment in tap.c and remove redundant argument check in tap_push_l2() v5: -Ensured that MAC argument to tap_push_l2h() never is NULL. v6: -Clarification of comment about ARP lookup v12: - Minor updates after feedback from Stefano --- tap.c | 16 +++++++++------- tap.h | 3 ++- tcp.c | 6 ++++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/tap.c b/tap.c index 250a0f6..84266d0 100644 --- a/tap.c +++ b/tap.c @@ -171,17 +171,19 @@ const struct in6_addr *tap_ip6_daddr(const struct ctx *c, * tap_push_l2h() - Build an L2 header for an inbound packet * @c: Execution context * @buf: Buffer address at which to generate header + * @src_mac: MAC address to be used as source for message * @proto: Ethernet protocol number for L3 * * Return: pointer at which to write the packet's payload */ -void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto) +void *tap_push_l2h(const struct ctx *c, void *buf, + const void *src_mac, uint16_t proto) { struct ethhdr *eh = (struct ethhdr *)buf; - /* TODO: ARP table lookup */ + /* TODO: ARP lookup on tap side */ memcpy(eh->h_dest, c->guest_mac, ETH_ALEN); - memcpy(eh->h_source, c->our_tap_mac, ETH_ALEN); + memcpy(eh->h_source, src_mac, ETH_ALEN); eh->h_proto = ntohs(proto); return eh + 1; } @@ -261,7 +263,7 @@ void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport, { size_t l4len = dlen + sizeof(struct udphdr); char buf[USHRT_MAX]; - struct iphdr *ip4h = tap_push_l2h(c, buf, ETH_P_IP); + struct iphdr *ip4h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP); struct udphdr *uh = tap_push_ip4h(ip4h, src, dst, l4len, IPPROTO_UDP); char *data = tap_push_uh4(uh, src, sport, dst, dport, in, dlen); @@ -281,7 +283,7 @@ void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_addr dst, const void *in, size_t l4len) { char buf[USHRT_MAX]; - struct iphdr *ip4h = tap_push_l2h(c, buf, ETH_P_IP); + struct iphdr *ip4h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP); struct icmphdr *icmp4h = tap_push_ip4h(ip4h, src, dst, l4len, IPPROTO_ICMP); @@ -367,7 +369,7 @@ void tap_udp6_send(const struct ctx *c, { size_t l4len = dlen + sizeof(struct udphdr); char buf[USHRT_MAX]; - struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6); + struct ipv6hdr *ip6h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV6); struct udphdr *uh = tap_push_ip6h(ip6h, src, dst, l4len, IPPROTO_UDP, flow); char *data = tap_push_uh6(uh, src, sport, dst, dport, in, dlen); @@ -389,7 +391,7 @@ void tap_icmp6_send(const struct ctx *c, const void *in, size_t l4len) { char buf[USHRT_MAX]; - struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6); + struct ipv6hdr *ip6h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV6); struct icmp6hdr *icmp6h = tap_push_ip6h(ip6h, src, dst, l4len, IPPROTO_ICMPV6, 0); diff --git a/tap.h b/tap.h index 21db4d2..02f7761 100644 --- a/tap.h +++ b/tap.h @@ -70,7 +70,8 @@ static inline void tap_hdr_update(struct tap_hdr *thdr, size_t l2len) } unsigned long tap_l2_max_len(const struct ctx *c); -void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto); +void *tap_push_l2h(const struct ctx *c, void *buf, + const void *src_mac, uint16_t proto); void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src, struct in_addr dst, size_t l4len, uint8_t proto); void *tap_push_uh4(struct udphdr *uh, struct in_addr src, in_port_t sport, diff --git a/tcp.c b/tcp.c index 06b07f8..79d2efe 100644 --- a/tcp.c +++ b/tcp.c @@ -1979,7 +1979,8 @@ static void tcp_rst_no_conn(const struct ctx *c, int af, return; if (af == AF_INET) { - struct iphdr *ip4h = tap_push_l2h(c, buf, ETH_P_IP); + struct iphdr *ip4h = tap_push_l2h(c, buf, c->our_tap_mac, + ETH_P_IP); const struct in_addr *rst_src = daddr; const struct in_addr *rst_dst = saddr; @@ -1989,7 +1990,8 @@ static void tcp_rst_no_conn(const struct ctx *c, int af, *rst_src, *rst_dst); } else { - struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6); + struct ipv6hdr *ip6h = tap_push_l2h(c, buf, c->our_tap_mac, + ETH_P_IPV6); const struct in6_addr *rst_src = daddr; const struct in6_addr *rst_dst = saddr; -- 2.50.1

Even ICMP needs to be updated to use the external MAC address instead of just the own tap address when applicable. We do that here. Signed-off-by: Jon Maloy Reviewed-by: David Gibson --- v3: - Adapted to the move of external MAC address from struct flowside to struct flow_common v4: - Adapted to name changes in previous commits in this series v5: - Added conditional lookup in ARP/NDP if the flow's tap_omac is undefined v6: - Looking up MAC of ICMP generating node in udp_send_tap_icmp4/6() when available, instead trusting the contents of flow->tap_omac. v12: - Using MAC_IS_UNDEF() instead of MAC_IS_ZERO() - Comment update after feedback from Stefano --- icmp.c | 8 ++++++-- ndp.c | 2 +- tap.c | 10 ++++++---- tap.h | 4 ++-- udp.c | 12 ++++++++++-- 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/icmp.c b/icmp.c index 6dffafb..93b394a 100644 --- a/icmp.c +++ b/icmp.c @@ -125,17 +125,21 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref) flow_dbg(pingf, "echo reply to tap, ID: %"PRIu16", seq: %"PRIu16, ini->eport, seq); + /* Find if neighbour table has a recorded MAC address */ + if (MAC_IS_UNDEF(pingf->f.tap_omac)) + fwd_neigh_mac_get(c, &ini->oaddr, pingf->f.tap_omac); + if (pingf->f.type == FLOW_PING4) { const struct in_addr *saddr = inany_v4(&ini->oaddr); const struct in_addr *daddr = inany_v4(&ini->eaddr); ASSERT(saddr && daddr); /* Must have IPv4 addresses */ - tap_icmp4_send(c, *saddr, *daddr, buf, n); + tap_icmp4_send(c, *saddr, *daddr, buf, pingf->f.tap_omac, n); } else if (pingf->f.type == FLOW_PING6) { const struct in6_addr *saddr = &ini->oaddr.a6; const struct in6_addr *daddr = &ini->eaddr.a6; - tap_icmp6_send(c, saddr, daddr, buf, n); + tap_icmp6_send(c, saddr, daddr, buf, pingf->f.tap_omac, n); } return; diff --git a/ndp.c b/ndp.c index 6bba0a8..1af5e4b 100644 --- a/ndp.c +++ b/ndp.c @@ -184,7 +184,7 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst, { const struct in6_addr *src = &c->ip6.our_tap_ll; - tap_icmp6_send(c, src, dst, buf, l4len); + tap_icmp6_send(c, src, dst, buf, c->our_tap_mac, l4len); } /** diff --git a/tap.c b/tap.c index 84266d0..3ef2734 100644 --- a/tap.c +++ b/tap.c @@ -277,13 +277,14 @@ void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport, * @src: IPv4 source address * @dst: IPv4 destination address * @in: ICMP packet, including ICMP header + * @src_mac: MAC address to be used as source for message * @l4len: ICMP packet length, including ICMP header */ void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_addr dst, - const void *in, size_t l4len) + const void *in, const void *src_mac, size_t l4len) { char buf[USHRT_MAX]; - struct iphdr *ip4h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP); + struct iphdr *ip4h = tap_push_l2h(c, buf, src_mac, ETH_P_IP); struct icmphdr *icmp4h = tap_push_ip4h(ip4h, src, dst, l4len, IPPROTO_ICMP); @@ -384,14 +385,15 @@ void tap_udp6_send(const struct ctx *c, * @src: IPv6 source address * @dst: IPv6 destination address * @in: ICMP packet, including ICMP header + * @src_mac: MAC address to be used as source for message * @l4len: ICMP packet length, including ICMP header */ void tap_icmp6_send(const struct ctx *c, const struct in6_addr *src, const struct in6_addr *dst, - const void *in, size_t l4len) + const void *in, const void *src_mac, size_t l4len) { char buf[USHRT_MAX]; - struct ipv6hdr *ip6h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV6); + struct ipv6hdr *ip6h = tap_push_l2h(c, buf, src_mac, ETH_P_IPV6); struct icmp6hdr *icmp6h = tap_push_ip6h(ip6h, src, dst, l4len, IPPROTO_ICMPV6, 0); diff --git a/tap.h b/tap.h index 02f7761..1864173 100644 --- a/tap.h +++ b/tap.h @@ -91,7 +91,7 @@ void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport, struct in_addr dst, in_port_t dport, const void *in, size_t dlen); void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_addr dst, - const void *in, size_t l4len); + const void *in, const void *src_mac, size_t l4len); const struct in6_addr *tap_ip6_daddr(const struct ctx *c, const struct in6_addr *src); void *tap_push_ip6h(struct ipv6hdr *ip6h, @@ -103,7 +103,7 @@ void tap_udp6_send(const struct ctx *c, uint32_t flow, void *in, size_t dlen); void tap_icmp6_send(const struct ctx *c, const struct in6_addr *src, const struct in6_addr *dst, - const void *in, size_t l4len); + const void *in, const void *src_mac, size_t l4len); void tap_send_single(const struct ctx *c, const void *data, size_t l2len); size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, size_t bufs_per_frame, size_t nframes); diff --git a/udp.c b/udp.c index 3acc401..3981e9b 100644 --- a/udp.c +++ b/udp.c @@ -400,6 +400,8 @@ static void udp_send_tap_icmp4(const struct ctx *c, struct in_addr eaddr = toside->eaddr.v4mapped.a4; in_port_t eport = toside->eport; in_port_t oport = toside->oport; + union inany_addr saddr_any; + uint8_t tap_omac[ETH_ALEN]; struct { struct icmphdr icmp4h; struct iphdr ip4h; @@ -421,7 +423,10 @@ static void udp_send_tap_icmp4(const struct ctx *c, tap_push_uh4(&msg.uh, eaddr, eport, oaddr, oport, in, dlen); memcpy(&msg.data, in, dlen); - tap_icmp4_send(c, saddr, eaddr, &msg, msglen); + /* Try to obtain the MAC address of the generating node */ + saddr_any = inany_from_v4(saddr); + fwd_neigh_mac_get(c, &saddr_any, tap_omac); + tap_icmp4_send(c, saddr, eaddr, &msg, tap_omac, msglen); } @@ -445,6 +450,7 @@ static void udp_send_tap_icmp6(const struct ctx *c, const struct in6_addr *eaddr = &toside->eaddr.a6; in_port_t eport = toside->eport; in_port_t oport = toside->oport; + uint8_t tap_omac[ETH_ALEN]; struct { struct icmp6_hdr icmp6h; struct ipv6hdr ip6h; @@ -466,7 +472,9 @@ static void udp_send_tap_icmp6(const struct ctx *c, tap_push_uh6(&msg.uh, eaddr, eport, oaddr, oport, in, dlen); memcpy(&msg.data, in, dlen); - tap_icmp6_send(c, saddr, eaddr, &msg, msglen); + /* Try to obtain the MAC address of the generating node */ + fwd_neigh_mac_get(c, (union inany_addr *) saddr, tap_omac); + tap_icmp6_send(c, saddr, eaddr, &msg, tap_omac, msglen); } /** -- 2.50.1

On Thu, Oct 02, 2025 at 08:34:04PM -0400, Jon Maloy wrote:

The solution to bug https://bugs.passt.top/show_bug.cgi?id=120 requires the ability to translate from an IP address to its corresponding MAC address in cases where those are present in the ARP or NDP tables.

To keep track of the contents of these tables we add a netlink based neighbour subscription feature.

Signed-off-by: Jon Maloy Reviewed-by: David Gibson

--- v3: - Added an attribute contianing NDA_DST to sent message, so that we let the kernel do the filtering of the IP address and return only one entry. - Added interface index to the call signature. Since the only interface we know is the template interface, this limits the number of hosts that will be seen as 'network segment local' from a PASST viewpoint. v4: - Made loop independent of attribute order. - Ignoring L2 addresses which are not of size ETH_ALEN. v5: - Changed return value of new function, so caller can know if a MAC address really was found. v6: - Removed warning printout which had ended up in the wrong commit. v8: - Changed to neighbour event subscription model - netlink: arp/ndp table subscription v10:- Updated according to David's latest comments on v8 - Added functionaly where we initially read current state of ARP/NDP tables v12:- Updates based on feedback from David and Stefano --- epoll_type.h | 2 + netlink.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++- netlink.h | 4 + passt.c | 7 ++ 4 files changed, 214 insertions(+), 3 deletions(-)

diff --git a/epoll_type.h b/epoll_type.h index 12ac59b..a90ffb6 100644 --- a/epoll_type.h +++ b/epoll_type.h @@ -44,6 +44,8 @@ enum epoll_type { EPOLL_TYPE_REPAIR_LISTEN, /* TCP_REPAIR helper socket */ EPOLL_TYPE_REPAIR, + /* Netlink neighbour subscription socket */ + EPOLL_TYPE_NL_NEIGH,

EPOLL_NUM_TYPES, }; diff --git a/netlink.c b/netlink.c index c436780..3fe2fdd 100644 --- a/netlink.c +++ b/netlink.c @@ -26,6 +26,7 @@ #include #include #include +#include

#include #include @@ -40,6 +41,10 @@ #define RTNH_NEXT_AND_DEC(rtnh, attrlen) \ ((attrlen) -= RTNH_ALIGN((rtnh)->rtnh_len), RTNH_NEXT(rtnh))

+/* Convenience macro borrowed from kernel */ +#define NUD_VALID \ + (NUD_PERMANENT | NUD_NOARP | NUD_REACHABLE | NUD_PROBE | NUD_STALE) + /* Netlink expects a buffer of at least 8kiB or the system page size, * whichever is larger. 32kiB is recommended for more efficient. * Since the largest page size on any remotely common Linux setup is @@ -50,9 +55,10 @@ #define NLBUFSIZ 65536

/* Socket in init, in target namespace, sequence (just needs to be monotonic) */ -int nl_sock = -1; -int nl_sock_ns = -1; -static int nl_seq = 1; +int nl_sock = -1; +int nl_sock_ns = -1; +static int nl_sock_neigh = -1; +static int nl_seq = 1;

/** * nl_sock_init_do() - Set up netlink sockets in init or target namespace @@ -1103,3 +1109,195 @@ int nl_link_set_flags(int s, unsigned int ifi,

return nl_do(s, &req, RTM_NEWLINK, 0, sizeof(req)); } + +/** + * nl_neigh_msg_read() - Interpret a neigbour state message from netlink + * @c: Execution context + * @nh: Message to be read + * + */ +static void nl_neigh_msg_read(const struct ctx *c, struct nlmsghdr *nh) +{ + struct ndmsg *ndm = NLMSG_DATA(nh); + struct rtattr *rta = (struct rtattr *)(ndm + 1); + size_t na = NLMSG_PAYLOAD(nh, sizeof(*ndm)); + char ip_str[INET6_ADDRSTRLEN]; + char mac_str[ETH_ADDRSTRLEN]; + const uint8_t *lladdr = NULL; + const void *dst = NULL; + size_t lladdr_len = 0; + uint8_t mac[ETH_ALEN]; + union inany_addr addr; + size_t dstlen = 0; + + if (nh->nlmsg_type == NLMSG_DONE) + return; + + if (nh->nlmsg_type == NLMSG_ERROR) { + warn_perror("nlmsg_type error at msg read");

warn_perror() won't work here, because the error code isn't in global errno. See nl_status() for how to extract the error code from the message.

+ return; + } + + if (nh->nlmsg_type != RTM_NEWNEIGH && + nh->nlmsg_type != RTM_DELNEIGH) + return; + + for (; RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { + switch (rta->rta_type) { + case NDA_DST: + dst = RTA_DATA(rta); + dstlen = RTA_PAYLOAD(rta); + break; + case NDA_LLADDR: + lladdr = RTA_DATA(rta); + lladdr_len = RTA_PAYLOAD(rta); + break; + default: + break; + } + } + + if (!dst) + return; + + if (!lladdr || lladdr_len != ETH_ALEN) + return;

Do we need lladdr for the DELNEIGH case?

+ + if (ndm->ndm_type != ARPHRD_ETHER) + return; + + memcpy(mac, lladdr, ETH_ALEN); + eth_ntop(mac, mac_str, sizeof(mac_str)); + + if (ndm->ndm_family == AF_INET && + ndm->ndm_ifindex != c->ifi4) + return; + + if (ndm->ndm_family == AF_INET6 && + ndm->ndm_ifindex != c->ifi6) + return; + + if (ndm->ndm_family != AF_INET && + ndm->ndm_family != AF_INET6) + return; + + if (ndm->ndm_family == AF_INET && + dstlen != sizeof(struct in_addr)) + return; + + if (ndm->ndm_family == AF_INET6 && + dstlen != sizeof(struct in6_addr)) + return;

I'd suggest a warn() for those last two cases, since it means the kernel is doing something really weird.

+ inany_from_af(&addr, ndm->ndm_family, dst); + inany_ntop(dst, ip_str, sizeof(ip_str)); + + if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) + trace("neigh table update: %s / %s", ip_str, mac_str); + else + trace("neigh table delete: %s / %s", ip_str, mac_str);

This will give a misleading message if you have RTM_NEWNEIGH and !NUD_VALID.

+} + +/** + * nl_neigh_sync() - Read current contents ARP/NDP tables + * @c: Execution context + * @proto: Protocol, AF_INET or AF_INET6 + * @ifi: Interface index + * + */ +static void nl_neigh_sync(const struct ctx *c, int proto, int ifi) +{ + struct { + struct nlmsghdr nlh; + struct ndmsg ndm; + } req = { + .nlh = {0}, + .ndm.ndm_family = proto, + .ndm.ndm_ifindex = ifi, + .ndm.ndm_state = 0, + .ndm.ndm_flags = 0, + .ndm.ndm_type = 0 + }; + struct nlmsghdr *nh; + char buf[NLBUFSIZ]; + ssize_t status; + uint32_t seq; + + seq = nl_send(nl_sock_neigh, &req, RTM_GETNEIGH, NLM_F_DUMP, sizeof(req)); + nl_foreach_oftype(nh, status, nl_sock_neigh, buf, seq, RTM_NEWNEIGH) + nl_neigh_msg_read(c, nh); + if (status < 0) + warn("Failed to read netlink message. status == %li", status); +} + +/** + * nl_neigh_notify_handler() - Non-blocking drain of pending neighbour updates + * @c: Execution context + */ +void nl_neigh_notify_handler(const struct ctx *c) +{ + char buf[NLBUFSIZ]; + + for (;;) { + ssize_t n = recv(nl_sock_neigh, buf, sizeof(buf), MSG_DONTWAIT); + struct nlmsghdr *nh; + + if (n < 0) { + if (errno != EAGAIN) + warn_perror("Failed to read from netlink sock"); + return; + } + nh = (struct nlmsghdr *)buf; + for (; NLMSG_OK(nh, n); nh = NLMSG_NEXT(nh, n)) + nl_neigh_msg_read(c, nh); + } +} + +/** + * nl_neigh_notify_init() - Open netlink sock and subscribe to neighbour events + * @c: Execution context + * + * Return: 0 on success, -1 on failure + */ +int nl_neigh_notify_init(const struct ctx *c) +{ + union epoll_ref ref = { + .type = EPOLL_TYPE_NL_NEIGH + }; + struct epoll_event ev = { + .events = EPOLLIN + }; + struct sockaddr_nl addr = { + .nl_family = AF_NETLINK, + .nl_groups = RTMGRP_NEIGH, + }; + + if (nl_sock_neigh >= 0) { + warn("RTMGRP_NEIGH already exists"); + return 0; + } + nl_sock_neigh = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_ROUTE); + if (nl_sock_neigh < 0) { + warn("Failed create RTMGRP_NEIGH socket"); + return -1; + } + if (bind(nl_sock_neigh, (struct sockaddr *)&addr, sizeof(addr)) < 0) { + close(nl_sock_neigh); + nl_sock_neigh = -1; + warn("Failed bind RTMGRP_NEIGH socket"); + return -1; + } + + ev.data.u64 = ref.u64; + if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, nl_sock_neigh, &ev) == -1) { + close(nl_sock_neigh); + nl_sock_neigh = -1; + warn("Failed do epoll_ctrl() on RTMGRP_NEIGH socket"); + return -1; + } + + nl_neigh_sync(c, AF_INET, c->ifi4); + nl_neigh_sync(c, AF_INET6, c->ifi6); + + return 0; +} diff --git a/netlink.h b/netlink.h index b51e99c..8f1e9b9 100644 --- a/netlink.h +++ b/netlink.h @@ -17,6 +17,8 @@ int nl_route_dup(int s_src, unsigned int ifi_src, int s_dst, unsigned int ifi_dst, sa_family_t af); int nl_addr_get(int s, unsigned int ifi, sa_family_t af, void *addr, int *prefix_len, void *addr_l); +bool nl_neigh_mac_get(int s, const union inany_addr *addr, int ifi, + unsigned char *mac); int nl_addr_set(int s, unsigned int ifi, sa_family_t af, const void *addr, int prefix_len); int nl_addr_get_ll(int s, unsigned int ifi, struct in6_addr *addr); @@ -28,5 +30,7 @@ int nl_link_set_mac(int s, unsigned int ifi, const void *mac); int nl_link_set_mtu(int s, unsigned int ifi, int mtu); int nl_link_set_flags(int s, unsigned int ifi, unsigned int set, unsigned int change); +int nl_neigh_notify_init(const struct ctx *c); +void nl_neigh_notify_handler(const struct ctx *c);

#endif /* NETLINK_H */ diff --git a/passt.c b/passt.c index 31fbb75..e21d6ba 100644 --- a/passt.c +++ b/passt.c @@ -53,6 +53,7 @@ #include "vu_common.h" #include "migrate.h" #include "repair.h" +#include "netlink.h"

#define EPOLL_EVENTS 8

@@ -79,6 +80,7 @@ char *epoll_type_str[] = { [EPOLL_TYPE_VHOST_KICK] = "vhost-user kick socket", [EPOLL_TYPE_REPAIR_LISTEN] = "TCP_REPAIR helper listening socket", [EPOLL_TYPE_REPAIR] = "TCP_REPAIR helper socket", + [EPOLL_TYPE_NL_NEIGH] = "netlink neighbour subscription socket", }; static_assert(ARRAY_SIZE(epoll_type_str) == EPOLL_NUM_TYPES, "epoll_type_str[] doesn't match enum epoll_type"); @@ -322,6 +324,8 @@ int main(int argc, char **argv)

pcap_init(&c);

+ nl_neigh_notify_init(&c); + if (!c.foreground) { if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0) die_perror("Failed to open /dev/null"); @@ -414,6 +418,9 @@ loop: case EPOLL_TYPE_REPAIR: repair_handler(&c, eventmask); break; + case EPOLL_TYPE_NL_NEIGH: + nl_neigh_notify_handler(&c); + break; default: /* Can't happen */ ASSERT(0); -- 2.50.1

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

On Thu, Oct 02, 2025 at 08:34:05PM -0400, Jon Maloy wrote:

We add a cache table to keep track of the contents of the kernel ARP and NDP tables. The table is fed from the just introduced netlink based neigbour subscription function. The new table eliminates the need for explicit netlink calls to find a host's MAC address.

Signed-off-by: Jon Maloy

--- v5: - Moved to earlier in series to reduce rebase conflicts v6: - Sqashed the hash list commit and the FIFO/LRU queue commit - Removed hash lookup. We now only use linear lookup in a linked list - Eliminated dynamic memory allocation. - Ensured there is only one call to clock_gettime() - Using MAC_ZERO instead of the previously dedicated definitions v7: - NOW using MAC_ZERO where needed - I am still using linear back-off for empty cache entries. Even an incoming, flow-creating packet from a local host gives no guarantee that its MAC address is in the ARP table, so we must allow for a few new attempts at first possible occasions. Only after several failed lookups can we conclude that we probably never will succeed. Hence the back-off. - Fixed a bug that David inadvertently made me aware of: I only intended to set the initial expiry value to MAC_CACHE_RENEWAL when an ARP/NDP table lookup was successful. - Improved struct and function description comments. v8: - Total re-design of table, adapting to the new, subscription based way of updating it. v9: - Catering for MAC address change for an existing host. v10: - Changes according to feedback from David Gibson v12: - Changes according to feedback from David and Stefano - Added dummy entries for loopback and default GW addresses --- conf.c | 1 + fwd.c | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ fwd.h | 7 +++ netlink.c | 7 ++- 4 files changed, 195 insertions(+), 2 deletions(-)

diff --git a/conf.c b/conf.c index 66b9e63..3224ee6 100644 --- a/conf.c +++ b/conf.c [snip] +/** + * fwd_neigh_table_update() - Allocate or update neighbour table entry + * @c: Execution context + * @addr: IP address used to determine insertion slot and store in entry + * @mac: The MAC address associated with the neighbour address + */ +void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr, + const uint8_t *mac) +{ + struct neigh_table *t = &neigh_table; + struct neigh_table_entry *e; + ssize_t slot; + + /* MAC address might change sometimes */ + e = fwd_neigh_table_find(c, addr); + if (e) { + if (inany_equals(addr, &inany_from_v4(c->ip4.guest_gw))) + return; + if (inany_equals(addr, (union inany_addr *)&c->ip6.guest_gw)) + return;

This doesn't make sense to me. From the way its looked up in 4/9, the IP addresses in the table are as seen by the host, not by the guest (we look up the table *after* applying NAT). Which makes guest_gw not meaningful here. You _do_ need to handle the case that addr is loopback (which guest_gw might be translated to), and that's handled by your dummy entries. The other case we might need to consider here is if the (translated) address is the host's IP. Do we want to give out_tap_addr for that case? Or the host's real MAC on the template interface? If the latter will that be in the host ARP table, or would we have to handle it specially?

+ + memcpy(e->mac, mac, ETH_ALEN); + return; + } + + e = t->free; + if (!e) { + debug("Failed to allocate neighbour table entry"); + return; + } + t->free = e->next; + + slot = neigh_table_slot(c, addr); + e->next = t->slots[slot]; + t->slots[slot] = e; + + memcpy(&e->addr, addr, sizeof(*addr)); + memcpy(e->mac, mac, ETH_ALEN); +} + +/** + * fwd_neigh_table_free() - Remove an entry from a slot and add it to free list + * @c: Execution context + * @addr: IP address used to find the slot for the entry + */ +void fwd_neigh_table_free(const struct ctx *c, const union inany_addr *addr) +{ + ssize_t slot = neigh_table_slot(c, addr); + struct neigh_table *t = &neigh_table; + struct neigh_table_entry *e, **prev; + + prev = &t->slots[slot]; + e = t->slots[slot]; + while (e && !inany_equals(&e->addr, addr)) { + prev = &e->next; + e = e->next; + } + if (!e) + return; + + *prev = e->next; + e->next = t->free; + t->free = e; + memset(&e->addr, 0, sizeof(*addr)); + memset(e->mac, 0, ETH_ALEN);

As Stefano noted earlier, 0xff is probably a better placeholder here, since 00:00:00:00:00:00 is a valid MAC.

+} + +/** + * fwd_neigh_mac_get() - Look up MAC address in the ARP/NDP table + * @c: Execution context + * @addr: Neighbour IP address used as lookup key + * @mac: Buffer for Ethernet MAC to return, found or default value. + * + * Return: true if real MAC found, false if not found or if failure + */ +bool fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, + uint8_t *mac) +{ + const struct neigh_table_entry *e = fwd_neigh_table_find(c, addr); + + if (e) + memcpy(mac, e->mac, ETH_ALEN); + else + memcpy(mac, c->our_tap_mac, ETH_ALEN); + + return !!e; +} + +/** + * fwd_neigh_table_init() - Initialize the neighbour table + * @c: Execution context + */ +void fwd_neigh_table_init(const struct ctx *c) +{ + struct neigh_table *t = &neigh_table; + const uint8_t *omac = c->our_tap_mac; + struct neigh_table_entry *e; + int i; + + memset(t, 0, sizeof(*t)); + for (i = 0; i < NEIGH_TABLE_SIZE; i++) { + e = &t->entries[i]; + e->next = t->free; + t->free = e; + } + + /* These addresses must always map to our own MAC address */ + fwd_neigh_table_update(c, &inany_loopback4, omac); + fwd_neigh_table_update(c, &inany_loopback6, omac);

These make sense.

+ fwd_neigh_table_update(c, &inany_from_v4(c->ip4.guest_gw), omac); + fwd_neigh_table_update(c, (union inany_addr *)&c->ip6.guest_gw, omac);

But these don't. For two reasons: * As noted guest_gw is only meaningful guest side, but the table is based on host side addresses. * These will be no-ops, since you explicitly exclude these addresses in fwd_neigh_table_update().

+} + /** fwd_probe_ephemeral() - Determine what ports this host considers ephemeral * * Work out what ports the host thinks are emphemeral and record it for later diff --git a/fwd.h b/fwd.h index 65c7c96..6ca743c 100644 --- a/fwd.h +++ b/fwd.h @@ -56,5 +56,12 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); +void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr, + const uint8_t *mac); +void fwd_neigh_table_free(const struct ctx *c, + const union inany_addr *addr); +bool fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, + uint8_t *mac); +void fwd_neigh_table_init(const struct ctx *c);

#endif /* FWD_H */ diff --git a/netlink.c b/netlink.c index 3fe2fdd..4be5fcf 100644 --- a/netlink.c +++ b/netlink.c @@ -1192,10 +1192,13 @@ static void nl_neigh_msg_read(const struct ctx *c, struct nlmsghdr *nh) inany_from_af(&addr, ndm->ndm_family, dst); inany_ntop(dst, ip_str, sizeof(ip_str));

- if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) + if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) { trace("neigh table update: %s / %s", ip_str, mac_str); - else + fwd_neigh_table_update(c, &addr, mac); + } else { trace("neigh table delete: %s / %s", ip_str, mac_str); + fwd_neigh_table_free(c, &addr); + } }

/** -- 2.50.1

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

On Thu, Oct 02, 2025 at 08:34:07PM -0400, Jon Maloy wrote:

When we receive an ARP request or NDP neigbour solicitation over the tap interface for a host on the local network segment attached to the template interface, we respond with that host's real MAC address, if available.

Signed-off-by: Jon Maloy Reviewed-by: David Gibson

--- v3: - Added helper function to find out if a remote ip address is subject to NAT. This filters out local host addresses which should be presented with the passt/pasta local MAC address 9a:55:9a:55:9a:55 even though it is on the local segment. - Adapted to the change in nl_mac_get() function, so that we now consider only the template interface when checking the ARP/NDP table. v4: - Moved NAT check into the function nat_outbound() to obtain more precise criteria for when NAT is used. We may in theory have NAT even if original and translated addresses are equal, and we want to catch this case. - I chose to keep the wrapper funtion inany_nat(), but moved it to fwd.h/fwd.c and renamed it to fwd_inany_nat(). v5: - Simplified criteria for when we do ARP/NDP lookup. Now, we just try with the potentially translated address after an attempted NAT check. - Using the new ARP/NDP cache table instead of using netlink directly. v6: - Fixes after feedback from David: - Renamed nat_outbound() to fwd_nat_outbound() - Eliminated unnecessary temporary variable in arp.c::arp() v12: - Some minor changes after feedback from Stefano --- arp.c | 9 +++++++-- fwd.c | 8 ++++---- fwd.h | 2 ++ inany.c | 1 + ndp.c | 8 ++++++++ 5 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/arp.c b/arp.c index b08780f..a95c377 100644 --- a/arp.c +++ b/arp.c @@ -69,6 +69,7 @@ static bool ignore_arp(const struct ctx *c, */ int arp(const struct ctx *c, struct iov_tail *data) { + union inany_addr tgt, tgt_nat; struct { struct ethhdr eh; struct arphdr ah; @@ -102,8 +103,12 @@ int arp(const struct ctx *c, struct iov_tail *data) resp.ah.ar_hln = ah->ar_hln; resp.ah.ar_pln = ah->ar_pln;

- /* ARP message */ - memcpy(resp.am.sha, c->our_tap_mac, sizeof(resp.am.sha)); + /* MAC address to return in ARP message */ + inany_from_af(&tgt, AF_INET, am->tip); + fwd_nat_outbound(c, &tgt, &tgt_nat); + fwd_neigh_mac_get(c, &tgt_nat, resp.am.sha); + /* Rest of ARP message */ memcpy(resp.am.sip, am->tip, sizeof(resp.am.sip)); memcpy(resp.am.tha, am->sha, sizeof(resp.am.tha)); memcpy(resp.am.tip, am->sip, sizeof(resp.am.tip)); diff --git a/fwd.c b/fwd.c index ade97c8..a69d84d 100644 --- a/fwd.c +++ b/fwd.c @@ -522,7 +522,7 @@ static bool fwd_guest_accessible(const struct ctx *c, }

/** - * nat_outbound() - Apply address translation for outbound (TAP to HOST) + * fwd_nat_outbound() - Apply address translation for outbound (TAP to HOST) * @c: Execution context * @addr: Input address (as seen on TAP interface) * @translated: Output address (as seen on HOST interface) @@ -530,8 +530,8 @@ static bool fwd_guest_accessible(const struct ctx *c, * Only handles translations that depend *only* on the address. Anything * related to specific ports or flows is handled elsewhere. */ -static void nat_outbound(const struct ctx *c, const union inany_addr *addr, - union inany_addr *translated) +void fwd_nat_outbound(const struct ctx *c, const union inany_addr *addr, + union inany_addr *translated) { if (inany_equals4(addr, &c->ip4.map_host_loopback)) *translated = inany_loopback4; @@ -565,7 +565,7 @@ uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, inany_equals6(&ini->oaddr, &c->ip6.dns_match)) tgt->eaddr.a6 = c->ip6.dns_host; else - nat_outbound(c, &ini->oaddr, &tgt->eaddr); + fwd_nat_outbound(c, &ini->oaddr, &tgt->eaddr);

tgt->eport = ini->oport;

diff --git a/fwd.h b/fwd.h index 6ca743c..f91fa55 100644 --- a/fwd.h +++ b/fwd.h @@ -50,6 +50,8 @@ void fwd_scan_ports_init(struct ctx *c);

bool nat_inbound(const struct ctx *c, const union inany_addr *addr, union inany_addr *translated); +void fwd_nat_outbound(const struct ctx *c, const union inany_addr *addr, + union inany_addr *translated); uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto, diff --git a/inany.c b/inany.c index 65a39f9..7680439 100644 --- a/inany.c +++ b/inany.c @@ -16,6 +16,7 @@ #include "ip.h" #include "siphash.h" #include "inany.h" +#include "fwd.h"

const union inany_addr inany_loopback4 = INANY_INIT4(IN4ADDR_LOOPBACK_INIT); const union inany_addr inany_any4 = INANY_INIT4(IN4ADDR_ANY_INIT); diff --git a/ndp.c b/ndp.c index d7f64a3..6bba0a8 100644 --- a/ndp.c +++ b/ndp.c @@ -196,6 +196,7 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst, static void ndp_na(const struct ctx *c, const struct in6_addr *dst, const struct in6_addr *addr) { + union inany_addr tgt, tgt_nat; struct ndp_na na = { .ih = { .icmp6_type = NA, @@ -215,6 +216,13 @@ static void ndp_na(const struct ctx *c, const struct in6_addr *dst,

memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);

+ /* Respond with recorded MAC address if remote host's address + * or NAT translated address can be found in NDP table. + */ + inany_from_af(&tgt, AF_INET6, addr); + fwd_nat_outbound(c, &tgt, &tgt_nat); + fwd_neigh_mac_get(c, &tgt_nat, na.target_l2_addr.mac);

You do indeed need nat_outbound() here, and as noted on the previous patch you need nat_inbound() for unsolicited NAs triggered by host neighbour updates. But you want to avoid doing both an inbound and outbound NAT for the same operation, which might require some restructuring here. Or, I guess, you could convert the whole neighbour table to be in terms of guest side addresses, in which case you'd remove the NAT here, and instead need a nat_inbound() before you insert things into the table.

ndp_send(c, dst, &na, sizeof(na)); }

-- 2.50.1

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

In the title: s/subsciption/subscription/ On Thu, 2 Oct 2025 20:34:04 -0400 Jon Maloy wrote:

The solution to bug https://bugs.passt.top/show_bug.cgi?id=120 requires the ability to translate from an IP address to its corresponding MAC address in cases where those are present in the ARP or NDP tables.

To keep track of the contents of these tables we add a netlink based neighbour subscription feature.

Signed-off-by: Jon Maloy Reviewed-by: David Gibson

--- v3: - Added an attribute contianing NDA_DST to sent message, so that we let the kernel do the filtering of the IP address and return only one entry. - Added interface index to the call signature. Since the only interface we know is the template interface, this limits the number of hosts that will be seen as 'network segment local' from a PASST viewpoint. v4: - Made loop independent of attribute order. - Ignoring L2 addresses which are not of size ETH_ALEN. v5: - Changed return value of new function, so caller can know if a MAC address really was found. v6: - Removed warning printout which had ended up in the wrong commit. v8: - Changed to neighbour event subscription model - netlink: arp/ndp table subscription v10:- Updated according to David's latest comments on v8 - Added functionaly where we initially read current state of ARP/NDP tables v12:- Updates based on feedback from David and Stefano --- epoll_type.h | 2 + netlink.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++- netlink.h | 4 + passt.c | 7 ++ 4 files changed, 214 insertions(+), 3 deletions(-)

diff --git a/epoll_type.h b/epoll_type.h index 12ac59b..a90ffb6 100644 --- a/epoll_type.h +++ b/epoll_type.h @@ -44,6 +44,8 @@ enum epoll_type { EPOLL_TYPE_REPAIR_LISTEN, /* TCP_REPAIR helper socket */ EPOLL_TYPE_REPAIR, + /* Netlink neighbour subscription socket */ + EPOLL_TYPE_NL_NEIGH,

EPOLL_NUM_TYPES, }; diff --git a/netlink.c b/netlink.c index c436780..3fe2fdd 100644 --- a/netlink.c +++ b/netlink.c @@ -26,6 +26,7 @@ #include #include #include +#include

#include #include @@ -40,6 +41,10 @@ #define RTNH_NEXT_AND_DEC(rtnh, attrlen) \ ((attrlen) -= RTNH_ALIGN((rtnh)->rtnh_len), RTNH_NEXT(rtnh))

+/* Convenience macro borrowed from kernel */ +#define NUD_VALID \ + (NUD_PERMANENT | NUD_NOARP | NUD_REACHABLE | NUD_PROBE | NUD_STALE) + /* Netlink expects a buffer of at least 8kiB or the system page size, * whichever is larger. 32kiB is recommended for more efficient. * Since the largest page size on any remotely common Linux setup is @@ -50,9 +55,10 @@ #define NLBUFSIZ 65536

/* Socket in init, in target namespace, sequence (just needs to be monotonic) */ -int nl_sock = -1; -int nl_sock_ns = -1; -static int nl_seq = 1; +int nl_sock = -1; +int nl_sock_ns = -1; +static int nl_sock_neigh = -1; +static int nl_seq = 1;

/** * nl_sock_init_do() - Set up netlink sockets in init or target namespace @@ -1103,3 +1109,195 @@ int nl_link_set_flags(int s, unsigned int ifi,

return nl_do(s, &req, RTM_NEWLINK, 0, sizeof(req)); } + +/** + * nl_neigh_msg_read() - Interpret a neigbour state message from netlink + * @c: Execution context + * @nh: Message to be read + *

Nit: excess newline.

+ */ +static void nl_neigh_msg_read(const struct ctx *c, struct nlmsghdr *nh) +{ + struct ndmsg *ndm = NLMSG_DATA(nh); + struct rtattr *rta = (struct rtattr *)(ndm + 1); + size_t na = NLMSG_PAYLOAD(nh, sizeof(*ndm)); + char ip_str[INET6_ADDRSTRLEN]; + char mac_str[ETH_ADDRSTRLEN]; + const uint8_t *lladdr = NULL; + const void *dst = NULL; + size_t lladdr_len = 0; + uint8_t mac[ETH_ALEN]; + union inany_addr addr; + size_t dstlen = 0; + + if (nh->nlmsg_type == NLMSG_DONE) + return; + + if (nh->nlmsg_type == NLMSG_ERROR) { + warn_perror("nlmsg_type error at msg read"); + return; + } + + if (nh->nlmsg_type != RTM_NEWNEIGH && + nh->nlmsg_type != RTM_DELNEIGH) + return; + + for (; RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { + switch (rta->rta_type) { + case NDA_DST: + dst = RTA_DATA(rta); + dstlen = RTA_PAYLOAD(rta); + break; + case NDA_LLADDR: + lladdr = RTA_DATA(rta); + lladdr_len = RTA_PAYLOAD(rta); + break; + default: + break; + } + } + + if (!dst) + return; + + if (!lladdr || lladdr_len != ETH_ALEN) + return; + + if (ndm->ndm_type != ARPHRD_ETHER) + return; + + memcpy(mac, lladdr, ETH_ALEN); + eth_ntop(mac, mac_str, sizeof(mac_str)); + + if (ndm->ndm_family == AF_INET && + ndm->ndm_ifindex != c->ifi4) + return; + + if (ndm->ndm_family == AF_INET6 && + ndm->ndm_ifindex != c->ifi6) + return; + + if (ndm->ndm_family != AF_INET && + ndm->ndm_family != AF_INET6) + return; + + if (ndm->ndm_family == AF_INET && + dstlen != sizeof(struct in_addr)) + return; + + if (ndm->ndm_family == AF_INET6 && + dstlen != sizeof(struct in6_addr)) + return; + + inany_from_af(&addr, ndm->ndm_family, dst); + inany_ntop(dst, ip_str, sizeof(ip_str)); + + if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) + trace("neigh table update: %s / %s", ip_str, mac_str); + else + trace("neigh table delete: %s / %s", ip_str, mac_str); +} + +/** + * nl_neigh_sync() - Read current contents ARP/NDP tables + * @c: Execution context + * @proto: Protocol, AF_INET or AF_INET6 + * @ifi: Interface index + * + */ +static void nl_neigh_sync(const struct ctx *c, int proto, int ifi) +{ + struct { + struct nlmsghdr nlh; + struct ndmsg ndm; + } req = { + .nlh = {0}, + .ndm.ndm_family = proto, + .ndm.ndm_ifindex = ifi, + .ndm.ndm_state = 0, + .ndm.ndm_flags = 0, + .ndm.ndm_type = 0 + }; + struct nlmsghdr *nh; + char buf[NLBUFSIZ]; + ssize_t status; + uint32_t seq; + + seq = nl_send(nl_sock_neigh, &req, RTM_GETNEIGH, NLM_F_DUMP, sizeof(req)); + nl_foreach_oftype(nh, status, nl_sock_neigh, buf, seq, RTM_NEWNEIGH) + nl_neigh_msg_read(c, nh); + if (status < 0) + warn("Failed to read netlink message. status == %li", status);

That should be a human-readable message, so I'm not sure if the C notation for equality belongs there. In general we use the same format as perror() (you can't use warn_perror() here) that is, "Failed to read netlink message: %s", strerror_(status)); -- Stefano

On Thu, 2 Oct 2025 20:34:05 -0400 Jon Maloy wrote:

We add a cache table to keep track of the contents of the kernel ARP and NDP tables. The table is fed from the just introduced netlink based neigbour subscription function. The new table eliminates the need for explicit netlink calls to find a host's MAC address.

Signed-off-by: Jon Maloy

--- v5: - Moved to earlier in series to reduce rebase conflicts v6: - Sqashed the hash list commit and the FIFO/LRU queue commit - Removed hash lookup. We now only use linear lookup in a linked list - Eliminated dynamic memory allocation. - Ensured there is only one call to clock_gettime() - Using MAC_ZERO instead of the previously dedicated definitions v7: - NOW using MAC_ZERO where needed - I am still using linear back-off for empty cache entries. Even an incoming, flow-creating packet from a local host gives no guarantee that its MAC address is in the ARP table, so we must allow for a few new attempts at first possible occasions. Only after several failed lookups can we conclude that we probably never will succeed. Hence the back-off. - Fixed a bug that David inadvertently made me aware of: I only intended to set the initial expiry value to MAC_CACHE_RENEWAL when an ARP/NDP table lookup was successful. - Improved struct and function description comments. v8: - Total re-design of table, adapting to the new, subscription based way of updating it. v9: - Catering for MAC address change for an existing host. v10: - Changes according to feedback from David Gibson v12: - Changes according to feedback from David and Stefano - Added dummy entries for loopback and default GW addresses --- conf.c | 1 + fwd.c | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ fwd.h | 7 +++ netlink.c | 7 ++- 4 files changed, 195 insertions(+), 2 deletions(-)

diff --git a/conf.c b/conf.c index 66b9e63..3224ee6 100644 --- a/conf.c +++ b/conf.c @@ -2133,6 +2133,7 @@ void conf(struct ctx *c, int argc, char **argv) c->udp.fwd_out.mode = fwd_default;

fwd_scan_ports_init(c); + fwd_neigh_table_init(c);

if (!c->quiet) conf_print(c); diff --git a/fwd.c b/fwd.c index 250cf56..c34bb1c 100644 --- a/fwd.c +++ b/fwd.c @@ -33,6 +33,188 @@ static in_port_t fwd_ephemeral_max = NUM_PORTS - 1;

#define PORT_RANGE_SYSCTL "/proc/sys/net/ipv4/ip_local_port_range"

+#define NEIGH_TABLE_SLOTS 1024 /* Must be power of two */ +#define NEIGH_TABLE_SIZE (NEIGH_TABLE_SLOTS / 2) +static_assert((NEIGH_TABLE_SLOTS & (NEIGH_TABLE_SLOTS - 1)) == 0, + "NEIGH_TABLE_SLOTS must be a power of two"); + +/** + * struct neigh_table_entry - Entry in the ARP/NDP table

Nit: excess whitespace before "Entry".

+ * @next: Next entry in slot or free list + * @addr: IP address of represented host + * @mac: MAC address of represented host + */ +struct neigh_table_entry { + struct neigh_table_entry *next; + union inany_addr addr; + uint8_t mac[ETH_ALEN]; +}; + +/** + * struct neigh_table - Cache of ARP/NDP table contents

Same here.

+ * @entries: Entries to be plugged into the hash slots when allocated + * @slots: Hash table slots + * @free: Linked list of unused entries + */ +struct neigh_table { + struct neigh_table_entry entries[NEIGH_TABLE_SIZE]; + struct neigh_table_entry *slots[NEIGH_TABLE_SLOTS]; + struct neigh_table_entry *free; +}; + +static struct neigh_table neigh_table; + +/** + * neigh_table_slot() - Hash key to a number within the table range + * @c: Execution context + * @key: The key to be used for the hash + * + * Return: The resulting hash value

Nit: s/The/the/, it's part of the same statement / sentence.

+ */ +static size_t neigh_table_slot(const struct ctx *c, + const union inany_addr *key) +{ + struct siphash_state st = SIPHASH_INIT(c->hash_secret); + uint32_t i; + + inany_siphash_feed(&st, key); + i = siphash_final(&st, sizeof(*key), 0); + + return ((size_t)i) & (NEIGH_TABLE_SIZE - 1); +} + +/** + * fwd_neigh_table_find() - Find a MAC table entry + * @c: Execution context + * @addr: Neighbour address to be used as key for the lookup + * + * Return: The matching entry, if found. Otherwise NULL.

Same here.

+ */ +static struct neigh_table_entry *fwd_neigh_table_find(const struct ctx *c, + const union inany_addr *addr) +{ + size_t slot = neigh_table_slot(c, addr); + struct neigh_table_entry *e = neigh_table.slots[slot]; + + while (e && !inany_equals(&e->addr, addr)) + e = e->next; + + return e; +} + +/** + * fwd_neigh_table_update() - Allocate or update neighbour table entry + * @c: Execution context + * @addr: IP address used to determine insertion slot and store in entry + * @mac: The MAC address associated with the neighbour address + */ +void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr, + const uint8_t *mac) +{ + struct neigh_table *t = &neigh_table; + struct neigh_table_entry *e; + ssize_t slot; + + /* MAC address might change sometimes */ + e = fwd_neigh_table_find(c, addr); + if (e) { + if (inany_equals(addr, &inany_from_v4(c->ip4.guest_gw))) + return; + if (inany_equals(addr, (union inany_addr *)&c->ip6.guest_gw)) + return; + + memcpy(e->mac, mac, ETH_ALEN); + return; + } + + e = t->free; + if (!e) { + debug("Failed to allocate neighbour table entry"); + return; + } + t->free = e->next; + + slot = neigh_table_slot(c, addr); + e->next = t->slots[slot]; + t->slots[slot] = e; + + memcpy(&e->addr, addr, sizeof(*addr)); + memcpy(e->mac, mac, ETH_ALEN); +} + +/** + * fwd_neigh_table_free() - Remove an entry from a slot and add it to free list + * @c: Execution context + * @addr: IP address used to find the slot for the entry + */ +void fwd_neigh_table_free(const struct ctx *c, const union inany_addr *addr) +{ + ssize_t slot = neigh_table_slot(c, addr); + struct neigh_table *t = &neigh_table; + struct neigh_table_entry *e, **prev; + + prev = &t->slots[slot]; + e = t->slots[slot]; + while (e && !inany_equals(&e->addr, addr)) { + prev = &e->next; + e = e->next; + } + if (!e) + return; + + *prev = e->next; + e->next = t->free; + t->free = e; + memset(&e->addr, 0, sizeof(*addr)); + memset(e->mac, 0, ETH_ALEN); +} + +/** + * fwd_neigh_mac_get() - Look up MAC address in the ARP/NDP table + * @c: Execution context + * @addr: Neighbour IP address used as lookup key + * @mac: Buffer for Ethernet MAC to return, found or default value.

Nit: trailing '.' The whole description isn't really clear to me. It sounds like there are three possible alternatives, that is, "return", "found", or "default". Maybe simply: Buffer for returned MAC address ?

+ * + * Return: true if real MAC found, false if not found or if failure

This is a MAC address, not the media access control itself (you can't really... find it).

+ */ +bool fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, + uint8_t *mac)

As I pointed out on v11: you seem to be ignoring the return value everywhere in this series, so returning a value here seems to be a left-over. I'm not reviewing past this point.

+{ + const struct neigh_table_entry *e = fwd_neigh_table_find(c, addr); + + if (e) + memcpy(mac, e->mac, ETH_ALEN); + else + memcpy(mac, c->our_tap_mac, ETH_ALEN); + + return !!e; +} + +/** + * fwd_neigh_table_init() - Initialize the neighbour table + * @c: Execution context + */ +void fwd_neigh_table_init(const struct ctx *c) +{ + struct neigh_table *t = &neigh_table; + const uint8_t *omac = c->our_tap_mac; + struct neigh_table_entry *e; + int i; + + memset(t, 0, sizeof(*t)); + for (i = 0; i < NEIGH_TABLE_SIZE; i++) { + e = &t->entries[i]; + e->next = t->free; + t->free = e; + } + + /* These addresses must always map to our own MAC address */ + fwd_neigh_table_update(c, &inany_loopback4, omac); + fwd_neigh_table_update(c, &inany_loopback6, omac); + fwd_neigh_table_update(c, &inany_from_v4(c->ip4.guest_gw), omac); + fwd_neigh_table_update(c, (union inany_addr *)&c->ip6.guest_gw, omac); +} + /** fwd_probe_ephemeral() - Determine what ports this host considers ephemeral * * Work out what ports the host thinks are emphemeral and record it for later diff --git a/fwd.h b/fwd.h index 65c7c96..6ca743c 100644 --- a/fwd.h +++ b/fwd.h @@ -56,5 +56,12 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt); +void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr, + const uint8_t *mac); +void fwd_neigh_table_free(const struct ctx *c, + const union inany_addr *addr); +bool fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, + uint8_t *mac); +void fwd_neigh_table_init(const struct ctx *c);

#endif /* FWD_H */ diff --git a/netlink.c b/netlink.c index 3fe2fdd..4be5fcf 100644 --- a/netlink.c +++ b/netlink.c @@ -1192,10 +1192,13 @@ static void nl_neigh_msg_read(const struct ctx *c, struct nlmsghdr *nh) inany_from_af(&addr, ndm->ndm_family, dst); inany_ntop(dst, ip_str, sizeof(ip_str));

- if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) + if (nh->nlmsg_type == RTM_NEWNEIGH && ndm->ndm_state & NUD_VALID) { trace("neigh table update: %s / %s", ip_str, mac_str); - else + fwd_neigh_table_update(c, &addr, mac); + } else { trace("neigh table delete: %s / %s", ip_str, mac_str); + fwd_neigh_table_free(c, &addr); + } }

/**

-- Stefano

On Fri, 3 Oct 2025 14:41:56 +1000 David Gibson wrote:

On Thu, Oct 02, 2025 at 08:34:06PM -0400, Jon Maloy wrote:

...

ARP announcements and unsolicited NAs should be handled with caution because of the risk of malignant users emitting them to disturb network communication.

There is however one case we where we know it is legitimate and safe for us to send out such messages: The one time we switch from using ctx->own_tap_mac to a MAC address received via the recently added neigbour subscription function. Later changes to the MAC address of a host in an existing entry cannot be fully trusted, so we abstain from doing it in such cases.

When sending this type of messages, we notice that the guest accepts the update, but shortly later asks for a confirmation in the form of a regular ARP/NS request. This is responded to with the new value, and we have exactly the effect we wanted.

This commit adds this functionality.

Signed-off-by: Jon Maloy

--- v10: -Made small changes based of feedback from David G. v11: -Moved from 'Gratuitous ARP reply' model to 'ARP Announcement' model. v12: -Excluding loopback and default GW addresses from the ARP/NA announcement to be sent to the guest --- arp.c | 42 ++++++++++++++++++++++++++++++++++++++++++ arp.h | 2 ++ fwd.c | 16 ++++++++++++++++ ndp.c | 10 ++++++++++ ndp.h | 1 + 5 files changed, 71 insertions(+)

diff --git a/arp.c b/arp.c index ad088b1..b08780f 100644 --- a/arp.c +++ b/arp.c @@ -146,3 +146,45 @@ void arp_send_init_req(const struct ctx *c) debug("Sending initial ARP request for guest MAC address"); tap_send_single(c, &req, sizeof(req)); } + +/** + * arp_announce() - Send an ARP announcement for an IPv4 host + * @c: Execution context + * @ip: IPv4 address we announce as owned by @mac + * @mac: MAC address to advertise for @ip + */ +void arp_announce(const struct ctx *c, struct in_addr *ip, + const unsigned char *mac) +{ + char ip_str[INET_ADDRSTRLEN]; + char mac_str[ETH_ADDRSTRLEN]; + struct { + struct ethhdr eh; + struct arphdr ah; + struct arpmsg am; + } __attribute__((__packed__)) annc; + + /* Ethernet header */ + annc.eh.h_proto = htons(ETH_P_ARP); + memcpy(annc.eh.h_dest, MAC_BROADCAST, sizeof(annc.eh.h_dest)); + memcpy(annc.eh.h_source, mac, sizeof(annc.eh.h_source)); + + /* ARP header */ + annc.ah.ar_op = htons(ARPOP_REQUEST); + annc.ah.ar_hrd = htons(ARPHRD_ETHER); + annc.ah.ar_pro = htons(ETH_P_IP); + annc.ah.ar_hln = ETH_ALEN; + annc.ah.ar_pln = 4; + + /* ARP message */ + memcpy(annc.am.sha, mac, sizeof(annc.am.sha)); + memcpy(annc.am.sip, ip, sizeof(annc.am.sip)); + memcpy(annc.am.tha, MAC_BROADCAST, sizeof(annc.am.tha)); + memcpy(annc.am.tip, ip, sizeof(annc.am.tip));

As noted in several earlier revisions, having sip == tip (but with different mac addresses) looks odd. Is that what the RFCs say to do for ARP announcements?

...

+ inet_ntop(AF_INET, ip, ip_str, sizeof(ip_str)); + eth_ntop(mac, mac_str, sizeof(mac_str)); + debug("Announcing ARP for %s / %s", ip_str, mac_str); + + tap_send_single(c, &annc, sizeof(annc)); +} diff --git a/arp.h b/arp.h index d5ad0e1..4862e90 100644 --- a/arp.h +++ b/arp.h @@ -22,5 +22,7 @@ struct arpmsg {

int arp(const struct ctx *c, struct iov_tail *data); void arp_send_init_req(const struct ctx *c); +void arp_announce(const struct ctx *c, struct in_addr *ip, + const unsigned char *mac);

#endif /* ARP_H */ diff --git a/fwd.c b/fwd.c index c34bb1c..ade97c8 100644 --- a/fwd.c +++ b/fwd.c @@ -26,6 +26,8 @@ #include "passt.h" #include "lineread.h" #include "flow_table.h" +#include "arp.h" +#include "ndp.h"

/* Empheral port range: values from RFC 6335 */ static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); @@ -140,6 +142,20 @@ void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr,

memcpy(&e->addr, addr, sizeof(*addr)); memcpy(e->mac, mac, ETH_ALEN); + + if (inany_equals(addr, &inany_loopback4)) + return; + if (inany_equals(addr, &inany_loopback6)) + return;

Since you need these explicit checks anyway, there's not much point to the dummy entries you created - you could exit on these addresses before even looking up the table.

I guess those entries make sense if we can drop all these checks as a result. I think we should be able to. But if we can't, then yes, let's just go with explicit checks everywhere, so that it's consistent. -- Stefano

On Wed, 8 Oct 2025 11:27:32 +1100 David Gibson wrote:

On Tue, Oct 07, 2025 at 12:10:22PM +0200, Stefano Brivio wrote:

...

On Fri, 3 Oct 2025 14:41:56 +1000 David Gibson wrote:

...

On Thu, Oct 02, 2025 at 08:34:06PM -0400, Jon Maloy wrote:

...

ARP announcements and unsolicited NAs should be handled with caution because of the risk of malignant users emitting them to disturb network communication.

There is however one case we where we know it is legitimate and safe for us to send out such messages: The one time we switch from using ctx->own_tap_mac to a MAC address received via the recently added neigbour subscription function. Later changes to the MAC address of a host in an existing entry cannot be fully trusted, so we abstain from doing it in such cases.

When sending this type of messages, we notice that the guest accepts the update, but shortly later asks for a confirmation in the form of a regular ARP/NS request. This is responded to with the new value, and we have exactly the effect we wanted.

This commit adds this functionality.

Signed-off-by: Jon Maloy

--- v10: -Made small changes based of feedback from David G. v11: -Moved from 'Gratuitous ARP reply' model to 'ARP Announcement' model. v12: -Excluding loopback and default GW addresses from the ARP/NA announcement to be sent to the guest --- arp.c | 42 ++++++++++++++++++++++++++++++++++++++++++ arp.h | 2 ++ fwd.c | 16 ++++++++++++++++ ndp.c | 10 ++++++++++ ndp.h | 1 + 5 files changed, 71 insertions(+)

diff --git a/arp.c b/arp.c index ad088b1..b08780f 100644 --- a/arp.c +++ b/arp.c @@ -146,3 +146,45 @@ void arp_send_init_req(const struct ctx *c) debug("Sending initial ARP request for guest MAC address"); tap_send_single(c, &req, sizeof(req)); } + +/** + * arp_announce() - Send an ARP announcement for an IPv4 host + * @c: Execution context + * @ip: IPv4 address we announce as owned by @mac + * @mac: MAC address to advertise for @ip + */ +void arp_announce(const struct ctx *c, struct in_addr *ip, + const unsigned char *mac) +{ + char ip_str[INET_ADDRSTRLEN]; + char mac_str[ETH_ADDRSTRLEN]; + struct { + struct ethhdr eh; + struct arphdr ah; + struct arpmsg am; + } __attribute__((__packed__)) annc; + + /* Ethernet header */ + annc.eh.h_proto = htons(ETH_P_ARP); + memcpy(annc.eh.h_dest, MAC_BROADCAST, sizeof(annc.eh.h_dest)); + memcpy(annc.eh.h_source, mac, sizeof(annc.eh.h_source)); + + /* ARP header */ + annc.ah.ar_op = htons(ARPOP_REQUEST); + annc.ah.ar_hrd = htons(ARPHRD_ETHER); + annc.ah.ar_pro = htons(ETH_P_IP); + annc.ah.ar_hln = ETH_ALEN; + annc.ah.ar_pln = 4; + + /* ARP message */ + memcpy(annc.am.sha, mac, sizeof(annc.am.sha)); + memcpy(annc.am.sip, ip, sizeof(annc.am.sip)); + memcpy(annc.am.tha, MAC_BROADCAST, sizeof(annc.am.tha)); + memcpy(annc.am.tip, ip, sizeof(annc.am.tip));

As noted in several earlier revisions, having sip == tip (but with different mac addresses) looks odd. Is that what the RFCs say to do for ARP announcements?

...

+ inet_ntop(AF_INET, ip, ip_str, sizeof(ip_str)); + eth_ntop(mac, mac_str, sizeof(mac_str)); + debug("Announcing ARP for %s / %s", ip_str, mac_str); + + tap_send_single(c, &annc, sizeof(annc)); +} diff --git a/arp.h b/arp.h index d5ad0e1..4862e90 100644 --- a/arp.h +++ b/arp.h @@ -22,5 +22,7 @@ struct arpmsg {

int arp(const struct ctx *c, struct iov_tail *data); void arp_send_init_req(const struct ctx *c); +void arp_announce(const struct ctx *c, struct in_addr *ip, + const unsigned char *mac);

#endif /* ARP_H */ diff --git a/fwd.c b/fwd.c index c34bb1c..ade97c8 100644 --- a/fwd.c +++ b/fwd.c @@ -26,6 +26,8 @@ #include "passt.h" #include "lineread.h" #include "flow_table.h" +#include "arp.h" +#include "ndp.h"

/* Empheral port range: values from RFC 6335 */ static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); @@ -140,6 +142,20 @@ void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr,

memcpy(&e->addr, addr, sizeof(*addr)); memcpy(e->mac, mac, ETH_ALEN); + + if (inany_equals(addr, &inany_loopback4)) + return; + if (inany_equals(addr, &inany_loopback6)) + return;

Since you need these explicit checks anyway, there's not much point to the dummy entries you created - you could exit on these addresses before even looking up the table.

I guess those entries make sense if we can drop all these checks as a result. I think we should be able to.

We couldn't in this version, because that might have allowed the entries for loopback to be updated, which is certainly wrong. But it will all need re-examination after moving everything over to guest side addresses which AIUI is the plan for the next spin.

Yes, I was talking about the next version. For context, when we first discussed about the possibility of these entries with Jon, my assumption was that the whole series used guest-side link-layer addresses exclusively, but that wasn't the case, hence (I think) the current struggle. If we go in that direction, I hope it's possible. By the way, while they are probably more elegant because we can skip explicit cases, they might be a bit more complicated to manage compared to those explicit cases the day we get to change addresses and routes dynamically using a netlink monitor, because at that point we might need to remove some entries based on old addresses / default gateways. But given that this is already complicated enough, we can keep that problem for later, and just go with the simplest possible approach (whatever it is) for the moment. -- Stefano

On Thu, 9 Oct 2025 14:51:02 +1100 David Gibson wrote:

On Wed, Oct 08, 2025 at 12:01:18PM +0200, Stefano Brivio wrote:

...

On Wed, 8 Oct 2025 11:27:32 +1100 David Gibson wrote:

...

On Tue, Oct 07, 2025 at 12:10:22PM +0200, Stefano Brivio wrote:

...

On Fri, 3 Oct 2025 14:41:56 +1000 David Gibson wrote:

...

On Thu, Oct 02, 2025 at 08:34:06PM -0400, Jon Maloy wrote:

...

ARP announcements and unsolicited NAs should be handled with caution because of the risk of malignant users emitting them to disturb network communication.

There is however one case we where we know it is legitimate and safe for us to send out such messages: The one time we switch from using ctx->own_tap_mac to a MAC address received via the recently added neigbour subscription function. Later changes to the MAC address of a host in an existing entry cannot be fully trusted, so we abstain from doing it in such cases.

When sending this type of messages, we notice that the guest accepts the update, but shortly later asks for a confirmation in the form of a regular ARP/NS request. This is responded to with the new value, and we have exactly the effect we wanted.

This commit adds this functionality.

Signed-off-by: Jon Maloy

--- v10: -Made small changes based of feedback from David G. v11: -Moved from 'Gratuitous ARP reply' model to 'ARP Announcement' model. v12: -Excluding loopback and default GW addresses from the ARP/NA announcement to be sent to the guest --- arp.c | 42 ++++++++++++++++++++++++++++++++++++++++++ arp.h | 2 ++ fwd.c | 16 ++++++++++++++++ ndp.c | 10 ++++++++++ ndp.h | 1 + 5 files changed, 71 insertions(+)

diff --git a/arp.c b/arp.c index ad088b1..b08780f 100644 --- a/arp.c +++ b/arp.c @@ -146,3 +146,45 @@ void arp_send_init_req(const struct ctx *c) debug("Sending initial ARP request for guest MAC address"); tap_send_single(c, &req, sizeof(req)); } + +/** + * arp_announce() - Send an ARP announcement for an IPv4 host + * @c: Execution context + * @ip: IPv4 address we announce as owned by @mac + * @mac: MAC address to advertise for @ip + */ +void arp_announce(const struct ctx *c, struct in_addr *ip, + const unsigned char *mac) +{ + char ip_str[INET_ADDRSTRLEN]; + char mac_str[ETH_ADDRSTRLEN]; + struct { + struct ethhdr eh; + struct arphdr ah; + struct arpmsg am; + } __attribute__((__packed__)) annc; + + /* Ethernet header */ + annc.eh.h_proto = htons(ETH_P_ARP); + memcpy(annc.eh.h_dest, MAC_BROADCAST, sizeof(annc.eh.h_dest)); + memcpy(annc.eh.h_source, mac, sizeof(annc.eh.h_source)); + + /* ARP header */ + annc.ah.ar_op = htons(ARPOP_REQUEST); + annc.ah.ar_hrd = htons(ARPHRD_ETHER); + annc.ah.ar_pro = htons(ETH_P_IP); + annc.ah.ar_hln = ETH_ALEN; + annc.ah.ar_pln = 4; + + /* ARP message */ + memcpy(annc.am.sha, mac, sizeof(annc.am.sha)); + memcpy(annc.am.sip, ip, sizeof(annc.am.sip)); + memcpy(annc.am.tha, MAC_BROADCAST, sizeof(annc.am.tha)); + memcpy(annc.am.tip, ip, sizeof(annc.am.tip));

As noted in several earlier revisions, having sip == tip (but with different mac addresses) looks odd. Is that what the RFCs say to do for ARP announcements?

...

+ inet_ntop(AF_INET, ip, ip_str, sizeof(ip_str)); + eth_ntop(mac, mac_str, sizeof(mac_str)); + debug("Announcing ARP for %s / %s", ip_str, mac_str); + + tap_send_single(c, &annc, sizeof(annc)); +} diff --git a/arp.h b/arp.h index d5ad0e1..4862e90 100644 --- a/arp.h +++ b/arp.h @@ -22,5 +22,7 @@ struct arpmsg {

int arp(const struct ctx *c, struct iov_tail *data); void arp_send_init_req(const struct ctx *c); +void arp_announce(const struct ctx *c, struct in_addr *ip, + const unsigned char *mac);

#endif /* ARP_H */ diff --git a/fwd.c b/fwd.c index c34bb1c..ade97c8 100644 --- a/fwd.c +++ b/fwd.c @@ -26,6 +26,8 @@ #include "passt.h" #include "lineread.h" #include "flow_table.h" +#include "arp.h" +#include "ndp.h"

/* Empheral port range: values from RFC 6335 */ static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); @@ -140,6 +142,20 @@ void fwd_neigh_table_update(const struct ctx *c, const union inany_addr *addr,

memcpy(&e->addr, addr, sizeof(*addr)); memcpy(e->mac, mac, ETH_ALEN); + + if (inany_equals(addr, &inany_loopback4)) + return; + if (inany_equals(addr, &inany_loopback6)) + return;

Since you need these explicit checks anyway, there's not much point to the dummy entries you created - you could exit on these addresses before even looking up the table.

I guess those entries make sense if we can drop all these checks as a result. I think we should be able to.

We couldn't in this version, because that might have allowed the entries for loopback to be updated, which is certainly wrong. But it will all need re-examination after moving everything over to guest side addresses which AIUI is the plan for the next spin.

Yes, I was talking about the next version. For context, when we first discussed about the possibility of these entries with Jon, my assumption was that the whole series used guest-side link-layer addresses exclusively,

We did use guest-side link-layer addresses - host-side LL addresses might not even exist. The question is about whether we use guest side or host side IP addresses to index the table.

Sorry, yes, I meant to write network and I wrote link-layer.

...

but that wasn't the case, hence (I think) the current struggle. If we go in that direction, I hope it's possible.

Thinking a bit more closely, I don't think it is, for much the same reason it wasn't in this draft.

According to the rules Jon and I thrashed out elsewhere in the thread, there are certain guest side addresses that must be locked to use our_tap_mac. We're essentially shadowing something that might exist on the host side, so we should use our MAC not the MAC of whatever is shadowed.

Just pre-populating an entry won't do the trick, because it could be overwritten if the right events occur for the shadowed host.

Right, sorry, I omitted another bit of context: I've been suggesting to Jon that he'd introduce some kind of "permanent" or "administrative" bit, and keep those entries at the beginning of the chain, exactly for the reason you mention. I can imagine we'll need those at some point if we ever want to offer explicit link-layer address mapping in the future, and they're probably convenient the day one can change map_guest_addr and map_host_loopback at runtime. We can also happily skip that for the moment, though, it's another problem we can keep for later.

...

By the way, while they are probably more elegant because we can skip explicit cases, they might be a bit more complicated to manage compared to those explicit cases the day we get to change addresses and routes dynamically using a netlink monitor, because at that point we might need to remove some entries based on old addresses / default gateways.

But given that this is already complicated enough, we can keep that problem for later, and just go with the simplest possible approach (whatever it is) for the moment.

-- Stefano

-- Stefano