Re: [PATCH] netlink: For IPv4, IFA_LOCAL is the interface address, not IFA_ADDRESS

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

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

On Thu, Apr 25, 2024 at 07:29:43AM +0200, Stefano Brivio wrote:
> See the comment to the unnamed enum in linux/if_addr.h, which
> currently states:
> 
>   /*
>    * Important comment:
>    * IFA_ADDRESS is prefix address, rather than local interface address.
>    * It makes no difference for normally configured broadcast interfaces,
>    * but for point-to-point IFA_ADDRESS is DESTINATION address,
>    * local address is supplied in IFA_LOCAL attribute.
>    *
>    * [...]
>    */
> 
> if we fetch IFA_ADDRESS, and we have a point-to-point link with a peer
> address configured, we'll source the peer address as "our" address,
> and refuse to resolve it in arp().
> 
> This was reported with pasta and a tun upstream interface configured
> by OpenVPN in "p2p" topology: the target namespace will have similar
> addresses and routes as the host, which is fine, and will try to
> resolve the point-to-point peer address (because it's the default
> gateway).
> 
> Given that we configure it as our address (only internally, not for
> visibly in the namespace), we'll fail to resolve that and traffic
> doesn't go anywhere.
> 
> Note that this is not the case for IPv6: there, IFA_ADDRESS is the
> actual, local address of the interface, and IFA_LOCAL is not
> necessarily present, so the comment in linux/if_addr.h doesn't apply
> either.

Huh.  Weird.

> Link: https://github.com/containers/podman/issues/22320
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

> ---
>  netlink.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/netlink.c b/netlink.c
> index 89c0641..447fea2 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -668,7 +668,8 @@ int nl_addr_get(int s, unsigned int ifi, sa_family_t af,
>  
>  		for (rta = IFA_RTA(ifa), na = IFA_PAYLOAD(nh); RTA_OK(rta, na);
>  		     rta = RTA_NEXT(rta, na)) {
> -			if (rta->rta_type != IFA_ADDRESS)
> +			if ((af == AF_INET  && rta->rta_type != IFA_LOCAL) ||
> +			    (af == AF_INET6 && rta->rta_type != IFA_ADDRESS))
>  				continue;
>  
>  			if (af == AF_INET && ifa->ifa_prefixlen > prefix_max) {

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