On Fri, 16 Feb 2024 11:23:30 +0100
Paul Holzinger <pholzing(a)redhat.com> wrote:

...
  Hi Stefano,
 
 On 16/02/2024 06:09, Stefano Brivio wrote:
  ...Podman users might get confused by the fact
that if we can't
 find a default route for a given IP version, we'll report that as a
 warning message and possibly just before actual error messages.

 However, a lack of routable interface for IPv4 or IPv6 can be a
 normal circumstance: don't warn about it, just state that as
 informational message, if those are displayed (they're not in
 non-error paths in Podman, for example).   
 
 A bit of topic but what actually is the default log level? 
It's LOG_INFO.

...
  Looking at 
 the main function I see:
 
 if (c.debug)
          __setlogmask(LOG_UPTO(LOG_DEBUG));
      else if (c.quiet)
          __setlogmask(LOG_UPTO(LOG_ERR)); 
                                     ^^^
...
       else
          __setlogmask(LOG_UPTO(LOG_INFO));
 
 So if the default is still log level is info there is no way for podman 
 to say show warnings/errors only. 
That's because the second clause above is wrong, I think. It should be:

	else if (c.quiet)
		__setlogmask(LOG_UPTO(LOG_WARN));

because we also say in the man page:

       -q, --quiet
              Don't print informational messages.

but nowhere it's written that we'll also hide warnings with it.

...
  We can use --quiet but I think the 
 warnings should be displayed to end users as well.
 So my next request would be to one of the following:
 a) change the default level to warn but then there no way show info 
 unless debug is set (or add a new flag for info) 
I think LOG_INFO should really be the default, if you use passt or
pasta stand-alone that's very helpful.

...
  b) add a flag to select warning level 
...which is however what --quiet is supposed to do.

...
  c) log info to stdout and warn/err to stderr then
podman could just show 
 stderr and hide stdout 
...which doesn't fit the meaning of standard output though: pasta has
no functional terminal output.

I would just fix --quiet if that suits Podman as well.

...
 
  While at it,
make it clear that we're disabling IPv4 or IPv6 if
 there's no routable interface for the corresponding IP version.

 Reported-by: Paul Holzinger <pholzing(a)redhat.com>
 Link: https://github.com/containers/podman/pull/21563#issuecomment-1937024642
 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
 ---
 v2: Report that we're disabling IPv4 or IPv6 in the message

   conf.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/conf.c b/conf.c
 index 5e15b66..3646700 100644
 --- a/conf.c
 +++ b/conf.c
 @@ -579,7 +579,7 @@ static unsigned int conf_ip4(unsigned int ifi,
   		ifi = nl_get_ext_if(nl_sock, AF_INET);
   
   	if (!ifi) {
 -		warn("No external routable interface for IPv4");
 +		info("No routable interface for IPv4: IPv4 is disabled");
   		return 0;
   	}
   
 @@ -651,7 +651,7 @@ static unsigned int conf_ip6(unsigned int ifi,
   		ifi = nl_get_ext_if(nl_sock, AF_INET6);
   
   	if (!ifi) {
 -		warn("No external routable interface for IPv6");
 +		info("No routable interface for IPv6: IPv6 is disabled");   
 The code only looks for a default route, so if one has some custom 
 internal routes then saying no routable interface found is confusing. 
 What this should really say is:
 No interface with a default route for IPv... 
Oops, sorry, I just pushed this.

I think it's clear enough in the sense that by design passt and pasta
need a default route, and if you have a specific route on a given
interface, that interface is not "routable" for our purposes.

But sure, we can make it more explicit. Building on your suggestion --
would this be okay:

  "No interface with a default route for IPv[46]: disabling IPv[64]"

? David?

-- 
Stefano