Re: [PATCH v12 2/9] fwd: Add cache table for ARP/NDP contents

[Jon Maloy <jmaloy@redhat.com>]

On 2025-10-02 20:34, 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 <jmaloy@redhat.com>
> 
> ---
> 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(-)
> 

[...]

> +	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);
> +}
> +

Stefano, I know this isn't exactly what we agreed upon when we discussed 
this, but I really cannot see the guest address being handled anywhere
in a way that justifies adding it to the the neigbour table.
As far as I can see, the above, plus a fix in the next commit, solves
all the problems regarding host and default GW access.
Please correct me if I am wrong.

///jon