Summary ======= The warning printed when the kernel is instructed to grow the routing tables with the addition of a new FIB is unhelpful and confusing. Currently the warning reads: WARNING: Adding ifaddrs to all fibs has been turned off by default. Consider tuning net.add_addr_allfibs if needed (sys/net/route/route_tables.c:233) Details ======= The issue with this message is that the action that triggers it is the act of growing the number of FIBs in the system -- which typically happens at boot time -- and not the act of adding address to any interfaces. The message makes it seem that the user has performed one of these actions when in fact they have not, and makes it difficult for the user to understand what needs to be done to REMOVE the message. I know this because I spent an hour trying to figure out what was wrong with my system configuration because of this message and in the end, I found out that NOTHING was wrong. Proposal ======== I propose that the warning be changed ever so slightly so that the user can tell 1. When it was generated (when net.fibs is adjusted) 2. That the warning is a reminder, not the detection of a problem. Perhaps: --- route_tables.c.orig 2024-07-02 12:18:49.627899000 -0700 +++ route_tables.c 2024-07-02 12:20:44.759937000 -0700 @@ -231,8 +231,11 @@ M_RTABLE, M_WAITOK | M_ZERO); if ((num_tables > 1) && (V_rt_add_addr_allfibs == 0)) - printf("WARNING: Adding ifaddrs to all fibs has been turned off " + printf("%s adjusted to %d. REMINDER: " + "Adding ifaddrs to all fibs has been turned off " "by default. Consider tuning %s if needed\n", + "net.fibs", + num_tables, "net.add_addr_allfibs"); #ifdef FIB_ALGO
I think this change is even better. --- route_tables.c.orig 2024-07-02 12:18:49.627899000 -0700 +++ route_tables.c 2024-07-02 13:03:15.319885000 -0700 @@ -231,9 +231,13 @@ M_RTABLE, M_WAITOK | M_ZERO); if ((num_tables > 1) && (V_rt_add_addr_allfibs == 0)) - printf("WARNING: Adding ifaddrs to all fibs has been turned off " - "by default. Consider tuning %s if needed\n", + printf("%s adjusted to %d. REMINDER: " + "Interface addresses are no longer automatically added " + "to additional FIBs. Consider setting %s if needed\n", + "net.fibs", + num_tables, "net.add_addr_allfibs"); + #ifdef FIB_ALGO fib_grow_rtables(num_tables);
Created attachment 251841 [details] Proposed patch
Created attachment 251842 [details] Better proposed patch (removes extraneous newline) Removes an extraneous newline.
That message was introduced while changing `net.add_addr_allfibs` default from 1 to 0. Since now all supported releases has that ( default 0 ), I think it is time to suppress the warning now.
(In reply to Zhenlei Huang from comment #4) Oh, that's also a perfectly good solution, as well!
(In reply to Zhenlei Huang from comment #4) Zhenlei, do you plan to write a patch for this?
(In reply to Mark Johnston from comment #6) That is quite simple. I can dot that at this weekend.
(In reply to Mark Johnston from comment #6) Post to https://reviews.freebsd.org/D45971 .
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a48f7a2eb90b0812281e6d69bb05eb61433ea247 commit a48f7a2eb90b0812281e6d69bb05eb61433ea247 Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2024-08-01 17:48:58 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2024-08-01 17:48:58 +0000 fibs: Suppress the WARNING message for setups with multiple fibs Change 2d3982419593 switched net.add_addr_allfibs default to 0. The warning message is for potential users of the feature. Well since all supported releases have 0 as default, those potential users may have already gotten the notification, emitting this WARNING every time increasing the fib number is less useful but rather confusing to other users. So let's suppress it right now. PR: 280097 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D45971 sys/net/route/route_tables.c | 5 ----- 1 file changed, 5 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=352dd826ffdd88d26744023fe8bcff795bdf64d8 commit 352dd826ffdd88d26744023fe8bcff795bdf64d8 Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2024-08-01 18:00:45 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2024-08-01 18:00:45 +0000 fibs: Limit the WARNING message to only once when setting up with multiple fibs In main [1] this warning message is suppressed but no plans to MFC the change as the message may be still useful for users that upgrade from older releases to 14.x or 13.x. Well emitting this warning message every time increasing the fib number is confusing for users not for the feature `net.add_addr_allfibs`, let's limit it to be printed only once. 1. a48f7a2eb90b fibs: Suppress the WARNING message for setups with multiple fibs This is a direct commit to stable/14 and stable/13. PR: 280097 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D46204 sys/net/route/route_tables.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=7797719d3a8b1eb50e57c6652f744a6bffc91599 commit 7797719d3a8b1eb50e57c6652f744a6bffc91599 Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2024-08-01 18:00:45 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2024-08-01 18:04:54 +0000 fibs: Limit the WARNING message to only once when setting up with multiple fibs In main [1] this warning message is suppressed but no plans to MFC the change as the message may be still useful for users that upgrade from older releases to 14.x or 13.x. Well emitting this warning message every time increasing the fib number is confusing for users not for the feature `net.add_addr_allfibs`, let's limit it to be printed only once. 1. a48f7a2eb90b fibs: Suppress the WARNING message for setups with multiple fibs This is a direct commit to stable/14 and stable/13. PR: 280097 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D46204 (cherry picked from commit 352dd826ffdd88d26744023fe8bcff795bdf64d8) sys/net/route/route_tables.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
(In reply to Jeremy Cooper from comment #5) Suppressed the warning message in main. Limited the warning message to be printed only once in stable/14 and stable/13.
I am reopening this because the main problem was the content of the message, not problems about when it is displayed. As it stands, the current solution still retains the CONFUSING wording. I propose that the warning be changed ever so slightly so that the user can tell 1. When it was generated (when net.fibs is adjusted) 2. That the warning is a reminder, not the detection of a problem. Can you please address this? I suggested the following message: printf("%s adjusted to %d. REMINDER: " "Adding ifaddrs to all fibs has been turned off " "by default. Consider tuning %s if needed\n", "net.fibs", num_tables, "net.add_addr_allfibs");
Pardon me, that was my old wording. I think I proposed the following in my last round: + printf("%s adjusted to %d. REMINDER: " + "Interface addresses are no longer automatically added " + "to additional FIBs. Consider setting %s if needed\n", + "net.fibs", + num_tables, "net.add_addr_allfibs"); +
(In reply to Jeremy Cooper from comment #13) > I am reopening this because the main problem was the content of the message, > not problems about when it is displayed. As it stands, the current solution > still retains the CONFUSING wording. Ah, I think the original warning message > WARNING: Adding ifaddrs to all fibs has been turned off by default. Consider tuning net.add_addr_allfibs if needed says exactly what it intends. Please note `Adding ifaddrs to all fibs` is a feature although it is not highlighted in the warning message. I'd admit that `ifaddrs` is more a developer friendly word, it refers to `interface addresses` exactly. > I propose that the warning be changed ever so slightly so that the user can tell > 1. When it was generated (when net.fibs is adjusted) Now only when the first time increasing `net.fibs` and `net.add_addr_allfibs == 0`. > 2. That the warning is a reminder, not the detection of a problem. Emm, the message was introduced to be a WARNING, exactly. And a WARNING does not necessarily mean a problem is detected. In this case it is something important ( a noticeable breaking change ) that user should be aware. (In reply to Jeremy Cooper from comment #14) > Pardon me, that was my old wording. I think I proposed the following in my last round: >+ printf("%s adjusted to %d. REMINDER: " The `adjusted to` is redundant. When doing the tuning ``` # sysctl net.fibs=2 net.fibs: 1 -> 2 ``` The console should print the new value (by default). >+ "Interface addresses are no longer automatically added " >+ "to additional FIBs. Consider setting %s if needed\n", The `additional` is not accurate. If so then what is the current FIB ? I'd prefer keep `tuning`. I think it is more natural. We are tuning a sysctl knob. >+ "net.fibs", >+ num_tables, > "net.add_addr_allfibs"); >+ If you insist then I'd propose to reword `ifaddrs` to `interface addresses`. The message will end up with ``` WARNING: Adding interface addresses to all fibs has been turned off by default. Consider tuning net.add_addr_allfibs if needed ``` Is it clear enough to you ?
(In reply to Zhenlei Huang from comment #15) No, this is not clear enough for me. My issue is not with the word "ifaddrs"; it with these two points, which I will repeat again: 1. The user needs to know why the message was generated at this particular point in the boot process. 2. The user needs to know that this message is a reminder, not the detection of an immediate problem, and given just a tiny hint about how to make the message go away. As a kernel developer it may be very clear to you why the message was generated. But it's not at all clear to the average admin. This message scrolls by very quickly at boot and is VERY hard to interpret in its current form. The message needs to address both of these points. And to do that, it needs to: 1. Mention that setting "net.fibs" is what triggered the message. 2. Give actionable steps that the user can take to eliminate the problem. My wording accomplishes both of these. I just ask that you still solve both issues if you choose to change it.
(In reply to Zhenlei Huang from comment #15) Also I want to clear up a misunderstanding you may have. You wrote: > The `adjusted to` is redundant. When doing the tuning > ``` > # sysctl net.fibs=2 > net.fibs: 1 -> 2 > ``` > The console should print the new value (by default). If you `sysctl` to set the variable, sure. It will give you immediate feedback about the old and new values. But what about when the variable was set AT BOOT, by `/boot/loader.conf`? My /boot/loader.conf is ``` net.fibs=2 ``` and it triggers this warning at boot. That's what has been so irritating.
(In reply to Jeremy Cooper from comment #17) >My /boot/loader.conf is >``` >net.fibs=2 >``` > >and it triggers this warning at boot. That's what has been so irritating. I hereby protest! What is this bug report about? I found the aforementioned warning extremely useful in the past, it saved me a lot of time when after the OS upgrade all the jails lost connectivity with some parts of the network. Furthermore, I greatly appreciate the intention of melifaro@, who left this in the code. Today, after a couple of years, this warning is no longer relevant and the code generating it should probably be wiped not only from CURRENT, but also from STABLE, at least from stable/14. Cheers Marek
(In reply to Marek Zarychta from comment #18) > What is this bug report about? The Warning printed when FIBs are expanded is unhelpful and confusing and could be improved. I'm trying to improve the message, not eliminate it.