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

[Jon Maloy <jmaloy@redhat.com>]

On 2025-09-23 23:22, David Gibson wrote:
> On Tue, Sep 23, 2025 at 09:13:30PM -0400, Jon Maloy wrote:
>> Gratuitious ARP and unsolicitated NA should be handled with caution
>> because of the risk of malignant users emitting them to disturb
>> network communication.
>>
[...]
>> +	req.ah.ar_op = htons(ARPOP_REPLY);
>> +	req.ah.ar_hrd = htons(ARPHRD_ETHER);
>> +	req.ah.ar_pro = htons(ETH_P_IP);
>> +	req.ah.ar_hln = ETH_ALEN;
>> +	req.ah.ar_pln = 4;
>> +
>> +	/* ARP message */
>> +	memcpy(req.am.sha, mac, sizeof(req.am.sha));
>> +	memcpy(req.am.sip, &ip, sizeof(req.am.sip));
>> +	memcpy(req.am.tha, MAC_BROADCAST, sizeof(req.am.tha));
>> +	memcpy(req.am.tip, &ip, sizeof(req.am.tip));
> 
> So, I was trying to check if it made sense to use the same IP for both
> source and target here, and came across
>      https://www.rfc-editor.org/rfc/rfc5227#section-3
> 
> Which suggests we should (counter intuitively) be using ARP requests,
> not ARP replies for announcements.

I have now read through it, and it seems to come to the conclusion
that this is not advisable. In principle it should work, if all 
implementations stick to standard, but there might be stacks which are 
not stateless in this regard, i.e., they only accepts ARP replies as a 
response to a sent request.
In short, I think I will stick to my current approach, since it is 
evidently harmless and is proven to work.

///jon


> 
>> +	inet_ntop(AF_INET, &ip, ip_str, sizeof(ip_str));
>> +	debug("Sending gratuitous ARP for %s", ip_str);
>> +	tap_send_single(c, &req, sizeof(req));
>> +}
>> diff --git a/arp.h b/arp.h
>> index d5ad0e1..b0dbb56 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_send_gratuitous(const struct ctx *c, struct in_addr ip,
>> +			 const unsigned char *mac);
>>   
>>   #endif /* ARP_H */
>> diff --git a/fwd.c b/fwd.c
>> index c6348ab..879a351 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);
>> @@ -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.
> 
>> +	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);
>> +	}
>>   }
>>   
>>   /**
>> diff --git a/ndp.c b/ndp.c
>> index 70b68aa..8914f31 100644
>> --- a/ndp.c
>> +++ b/ndp.c
>> @@ -226,6 +226,16 @@ static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
>>   	ndp_send(c, dst, &na, sizeof(na));
>>   }
>>   
>> +/**
>> + * ndp_send_unsolicited_na() - Send unsolicited NA
>> + * @c:		Execution context
>> + * @addr:	IPv6 address to advertise
>> + */
>> +void ndp_send_unsolicited_na(const struct ctx *c, const struct in6_addr *addr)
>> +{
>> +	ndp_na(c, &in6addr_ll_all_nodes, addr);
>> +}
>> +
>>   /**
>>    * ndp_ra() - Send an NDP Router Advertisement (RA) message
>>    * @c:		Execution context
>> diff --git a/ndp.h b/ndp.h
>> index 781ea86..320009c 100644
>> --- a/ndp.h
>> +++ b/ndp.h
>> @@ -12,5 +12,6 @@ int ndp(const struct ctx *c, const struct in6_addr *saddr,
>>   	struct iov_tail *data);
>>   void ndp_timer(const struct ctx *c, const struct timespec *now);
>>   void ndp_send_init_req(const struct ctx *c);
>> +void ndp_send_unsolicited_na(const struct ctx *c, const struct in6_addr *addr);
>>   
>>   #endif /* NDP_H */
>> -- 
>> 2.50.1
>>
>