Bug 205263 - Fix for netwait boot order issues and other bugs
Summary: Fix for netwait boot order issues and other bugs
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 10.2-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Ian Lepore
URL:
Keywords: easy, needs-qa, patch, patch-ready
Depends on:
Blocks: 205186
  Show dependency treegraph
 
Reported: 2015-12-12 05:05 UTC by Brendan Molloy
Modified: 2016-01-02 04:19 UTC (History)
2 users (show)

See Also:
ian: mfc-stable10+
koobs: mfc-stable9?


Attachments
netwait fix patch (3.08 KB, patch)
2015-12-12 05:05 UTC, Brendan Molloy
no flags Details | Diff
netwait fix patch r2 (3.08 KB, patch)
2015-12-12 05:43 UTC, Brendan Molloy
no flags Details | Diff
netwait fix patch r3 (3.27 KB, patch)
2015-12-12 23:40 UTC, Brendan Molloy
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan Molloy 2015-12-12 05:05:43 UTC
Created attachment 164144 [details]
netwait fix patch

There are a few issues with using netwait with late-appearing devices such as USB ethernet controllers that are fixed in the proposed patch:

 - $netwait_if_timeout would always bail out on first iteration if no interface found
 - Added netwait to NETWORKING after netif to ensure it runs at the correct time
 - $netwait_ip was mandatory even though making it optional does not break anything
 - Minor improvements to console text to make logging clearer

With this patch, my system boots properly, with correct routing.

I hope the patch is of some use, and look forward to your feedback. :)
Comment 1 Ian Lepore freebsd_committer freebsd_triage 2015-12-12 05:27:31 UTC
This patch won't work.  You can't make netwait run before routing when the way netwait works is to ping an arbitrary ip address which may require routing to be reachable.

I still think the root cause of the problem would be solved by pre-loading the if_axe module via loader.conf instead of relying on devd (which runs after netif) to load it.
Comment 2 Brendan Molloy 2015-12-12 05:40:18 UTC
(In reply to Ian Lepore from comment #1)

Ah yes, you are right. I will provide a better patch.
Comment 3 Brendan Molloy 2015-12-12 05:43:45 UTC
Created attachment 164145 [details]
netwait fix patch r2

Changes BEFORE: routing to REQUIRE: routing
Comment 4 Brendan Molloy 2015-12-12 05:47:23 UTC
(In reply to Ian Lepore from comment #1)

Further to your point about /boot/loader.conf being the correct way to solve this, that may well be the case. It certainly feels a lot better. However, while this patch might provide a less ideal solution, it doesn't require extra configuration to make the interface work, while not adding the patch has the consequence that my bootup is delayed by an extra 30 seconds while NFS cannot mount and stalls temporarily, which only serves to confuse the user further.

Also, I know that changing $netwait_ip to be optional is potentially a controversial move, and I am quite happy to remove that from the patch if it is seen as entirely disagreeable, but I saw no use for it as if it weren't for the fact I am waiting for a late interface to arrive, I wouldn't be pinging during the boot sequence anyway, so it is not useful in my use case of netwait.

I hope my explanation has been helpful!
Comment 5 Ian Lepore freebsd_committer freebsd_triage 2015-12-12 17:54:12 UTC
I agree that this change is shaping up nicely to help folks who use usb-based network interfaces that rely on devd to start up.

I think any potential controversy over making the ip list optional might evaporate if the logic were changed to look at both the ip and the interface list and if they're both empty say something like "Warning: No interfaces or IP addresses listed, nothing to wait for".  If either list is non-empty, just quietly do whatever work was requested.
Comment 6 Brendan Molloy 2015-12-12 23:40:59 UTC
Created attachment 164165 [details]
netwait fix patch r3

Added the conditional check for if both netwait_if and netwait_ip fields are blank.
Comment 7 Ian Lepore freebsd_committer freebsd_triage 2016-01-02 04:19:22 UTC
Fix commited as r292752, but I forgot to cite this PR number in the commit.