NetBSD Problem Report #55691

From gson@gson.org  Fri Oct  2 12:33:14 2020
Return-Path: <gson@gson.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 4DDEB1A9217
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  2 Oct 2020 12:33:14 +0000 (UTC)
Message-Id: <20201002123308.A5E38253EDA@guava.gson.org>
Date: Fri,  2 Oct 2020 15:33:08 +0300 (EEST)
From: gson@gson.org (Andreas Gustafsson)
Reply-To: gson@gson.org (Andreas Gustafsson)
To: gnats-bugs@NetBSD.org
Subject: rump kernels log "rn_init: radix functions require max_keylen be set"
X-Send-Pr-Version: 3.95

>Number:         55691
>Category:       kern
>Synopsis:       rump kernels log "rn_init: radix functions require max_keylen be set"
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Oct 02 12:35:00 +0000 2020
>Closed-Date:    Sun Oct 18 13:11:01 +0000 2020
>Last-Modified:  Sun Oct 18 16:45:01 +0000 2020
>Originator:     Andreas Gustafsson
>Release:        NetBSD-current
>Organization:
>Environment:
System: NetBSD
Architecture: x86_64
Machine: amd64
>Description:

When running the ATF test suite, the output from atf-run (without any
processing by atf-report) contains about 200 messages like this one:

  rn_init: radix functions require max_keylen be set

These are printed by rump kernels.  This does not appear to be causing
any tests to fail, but the messages clutter the output, and they cause
confusion because when a test fails and logs an error message, it's
easy to incorrectly assume that the two are related.

>How-To-Repeat:

cd /usr/tests
atf-run | grep max_keylen

>Fix:

>Release-Note:

>Audit-Trail:
From: Andreas Gustafsson <gson@gson.org>
To: gnats-bugs@netbsd.org
Cc: kre@NetBSD.org
Subject: Re: kern/55691: rump kernels log "rn_init: radix functions require max_keylen be set"
Date: Sat, 17 Oct 2020 12:04:23 +0300

 Earlier, I wrote:
 >   rn_init: radix functions require max_keylen be set
 > 
 > These are printed by rump kernels.  This does not appear to be causing
 > any tests to fail, but the messages clutter the output, and they cause
 > confusion because when a test fails and logs an error message, it's
 > easy to incorrectly assume that the two are related.

 This was discussed on current-users in April:

   http://mail-index.netbsd.org/current-users/2020/04/08/msg038300.html
   http://mail-index.netbsd.org/current-users/2020/04/08/msg038301.html

 In the first of the above messages, kre wrote:

 > Note the #ifdef _KERNEL
 >
 > That means that that code is absent in rump

 I don't think that's the case - rump is built with _KERNEL defined.

 An easy way to reproduce the issue without running the full ATF
 test suite is:

   cd /usr/tests/include/sys
   ./t_socket cmsg_sendfd

 Running that under gdb and putting a breakpoint in rn_init shows that
 in the following loop,

         DOMAIN_FOREACH(dp) {
                 if (dp->dom_maxrtkey > max_keylen)
                         max_keylen = dp->dom_maxrtkey;
         }

 there are three domains, "unix", "link", and "route", but they all
 have a dp->dom_maxrtkey of zero.

 Presumably they message will be printed by any kernel that is not
 configured with any routable networking domains (not limited to
 rump kernels).

 I'm leaning towards simply disabling the message in the _KERNEL case.
 Any objections?
 -- 
 Andreas Gustafsson, gson@gson.org

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: Andreas Gustafsson <gson@gson.org>
Subject: Re: kern/55691: rump kernels log "rn_init: radix functions require
 max_keylen be set"
Date: Sat, 17 Oct 2020 13:13:04 +0200

 On Sat, Oct 17, 2020 at 09:05:02AM +0000, Andreas Gustafsson wrote:
 >  Running that under gdb and putting a breakpoint in rn_init shows that
 >  in the following loop,
 >  
 >          DOMAIN_FOREACH(dp) {
 >                  if (dp->dom_maxrtkey > max_keylen)
 >                          max_keylen = dp->dom_maxrtkey;
 >          }
 >  
 >  there are three domains, "unix", "link", and "route", but they all
 >  have a dp->dom_maxrtkey of zero.
 >  
 >  Presumably they message will be printed by any kernel that is not
 >  configured with any routable networking domains (not limited to
 >  rump kernels).
 >  
 >  I'm leaning towards simply disabling the message in the _KERNEL case.
 >  Any objections?

 Isn't it better to set some (new) boolean after that loop has run and change
 the assertion to verify the loop has run already, but also make the
 call to rn_init() depend on max_keylen > 0 ?

 Do we have any modules that would load routable protocols later? Do they
 work? How do they interact with this?

 Martin

