Re: [PATCH 1/3] conf: Consistency check between configured IPv4 netmask and gateway

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 3 Nov 2022 14:17:57 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Nov 03, 2022 at 12:04:41AM +0100, Stefano Brivio wrote:
> > Seen in a Google Compute Engine environment with a machine configured
> > via cloud-init-dhcp, while testing Podman integration for pasta: the
> > assigned address has a /32 netmask, and there's a default route,
> > which can be added on the host because there's another route, also
> > /32, pointing to the default gateway.  
> 
> I'm afraid I'm having trouble getting a good picture of the situation
> from this description.  I think an actual example with addresses would
> make it much clearer.

Added in v2.

> > This is not a valid configuration as far as I can tell: if the
> > address is configured as /32, it shouldn't be used to reach a gateway
> > outside its derived netmask. However, Linux allows that, and
> > everything works.
> > 
> > The problem comes when pasta --config-net sources address and default
> > route from the host, and it can't configure the route in the target
> > namespace because the gateway is invalid.
> > 
> > Sourcing more routes than just the default is doable, but probably
> > undesirable: pasta users want to provide connectivity to a container,
> > not reflect exactly whatever trickery is configured on the host.
> > 
> > Add a consistency check: if the configured default gateway is not
> > reachable, shrink the given netmask until we can reach it.  
> 
> Hmm... this isn't merely a check, it's changing an otherwise
> configured value.

Well, yes, there's a check, and if that fails, we adjust the netmask,
as this paragraph says. Reporting "[c]onsistency check and eventual
adjustment" everywhere sounds a bit heavy.

> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  conf.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/conf.c b/conf.c
> > index 90214f5..5b88547 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -562,6 +562,10 @@ static unsigned int conf_ip4(unsigned int ifi,
> >  			ip4->mask = 0xffffffff;
> >  	}
> >  
> > +	/* Mask consistency check: ensure we can reach the default gateway */  
> 
> Since this is to handle a very weird situation, we absolutely need a
> more detailed comment here.

Added in v2.

-- 
Stefano