On Fri, Feb 02, 2024 at 12:32:57AM +0100, Stefano Brivio wrote:
> If the default route for a given IP version is a multipath one,
> instead of refusing to start because there's no RTA_GATEWAY attribute
> in the set returned by the kernel, we can just pick one of the paths.
> 
> To make this somewhat less arbitrary, pick the path with the highest
> weight, if weights differ.
> 
> Reported-by: Ed Santiago <santiago@redhat.com>
> Link: https://github.com/containers/podman/issues/20927
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

I still hope I can dramatically simplify this at some point by making
a distinction between a general "host" pif and pifs bound to specific
host interfaces.  But this looks reasonable for the time being.

> ---
>  netlink.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  passt.1   |  6 ++++--
>  2 files changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/netlink.c b/netlink.c
> index bf79dd4..f0b04cb 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -271,18 +271,60 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>  
>  		for (rta = RTM_RTA(rtm), na = RTM_PAYLOAD(nh); RTA_OK(rta, na);
>  		     rta = RTA_NEXT(rta, na)) {
> -			if (rta->rta_type != RTA_OIF)
> -				continue;
> +			if (rta->rta_type == RTA_OIF) {
> +				ifi = *(unsigned int *)RTA_DATA(rta);
> +			} else if (rta->rta_type == RTA_MULTIPATH) {
> +				struct rtnexthop *rtnh;
>  
> -			ifi = *(unsigned int *)RTA_DATA(rta);
> +				rtnh = (struct rtnexthop *)RTA_DATA(rta);
> +				ifi = rtnh->rtnh_ifindex;
> +			}
>  		}
>  	}
> +
>  	if (status < 0)
>  		warn("netlink: RTM_GETROUTE failed: %s", strerror(-status));
>  
>  	return ifi;
>  }
>  
> +/**
> + * nl_route_get_def_multipath() - Get lowest-weight route from nexthop list
> + * @rta:	Routing netlink attribute with type RTA_MULTIPATH
> + * @gw:		Default gateway to fill
> + *
> + * Return: true if a gateway was found, false otherwise
> + */
> +bool nl_route_get_def_multipath(struct rtattr *rta, void *gw)
> +{
> +	struct rtnexthop *rtnh;
> +	bool found = false;
> +	int hops = -1;
> +
> +	for (rtnh = (struct rtnexthop *)RTA_DATA(rta);
> +	     RTNH_OK(rtnh, RTA_PAYLOAD(rta)); rtnh = RTNH_NEXT(rtnh)) {
> +		size_t len = rtnh->rtnh_len - sizeof(*rtnh);
> +		struct rtattr *rta_inner;
> +
> +		if (rtnh->rtnh_hops < hops)
> +			continue;
> +
> +		hops = rtnh->rtnh_hops;
> +
> +		for (rta_inner = RTNH_DATA(rtnh); RTA_OK(rta_inner, len);
> +		     rta_inner = RTA_NEXT(rta_inner, len)) {
> +
> +			if (rta_inner->rta_type != RTA_GATEWAY)
> +				continue;
> +
> +			memcpy(gw, RTA_DATA(rta_inner), RTA_PAYLOAD(rta_inner));
> +			found = true;
> +		}
> +	}
> +
> +	return found;
> +}
> +
>  /**
>   * nl_route_get_def() - Get default route for given interface and address family
>   * @s:		Netlink socket
> @@ -326,6 +368,9 @@ int nl_route_get_def(int s, unsigned int ifi, sa_family_t af, void *gw)
>  
>  		for (rta = RTM_RTA(rtm), na = RTM_PAYLOAD(nh); RTA_OK(rta, na);
>  		     rta = RTA_NEXT(rta, na)) {
> +			if (rta->rta_type == RTA_MULTIPATH)
> +				found = nl_route_get_def_multipath(rta, gw);
> +
>  			if (rta->rta_type != RTA_GATEWAY)
>  				continue;
>  
> diff --git a/passt.1 b/passt.1
> index efd6bb7..cc678ed 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -171,8 +171,10 @@ Assign IPv4 \fIaddr\fR as default gateway via DHCP (option 3), or IPv6
>  \fIaddr\fR as source for NDP Router Advertisement and DHCPv6 messages.
>  This option can be specified zero (for defaults) to two times (once for IPv4,
>  once for IPv6).
> -By default, IPv4 and IPv6 addresses are taken from the host interface with the
> -first default route for the corresponding IP version.
> +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.
>  
>  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,

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