From: Andreas Gustafsson <gson@gson.org>
To: Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/55691: rump kernels log "rn_init: radix functions require
 max_keylen be set"
Date: Sat, 17 Oct 2020 15:41:17 +0300

 Martin Husemann wrote:
 > >  I'm leaning towards simply disabling the message in the _KERNEL case.
 > >  Any objections?
 > 
 > Isn't it better to set some (new) boolean after that loop has run and change
 > the assertion to verify the loop has run already,

 It's not an assertion, just a log message:

         if (max_keylen == 0) {
                 log(LOG_ERR,
                     "rn_init: radix functions require max_keylen be set\n");
                 return;
         }

 There's no point in introducing a boolean to verify that the loop has
 run already, because the above code is immediately after the loop, so
 it is only reached if the loop has run (in the _KERNEL case; in the
 not-_KERNEL case, there is no loop).

 > but also make the call to rn_init() depend on max_keylen > 0 ?

 I don't understand what you mean by this.

 > Do we have any modules that would load routable protocols later?

 I don't think so.

 To be clear, the change I'm proposing is as follows:

 diff -u -r1.48 radix.c
 --- radix.c	3 Sep 2018 16:29:35 -0000	1.48
 +++ radix.c	17 Oct 2020 12:24:33 -0000
 @@ -1119,8 +1119,10 @@
  	}
  #endif
  	if (max_keylen == 0) {
 +#ifndef _KERNEL
  		log(LOG_ERR,
  		    "rn_init: radix functions require max_keylen be set\n");
 +#endif
  		return;
  	}

 -- 
 Andreas Gustafsson, gson@gson.org

From: Martin Husemann <martin@duskware.de>
To: Andreas Gustafsson <gson@gson.org>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/55691: rump kernels log "rn_init: radix functions require
 max_keylen be set"
Date: Sat, 17 Oct 2020 14:51:46 +0200

 On Sat, Oct 17, 2020 at 03:41:17PM +0300, Andreas Gustafsson wrote:
 > There's no point in introducing a boolean to verify that the loop has
 > run already, because the above code is immediately after the loop, so
 > it is only reached if the loop has run (in the _KERNEL case; in the
 > not-_KERNEL case, there is no loop).

 I got it - the code is shared with userland (e.g. routed uses it and
 manually sets max_keylen upfront).

 Yes, your suggested change looks good.

 Martin

From: "Andreas Gustafsson" <gson@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55691 CVS commit: src/sys/net
Date: Sun, 18 Oct 2020 13:07:31 +0000

 Module Name:	src
 Committed By:	gson
 Date:		Sun Oct 18 13:07:31 UTC 2020

 Modified Files:
 	src/sys/net: radix.c

 Log Message:
 Suppress the "rn_init: radix functions require max_keylen be set"
 message when _KERNEL is defined, to avoid spurious messages from
 kernels that have no routable network domains.  Fixes PR kern/55691.


 To generate a diff of this commit:
 cvs rdiff -u -r1.48 -r1.49 src/sys/net/radix.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: gson@NetBSD.org
State-Changed-When: Sun, 18 Oct 2020 13:11:01 +0000
State-Changed-Why:
Fixed.


From: Andreas Gustafsson <gson@gson.org>
To: Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/55691: rump kernels log "rn_init: radix functions require max_keylen be set"
Date: Sun, 18 Oct 2020 19:41:15 +0300

 Martin Husemann wrote:
 > I got it - the code is shared with userland (e.g. routed uses it and
 > manually sets max_keylen upfront).

 The code in sys/net/radix.c does look like it's intended to be shared
 with userland, but nothing in the NetBSD userland actually uses it
 (rump doesn't count since it defines _KERNEL).  There's a separate
 copy of the code in src/sbin/routed/radix.c, and the two copies have
 diverged.
 -- 
 Andreas Gustafsson, gson@gson.org

>Unformatted:

Home	PR Database Search