Re: [PATCH v4 1/7] arp: Fix a handful of small warts

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

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

On Thu, Oct 17, 2024 at 10:45:24AM +0200, Stefano Brivio wrote:
> On Thu, 17 Oct 2024 16:32:58 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > This fixes a number of harmless but slightly ugly warts in the ARP
> > resolution code:
> >  * Use in4addr_any to represent 0.0.0.0 rather than hand constructing an
> >    example.
> >  * When comparing am->sip against 0.0.0.0 use sizeof(am->sip) instead of
> >    sizeof(am->tip) (same value, but makes more logical sense)
> >  * Described the guest's assigned address as such, rather than as "our
> >    address" - that's not usually what we mean by "our address" these days
> >  * Remove "we might have the same IP address" comment which I can't make
> >    sense of in context (possibly it's relating to the statement below,
> >    which already has its own comment?)
> 
> Right, yes, I added the part below later, in 64c0f20ab3f8 ("arp: Don't
> resolve own, configured IPv4 address").
> 
> The first check comes from 2166c5872e9b ("arp: Don't answer announcements
> from guest or namespace") instead. The commit title makes it obvious, the
> comment not so much.
> 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  arp.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arp.c b/arp.c
> > index 53334da..d34b20e 100644
> > --- a/arp.c
> > +++ b/arp.c
> > @@ -59,14 +59,12 @@ int arp(const struct ctx *c, const struct pool *p)
> >  	    ah->ar_op  != htons(ARPOP_REQUEST))
> >  		return 1;
> >  
> > -	/* Discard announcements (but not 0.0.0.0 "probes"): we might have the
> > -	 * same IP address, hide that.
> > -	 */
> > -	if (memcmp(am->sip, (unsigned char[4]){ 0 }, sizeof(am->tip)) &&
> > +	/* Discard announcements, but not 0.0.0.0 "probes" */
> > +	if (memcmp(am->sip, &in4addr_any, sizeof(am->sip)) &&
> >  	    !memcmp(am->sip, am->tip, sizeof(am->sip)))
> >  		return 1;
> >  
> > -	/* Don't resolve our own address, either. */
> > +	/* Don't resolve guest's assigned address, either. */
> 
> Nit: "the guest's assigned address".

Fixed.

> >  	if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip)))
> >  		return 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

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