[PATCH] netlink: Skip loopback interface while looking for a template

[PATCH] netlink: Skip loopback interface while looking for a template

[Stefano Brivio]

There might be reasons to have routes on the loopback interface, for
example Any-IP/AnyIP routes as implemented by Linux kernel commit
ab79ad14a2d5 ("ipv6: Implement Any-IP support for IPv6.").

If we use the loopback interface as a template, though, we'll pick
'lo' (typically) as interface name for our tap interface, but we'll
already have an interface called 'lo' in the target namespace, and as
we TUNSETIFF on it, we'll fail with EINVAL, because it's not a tap
interface.

Skip the loopback interface while looking for a template interface or,
more accurately, skip the interface with index 1.

Strictly speaking, we should fetch interface flags via RTM_GETLINK
instead, and check for IFF_LOOPBACK, but interleaving that request
while we're iterating over routes is unnecessarily complicated.

Link: https://www.reddit.com/r/podman/comments/1i6pj7u/starting_pod_without_external_network/
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 netlink.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/netlink.c b/netlink.c
index 0407692..37d8b5b 100644
--- a/netlink.c
+++ b/netlink.c
@@ -297,6 +297,10 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
 		if (!thisifi)
 			continue; /* No interface for this route */
 
+		/* Skip 'lo': we should test IFF_LOOPBACK, but keep it simple */
+		if (thisifi == 1)
+			continue;
+
 		/* Skip routes to link-local addresses */
 		if (af == AF_INET && dst &&
 		    IN4_IS_PREFIX_LINKLOCAL(dst, rtm->rtm_dst_len))
-- 
@@ -297,6 +297,10 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
 		if (!thisifi)
 			continue; /* No interface for this route */
 
+		/* Skip 'lo': we should test IFF_LOOPBACK, but keep it simple */
+		if (thisifi == 1)
+			continue;
+
 		/* Skip routes to link-local addresses */
 		if (af == AF_INET && dst &&
 		    IN4_IS_PREFIX_LINKLOCAL(dst, rtm->rtm_dst_len))
-- 
2.43.0

Re: [PATCH] netlink: Skip loopback interface while looking for a template

[Paul Holzinger]

On 23/01/2025 09:05, Stefano Brivio wrote:
> There might be reasons to have routes on the loopback interface, for
> example Any-IP/AnyIP routes as implemented by Linux kernel commit
> ab79ad14a2d5 ("ipv6: Implement Any-IP support for IPv6.").
>
> If we use the loopback interface as a template, though, we'll pick
> 'lo' (typically) as interface name for our tap interface, but we'll
> already have an interface called 'lo' in the target namespace, and as
> we TUNSETIFF on it, we'll fail with EINVAL, because it's not a tap
> interface.
>
> Skip the loopback interface while looking for a template interface or,
> more accurately, skip the interface with index 1.
>
> Strictly speaking, we should fetch interface flags via RTM_GETLINK
> instead, and check for IFF_LOOPBACK, but interleaving that request
> while we're iterating over routes is unnecessarily complicated.

I think hard coding 1 is fine but I think there is also the IFF_LOOPBACK 
flag that could be used instead.

 From strace:
ifi_index=if_nametoindex("lo"), 
ifi_flags=IFF_UP|IFF_LOOPBACK|IFF_RUNNING|IFF_LOWER_UP

>
> Link: https://www.reddit.com/r/podman/comments/1i6pj7u/starting_pod_without_external_network/
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>   netlink.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/netlink.c b/netlink.c
> index 0407692..37d8b5b 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -297,6 +297,10 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>   		if (!thisifi)
>   			continue; /* No interface for this route */
>   
> +		/* Skip 'lo': we should test IFF_LOOPBACK, but keep it simple */
> +		if (thisifi == 1)
> +			continue;
> +
>   		/* Skip routes to link-local addresses */
>   		if (af == AF_INET && dst &&
>   		    IN4_IS_PREFIX_LINKLOCAL(dst, rtm->rtm_dst_len))

-- 
Paul Holzinger

Re: [PATCH] netlink: Skip loopback interface while looking for a template

[Stefano Brivio]

On Thu, 23 Jan 2025 12:49:06 +0100
Paul Holzinger <pholzing@redhat.com> wrote:

