Re: [PATCH v9 9/9] arp/ndp: send gratuitous ARP / unsolicitated NA when MAC cache entry added

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]

On Fri, Sep 26, 2025 at 06:59:40PM -0400, Jon Maloy wrote:
> 
> 
> On 2025-09-25 20:47, David Gibson wrote:
> > On Thu, Sep 25, 2025 at 08:48:51AM -0400, Jon Maloy wrote:
> > > 
> > > 
> > > On 2025-09-25 02:38, David Gibson wrote:
> > > > On Wed, Sep 24, 2025 at 07:32:43PM -0400, Jon Maloy wrote:
> > > > > 
> > > > > 
> > > > > On 2025-09-24 18:18, Jon Maloy wrote:
> > > > > > 
> > > > > > 
> > > > > > On 2025-09-23 23:22, David Gibson wrote:
> > > > > > > On Tue, Sep 23, 2025 at 09:13:30PM -0400, Jon Maloy wrote:
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > > > --- 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);
> > > > > > > > @@ -129,6 +131,15 @@ void fwd_neigh_mac_cache_alloc(const struct ctx *c,
> > > > > > > >         memcpy(&e->addr, addr, sizeof(*addr));
> > > > > > > >         memcpy(e->mac, mac, ETH_ALEN);
> > > > > > > > +
> > > > > > > > +    /* Send gratuitous ARP / unsolicited NA for the new mapping */
> > > > > > > 
> > > > > > > AFAICT this doesn't actually implement what the commit message
> > > > > > > describes - it seems to always send an ARP/NA when the neighbour table
> > > > > > > is updated.
> > > > > 
> > > > > No. Check the code again.
> > > > 
> > > > Still not seeing it, I'm going to need a more specific pointer to what
> > > > I've missed.
> > > > 
> > > Look in fwd_neigh_mac_cache_alloc().
> > > If the entry already exists, the function updates the mac address just to be
> > > on the safe side, but then returns without sending any ARP/NA.
> > 
> > Uh... no, no it doesn't.  I applied all the patches on a branch and
> > here's what I see in fwd_neigh_mac_cache_alloc():
> > 
> > 	memcpy(&e->addr, addr, sizeof(*addr));
> > 	memcpy(e->mac, mac, ETH_ALEN);
> > 
> > 	/* Send gratuitous ARP / unsolicited NA for the new mapping */
> > 	if (inany_v4(addr)) {
> > 		struct in_addr ip4 = *inany_v4(addr);
> > 
> > 		arp_send_gratuitous(c, ip4, e->mac);
> > 	} else {
> > 		ndp_send_unsolicited_na(c, &addr->a6);
> > 	}
> > 
> > If it updates, it ARPs.  Do you have something in your tree that
> > didn't make it into the published patches?
> > 
> 
> This was introduced already in patch #1, not #9.
> The final function looks like this:
> 
> void fwd_neigh_mac_cache_alloc(const struct ctx *c,
>                                const union inany_addr *addr,
>                                uint8_t *mac)
> 
> {
>         struct mac_cache_table *t = &mac_cache;
>         struct mac_cache_entry *e;
>         ssize_t idx;
> 
>         /* MAC address might change sometimes */
>         e = fwd_mac_cache_find(c, addr);
>         if (e) {
>                 memcpy(e->mac, mac, ETH_ALEN);
> ====>           return;

Ah, yes, sorry.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]