public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] netlink: With no default route, pick the first interface with a route
@ 2024-06-18 17:18 Stefano Brivio
  2024-06-19  1:53 ` David Gibson
  2024-06-19  8:31 ` Paul Holzinger
  0 siblings, 2 replies; 3+ messages in thread
From: Stefano Brivio @ 2024-06-18 17:18 UTC (permalink / raw)
  To: passt-dev; +Cc: David Gibson, Jelle van der Waa, Martin Pitt

While commit f919dc7a4b1c ("conf, netlink: Don't require a default
route to start") sounded reasonable in the assumption that, if we
don't find default routes for a given address family, we can still
proceed by selecting an interface with any route *iff it's the only
one for that protocol family*, Jelle reported a further issue in a
similar setup.

There, multiple interfaces are present, and while remote container
connectivity doesn't matter for the container, local connectivity is
desired. There are no default routes, but those multiple interfaces
all have non-default routes, so we should just pick one and start.

Pick the first interface reported by the kernel with any route, if
there are no default routes. There should be no harm in doing so.

Reported-by: Jelle van der Waa <jvanderwaa@redhat.com>
Reported-by: Martin Pitt <mpitt@redhat.com>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2277954
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 netlink.c | 16 ++++++++--------
 passt.1   |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/netlink.c b/netlink.c
index 0be4ea3..3aed7e5 100644
--- a/netlink.c
+++ b/netlink.c
@@ -269,8 +269,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
 	size_t na;
 
 	/* 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.
+	 * for any interface with a route, and pick the first one, if any.
 	 */
 	seq = nl_send(s, &req, RTM_GETROUTE, NLM_F_DUMP, sizeof(req));
 	nl_foreach_oftype(nh, status, s, buf, seq, RTM_NEWROUTE) {
@@ -324,18 +323,19 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
 		warn("netlink: RTM_GETROUTE failed: %s", strerror(-status));
 
 	if (defifi) {
-		if (ndef > 1)
+		if (ndef > 1) {
 			info("Multiple default %s routes, picked first",
 			     af_name(af));
+		}
 		return defifi;
 	}
 
 	if (anyifi) {
-		if (nany == 1)
-			return anyifi;
-
-		info("Multiple interfaces with %s routes, use -i to select one",
-		     af_name(af));
+		if (nany > 1) {
+			info("Multiple interfaces with %s routes, picked first",
+			     af_name(af));
+		}
+		return anyifi;
 	}
 
 	if (!nany)
diff --git a/passt.1 b/passt.1
index 6dfa670..6ee1e2e 100644
--- a/passt.1
+++ b/passt.1
@@ -152,8 +152,8 @@ 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, 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.
+default routes are available and there is any interface with any route for a
+given IP version, the first of these interfaces will be chosen instead.
 
 .TP
 .BR \-n ", " \-\-netmask " " \fImask
-- 
@@ -152,8 +152,8 @@ 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, 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.
+default routes are available and there is any interface with any route for a
+given IP version, the first of these interfaces will be chosen instead.
 
 .TP
 .BR \-n ", " \-\-netmask " " \fImask
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] netlink: With no default route, pick the first interface with a route
  2024-06-18 17:18 [PATCH] netlink: With no default route, pick the first interface with a route Stefano Brivio
@ 2024-06-19  1:53 ` David Gibson
  2024-06-19  8:31 ` Paul Holzinger
  1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2024-06-19  1:53 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Jelle van der Waa, Martin Pitt

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

On Tue, Jun 18, 2024 at 07:18:03PM +0200, Stefano Brivio wrote:
> While commit f919dc7a4b1c ("conf, netlink: Don't require a default
> route to start") sounded reasonable in the assumption that, if we
> don't find default routes for a given address family, we can still
> proceed by selecting an interface with any route *iff it's the only
> one for that protocol family*, Jelle reported a further issue in a
> similar setup.
> 
> There, multiple interfaces are present, and while remote container
> connectivity doesn't matter for the container, local connectivity is
> desired. There are no default routes, but those multiple interfaces
> all have non-default routes, so we should just pick one and start.
> 
> Pick the first interface reported by the kernel with any route, if
> there are no default routes. There should be no harm in doing so.
> 
> Reported-by: Jelle van der Waa <jvanderwaa@redhat.com>
> Reported-by: Martin Pitt <mpitt@redhat.com>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2277954
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Seems reasonable until we can tackle this better.

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

> ---
>  netlink.c | 16 ++++++++--------
>  passt.1   |  4 ++--
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/netlink.c b/netlink.c
> index 0be4ea3..3aed7e5 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -269,8 +269,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>  	size_t na;
>  
>  	/* 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.
> +	 * for any interface with a route, and pick the first one, if any.
>  	 */
>  	seq = nl_send(s, &req, RTM_GETROUTE, NLM_F_DUMP, sizeof(req));
>  	nl_foreach_oftype(nh, status, s, buf, seq, RTM_NEWROUTE) {
> @@ -324,18 +323,19 @@ unsigned int nl_get_ext_if(int s, sa_family_t af)
>  		warn("netlink: RTM_GETROUTE failed: %s", strerror(-status));
>  
>  	if (defifi) {
> -		if (ndef > 1)
> +		if (ndef > 1) {
>  			info("Multiple default %s routes, picked first",
>  			     af_name(af));
> +		}
>  		return defifi;
>  	}
>  
>  	if (anyifi) {
> -		if (nany == 1)
> -			return anyifi;
> -
> -		info("Multiple interfaces with %s routes, use -i to select one",
> -		     af_name(af));
> +		if (nany > 1) {
> +			info("Multiple interfaces with %s routes, picked first",
> +			     af_name(af));
> +		}
> +		return anyifi;
>  	}
>  
>  	if (!nany)
> diff --git a/passt.1 b/passt.1
> index 6dfa670..6ee1e2e 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -152,8 +152,8 @@ 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, 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.
> +default routes are available and there is any interface with any route for a
> +given IP version, the first of these interfaces will be chosen instead.
>  
>  .TP
>  .BR \-n ", " \-\-netmask " " \fImask

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] netlink: With no default route, pick the first interface with a route
  2024-06-18 17:18 [PATCH] netlink: With no default route, pick the first interface with a route Stefano Brivio
  2024-06-19  1:53 ` David Gibson
@ 2024-06-19  8:31 ` Paul Holzinger
  1 sibling, 0 replies; 3+ messages in thread
From: Paul Holzinger @ 2024-06-19  8:31 UTC (permalink / raw)
  To: Stefano Brivio, passt-dev; +Cc: David Gibson, Jelle van der Waa, Martin Pitt


On 18/06/2024 19:18, Stefano Brivio wrote:
> While commit f919dc7a4b1c ("conf, netlink: Don't require a default
> route to start") sounded reasonable in the assumption that, if we
> don't find default routes for a given address family, we can still
> proceed by selecting an interface with any route *iff it's the only
> one for that protocol family*, Jelle reported a further issue in a
> similar setup.
>
> There, multiple interfaces are present, and while remote container
> connectivity doesn't matter for the container, local connectivity is
> desired. There are no default routes, but those multiple interfaces
> all have non-default routes, so we should just pick one and start.
>
> Pick the first interface reported by the kernel with any route, if
> there are no default routes. There should be no harm in doing so.
>
> Reported-by: Jelle van der Waa<jvanderwaa@redhat.com>
> Reported-by: Martin Pitt<mpitt@redhat.com>
> Link:https://bugzilla.redhat.com/show_bug.cgi?id=2277954
> Signed-off-by: Stefano Brivio<sbrivio@redhat.com>

Reviewed-by: Paul Holzinger <pholzing@redhat.com>


-- 
Paul Holzinger


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-06-19  8:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-18 17:18 [PATCH] netlink: With no default route, pick the first interface with a route Stefano Brivio
2024-06-19  1:53 ` David Gibson
2024-06-19  8:31 ` Paul Holzinger

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).