> On 23/01/2025 09:05, Stefano Brivio wrote:
> > There might be reasons to have routes on the loopback interface, for
> > example Any-IP/AnyIP routes as implemented by Linux kernel commit
> > ab79ad14a2d5 ("ipv6: Implement Any-IP support for IPv6.").
> >
> > If we use the loopback interface as a template, though, we'll pick
> > 'lo' (typically) as interface name for our tap interface, but we'll
> > already have an interface called 'lo' in the target namespace, and as
> > we TUNSETIFF on it, we'll fail with EINVAL, because it's not a tap
> > interface.
> >
> > Skip the loopback interface while looking for a template interface or,
> > more accurately, skip the interface with index 1.
> >
> > Strictly speaking, we should fetch interface flags via RTM_GETLINK
> > instead, and check for IFF_LOOPBACK, but interleaving that request
> > while we're iterating over routes is unnecessarily complicated.  
> 
> I think hard coding 1 is fine but I think there is also the IFF_LOOPBACK 
> flag that could be used instead.
> 
>  From strace:
> ifi_index=if_nametoindex("lo"), 
> ifi_flags=IFF_UP|IFF_LOOPBACK|IFF_RUNNING|IFF_LOWER_UP

Yeah but that's in the link flags, right? Am I missing something? Here
we're looking at routes.

Note that 'ip route show' will also fetch link attributes with
RTM_GETLINK, which is the second netlink query I wanted to avoid here.

-- 
Stefano

Re: [PATCH] netlink: Skip loopback interface while looking for a template

[Paul Holzinger]

On 23/01/2025 12:53, Stefano Brivio wrote:
> On Thu, 23 Jan 2025 12:49:06 +0100
> Paul Holzinger <pholzing@redhat.com> wrote:
>
>> On 23/01/2025 09:05, Stefano Brivio wrote:
>>> There might be reasons to have routes on the loopback interface, for
>>> example Any-IP/AnyIP routes as implemented by Linux kernel commit
>>> ab79ad14a2d5 ("ipv6: Implement Any-IP support for IPv6.").
>>>
>>> If we use the loopback interface as a template, though, we'll pick
>>> 'lo' (typically) as interface name for our tap interface, but we'll
>>> already have an interface called 'lo' in the target namespace, and as
>>> we TUNSETIFF on it, we'll fail with EINVAL, because it's not a tap
>>> interface.
>>>
>>> Skip the loopback interface while looking for a template interface or,
>>> more accurately, skip the interface with index 1.
>>>
>>> Strictly speaking, we should fetch interface flags via RTM_GETLINK
>>> instead, and check for IFF_LOOPBACK, but interleaving that request
>>> while we're iterating over routes is unnecessarily complicated.
>> I think hard coding 1 is fine but I think there is also the IFF_LOOPBACK
>> flag that could be used instead.
>>
>>   From strace:
>> ifi_index=if_nametoindex("lo"),
>> ifi_flags=IFF_UP|IFF_LOOPBACK|IFF_RUNNING|IFF_LOWER_UP
> Yeah but that's in the link flags, right? Am I missing something? Here
> we're looking at routes.
>
> Note that 'ip route show' will also fetch link attributes with
> RTM_GETLINK, which is the second netlink query I wanted to avoid here.

Ah yes totally overlooked the fact the ip route does the RTM_GETLINK 
call there.

-- 
Paul Holzinger

Re: [PATCH] netlink: Skip loopback interface while looking for a template

[David Gibson]

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

On Thu, Jan 23, 2025 at 09:05:48AM +0100, Stefano Brivio wrote:
> There might be reasons to have routes on the loopback interface, for
> example Any-IP/AnyIP routes as implemented by Linux kernel commit
> ab79ad14a2d5 ("ipv6: Implement Any-IP support for IPv6.").
> 
> If we use the loopback interface as a template, though, we'll pick
> 'lo' (typically) as interface name for our tap interface, but we'll
> already have an interface called 'lo' in the target namespace, and as
> we TUNSETIFF on it, we'll fail with EINVAL, because it's not a tap
> interface.
> 
> Skip the loopback interface while looking for a template interface or,
> more accurately, skip the interface with index 1.
> 
> Strictly speaking, we should fetch interface flags via RTM_GETLINK
> instead, and check for IFF_LOOPBACK, but interleaving that request
> while we're iterating over routes is unnecessarily complicated.
> 
> Link: https://www.reddit.com/r/podman/comments/1i6pj7u/starting_pod_without_external_network/
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

> ---
>  netlink.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/netlink.c b/netlink.c
> index 0407692..37d8b5b 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -297,6 +297,10 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>  		if (!thisifi)
>  			continue; /* No interface for this route */
>  
> +		/* Skip 'lo': we should test IFF_LOOPBACK, but keep it simple */
> +		if (thisifi == 1)
> +			continue;
> +
>  		/* Skip routes to link-local addresses */
>  		if (af == AF_INET && dst &&
>  		    IN4_IS_PREFIX_LINKLOCAL(dst, rtm->rtm_dst_len))

-- 
David Gibson (he or they)	| 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 --]