Re: [PATCH v2 07/10] conf: Don't exit if sourced default route has no gateway

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

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

On Mon, May 22, 2023 at 01:42:21AM +0200, Stefano Brivio wrote:
> If we use a template interface without a gateway on the default
> route, we can still offer almost complete functionality, except that,
> of course, we can't map the gateway address to the outer namespace or
> host, and that we have no obvious server address or identifier for
> use in DHCP's siaddr and option 54 (Server identifier, mandatory).
> 
> Continue, if we have a default route but no default gateway, and
> imply --no-map-gw and --no-dhcp in that case. NDP responder and
> DHCPv6 should be able to work as usual because we require a
> link-local address to be present, and we'll fall back to that.

Implying (rather than requiring) --no-map-gw and --no-dhcp does worry
me a bit.  I feel like it might violate the principle of least
surprise.

> Together with the previous commits implementing an actual copy of
> routes from the outer namespace, this should finally fix the
> operation of 'pasta --config-net' for cases where we have a default
> route on the host, but no default gateway, as it's the case for
> tap-style routes, including typical Wireguard endpoints.

Logic looks sound, though, so

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> 
> Reported-by: me@yawnt.com
> Link: https://bugs.passt.top/show_bug.cgi?id=49
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  conf.c  | 10 +++++++---
>  passt.1 |  6 ++++--
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 1392da5..c07b697 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -665,8 +665,7 @@ static unsigned int conf_ip4(unsigned int ifi,
>  	if (MAC_IS_ZERO(mac))
>  		nl_link(0, ifi, mac, 0, 0);
>  
> -	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->gw) ||
> -	    IN4_IS_ADDR_UNSPECIFIED(&ip4->addr) ||
> +	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr) ||
>  	    MAC_IS_ZERO(mac))
>  		return 0;
>  
> @@ -708,7 +707,6 @@ static unsigned int conf_ip6(unsigned int ifi,
>  		nl_link(0, ifi, mac, 0, 0);
>  
>  	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->gw) ||
> -	    IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ||
>  	    IN6_IS_ADDR_UNSPECIFIED(&ip6->addr_ll) ||
>  	    MAC_IS_ZERO(mac))
>  		return 0;
> @@ -1658,6 +1656,12 @@ void conf(struct ctx *c, int argc, char **argv)
>  	    (*c->ip6.ifname_out && !c->ifi6))
>  		die("External interface not usable");
>  
> +	if (c->ifi4 && IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw))
> +		c->no_map_gw = c->no_dhcp = 1;
> +
> +	if (c->ifi6 && IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw))
> +		c->no_map_gw = 1;
> +
>  	/* Inbound port options can be parsed now (after IPv4/IPv6 settings) */
>  	optind = 1;
>  	do {
> diff --git a/passt.1 b/passt.1
> index 10c96ae..f965c34 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -281,7 +281,8 @@ guest or target namespace will be silently dropped.
>  .TP
>  .BR \-\-no-dhcp
>  Disable the DHCP server. DHCP client requests coming from guest or target
> -namespace will be silently dropped.
> +namespace will be silently dropped. Implied if there is no gateway on the
> +selected IPv4 default route.
>  
>  .TP
>  .BR \-\-no-ndp
> @@ -301,7 +302,8 @@ namespace will be ignored.
>  .TP
>  .BR \-\-no-map-gw
>  Don't remap TCP connections and untracked UDP traffic, with the gateway address
> -as destination, to the host.
> +as destination, to the host. Implied if there is no gateway on the selected
> +default route for any of the enabled address families.
>  
>  .TP
>  .BR \-4 ", " \-\-ipv4-only

-- 
David Gibson			| 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 --]