Re: [PATCH v2] conf, netlink: Don't require a default route to start

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

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

On Fri, Mar 15, 2024 at 05:13:26PM +0100, Stefano Brivio wrote:
> There might be isolated testing environments where default routes and
> global connectivity are not needed, a single interface has all
> non-loopback addresses and routes, and still passt and pasta are
> expected to work.
> 
> In this case, it's pretty obvious what our upstream interface should
> be, so go ahead and select the only interface with at least one
> route, disabling DHCP and implying --no-map-gw as the documentation
> already states.
> 
> If there are multiple interfaces with routes, though, refuse to start,
> because at that point it's really not clear what we should do.
> 
> Reported-by: Martin Pitt <mpitt@redhat.com>
> Link: https://github.com/containers/podman/issues/21896
> Signed-off-by: Stefano brivio <sbrivio@redhat.com>

It's an ugly hack, but a useful one, so

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


> ---
> v2: Initialise rtnh in nl_get_ext_if() before using it...
> 
>  conf.c    |  4 ++--
>  netlink.c | 31 ++++++++++++++++++++++++++++---
>  passt.1   | 45 +++++++++++++++++++++++++++++----------------
>  3 files changed, 59 insertions(+), 21 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index ac9fb34..644752c 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -584,7 +584,7 @@ static unsigned int conf_ip4(unsigned int ifi,
>  		ifi = nl_get_ext_if(nl_sock, AF_INET);
>  
>  	if (!ifi) {
> -		info("No interface with a default route for IPv4: disabling IPv4");
> +		info("No interface with a route for IPv4: disabling IPv4");
>  		return 0;
>  	}
>  
> @@ -656,7 +656,7 @@ static unsigned int conf_ip6(unsigned int ifi,
>  		ifi = nl_get_ext_if(nl_sock, AF_INET6);
>  
>  	if (!ifi) {
> -		info("No interface with a default route for IPv6: disabling IPv6");
> +		info("No interface with a route for IPv6: disabling IPv6");
>  		return 0;
>  	}
>  
> diff --git a/netlink.c b/netlink.c
> index 20de9b3..f93f377 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -254,6 +254,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>  		.rtm.rtm_type	 = RTN_UNICAST,
>  		.rtm.rtm_family	 = af,
>  	};
> +	bool default_only = true;
>  	unsigned int ifi = 0;
>  	struct nlmsghdr *nh;
>  	struct rtattr *rta;
> @@ -262,21 +263,40 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>  	uint32_t seq;
>  	size_t na;
>  
> +again:
> +	/* Look for an interface with a default route first, failing that, look
> +	 * for any interface with a route, and pick it only if it's the only
> +	 * interface with a route.
> +	 */
>  	seq = nl_send(s, &req, RTM_GETROUTE, NLM_F_DUMP, sizeof(req));
>  	nl_foreach_oftype(nh, status, s, buf, seq, RTM_NEWROUTE) {
>  		struct rtmsg *rtm = (struct rtmsg *)NLMSG_DATA(nh);
>  
> -		if (ifi || rtm->rtm_dst_len || rtm->rtm_family != af)
> -			continue;
> +		if (default_only) {
> +			if (ifi || rtm->rtm_dst_len || rtm->rtm_family != af)
> +				continue;
> +		} else {
> +			if (rtm->rtm_family != af)
> +				continue;
> +		}
>  
>  		for (rta = RTM_RTA(rtm), na = RTM_PAYLOAD(nh); RTA_OK(rta, na);
>  		     rta = RTA_NEXT(rta, na)) {
>  			if (rta->rta_type == RTA_OIF) {
> +				if (!default_only && ifi &&
> +				    ifi != *(unsigned int *)RTA_DATA(rta))
> +					return 0;
> +
>  				ifi = *(unsigned int *)RTA_DATA(rta);
>  			} else if (rta->rta_type == RTA_MULTIPATH) {
>  				const struct rtnexthop *rtnh;
>  
>  				rtnh = (struct rtnexthop *)RTA_DATA(rta);
> +
> +				if (!default_only && ifi &&
> +				    (int)ifi != rtnh->rtnh_ifindex)
> +					return 0;
> +
>  				ifi = rtnh->rtnh_ifindex;
>  			}
>  		}
> @@ -285,6 +305,11 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>  	if (status < 0)
>  		warn("netlink: RTM_GETROUTE failed: %s", strerror(-status));
>  
> +	if (!ifi && default_only) {
> +		default_only = false;
> +		goto again;
> +	}
> +
>  	return ifi;
>  }
>  
> @@ -332,7 +357,7 @@ bool nl_route_get_def_multipath(struct rtattr *rta, void *gw)
>   * @af:		Address family
>   * @gw:		Default gateway to fill on NL_GET
>   *
> - * Return: 0 on success, negative error code on failure
> + * Return: error on netlink failure, or 0 (gw unset if default route not found)
>   */
>  int nl_route_get_def(int s, unsigned int ifi, sa_family_t af, void *gw)
>  {
> diff --git a/passt.1 b/passt.1
> index 9c492f5..3a23a43 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -148,7 +148,9 @@ for an IPv6 \fIaddr\fR.
>  This option can be specified zero (for defaults) to two times (once for IPv4,
>  once for IPv6).
>  By default, assigned IPv4 and IPv6 addresses are taken from the host interfaces
> -with the first default route for the corresponding IP version.
> +with the first default route, if any, for the corresponding IP version. If no
> +default routes are available and there is just one interface with any route,
> +that interface will be chosen instead.
>  
>  .TP
>  .BR \-n ", " \-\-netmask " " \fImask
> @@ -172,9 +174,11 @@ Assign IPv4 \fIaddr\fR as default gateway via DHCP (option 3), or IPv6
>  This option can be specified zero (for defaults) to two times (once for IPv4,
>  once for IPv6).
>  By default, IPv4 and IPv6 gateways are taken from the host interface with the
> -first default route for the corresponding IP version. If the default route is a
> -multipath one, the gateway is the first nexthop router returned by the kernel
> -which has the highest weight in the set of paths.
> +first default route, if any, for the corresponding IP version. If the default
> +route is a multipath one, the gateway is the first nexthop router returned by
> +the kernel which has the highest weight in the set of paths. If no default
> +routes are available and there is just one interface with any route, that
> +interface will be chosen instead.
>  
>  Note: these addresses are also used as source address for packets directed to
>  the guest or to the target namespace having a loopback or local source address,
> @@ -185,9 +189,11 @@ to allow mapping of local traffic to guest and target namespace. See the
>  .BR \-i ", " \-\-interface " " \fIname
>  Use host interface \fIname\fR to derive addresses and routes.
>  Default is to use the interfaces specified by \fB--outbound-if4\fR and
> -\fB--outbound-if6\fR, for IPv4 and IPv6 addresses and routes, respectively. If
> -no interfaces are given, the interface with the first default routes for each IP
> -version is selected.
> +\fB--outbound-if6\fR, for IPv4 and IPv6 addresses and routes, respectively.
> +
> +If no interfaces are given, the interface with the first default routes for each
> +IP version is selected. If no default routes are available and there is just one
> +interface with any route, that interface will be chosen instead.
>  
>  .TP
>  .BR \-o ", " \-\-outbound " " \fIaddr
> @@ -203,14 +209,20 @@ By default, the source address is selected by the routing tables.
>  Bind IPv4 outbound sockets to host interface \fIname\fR, and, unless another
>  interface is specified via \fB-i\fR, \fB--interface\fR, use this interface to
>  derive IPv4 addresses and routes.
> -By default, the interface given by the default route is selected.
> +
> +By default, the interface given by the default route is selected. If no default
> +routes are available and there is just one interface with any route, that
> +interface will be chosen instead.
>  
>  .TP
>  .BR \-\-outbound-if6 " " \fIname
>  Bind IPv6 outbound sockets to host interface \fIname\fR, and, unless another
>  interface is specified via \fB-i\fR, \fB--interface\fR, use this interface to
>  derive IPv6 addresses and routes.
> -By default, the interface given by the default route is selected.
> +
> +By default, the interface given by the default route is selected. If no default
> +routes are available and there is just one interface with any route, that
> +interface will be chosen instead.
>  
>  .TP
>  .BR \-D ", " \-\-dns " " \fIaddr
> @@ -305,19 +317,20 @@ namespace will be ignored.
>  .BR \-\-no-map-gw
>  Don't remap TCP connections and untracked UDP traffic, with the gateway address
>  as destination, to the host. Implied if there is no gateway on the selected
> -default route for any of the enabled address families.
> +default route, or if there is no default route, for any of the enabled address
> +families.
>  
>  .TP
>  .BR \-4 ", " \-\-ipv4-only
>  Enable IPv4-only operation. IPv6 traffic will be ignored.
> -By default, IPv6 operation is enabled as long as at least an IPv6 default route
> -and an interface address are configured on a given host interface.
> +By default, IPv6 operation is enabled as long as at least an IPv6 route and an
> +interface address are configured on a given host interface.
>  
>  .TP
>  .BR \-6 ", " \-\-ipv6-only
>  Enable IPv6-only operation. IPv4 traffic will be ignored.
> -By default, IPv4 operation is enabled as long as at least an IPv4 default route
> -and an interface address are configured on a given host interface.
> +By default, IPv4 operation is enabled as long as at least an IPv4 route and an
> +interface address are configured on a given host interface.
>  
>  .SS \fBpasst\fR-only options
>  
> @@ -817,8 +830,8 @@ local addresses, and it would also be impossible for guest or target namespace
>  to route answers back.
>  
>  For convenience, and somewhat arbitrarily, the source address on these packets
> -is translated to the address of the default IPv4 or IPv6 gateway -- this is
> -known to be an existing, valid address on the same subnet.
> +is translated to the address of the default IPv4 or IPv6 gateway (if any) --
> +this is known to be an existing, valid address on the same subnet.
>  
>  Loopback destination addresses are instead translated to the observed external
>  address of the guest or target namespace. For IPv6 packets, if usage of a

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