Opened 8 years ago

Closed 8 years ago

#1361 closed defect (fixed)

IfaceMgr.loDetect test fails on NetBSD

Reported by: tomek Owned by: tomek
Priority: low Milestone: Sprint-DHCP-20111221
Component: dhcp6 Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

See this log for details:
http://git.bind10.isc.org/~tester/builder/BIND10/20111027195652-NetBSD5-amd64/logs/unittests.out

There is no interface detection implemented yet. IfaceMgr? currently uses stub code to detect if there is lo (Linux) or lo0 (BSD) loopback interface. The code mostly works, but it breaks down on some NetBSDs.

Mid-term solution is to simplify stub code, but the real solution is to implement interface detection for real. This ticket is about making the stub code working on NetBSD. See ticket #1237 for "proper" interface detection.

Subtickets

Change History (10)

comment:1 Changed 8 years ago by tomek

  • Milestone changed from New Tasks to Sprint-DHCP-20111109
  • Owner set to tomek
  • Status changed from new to accepted

comment:2 follow-up: Changed 8 years ago by tomek

  • Owner changed from tomek to UnAssigned
  • Status changed from accepted to reviewing

Fix implemented. It works on Linux, Mac OS X and NetBSD.

Code on branch trac1361. Please review.

Last edited 8 years ago by tomek (previous) (diff)

comment:3 Changed 8 years ago by jreed

I looked at the branch. It does fix the problem for me on netBSD (at least it doesn't fail):

[ RUN      ] IfaceMgrTest.loDetect
This is BSD, using lo0 as loopback.
[       OK ] IfaceMgrTest.loDetect (0 ms)

Nevertheless, I am confused why this is a unit test, as far as I can tell this item doesn't test our code. Other tests do use the result (the name of the loopback device is defined).

comment:4 Changed 8 years ago by tomek

That is a workaround for missing interface detection. Its purpose is to detect what is a name of looppack interface: lo or lo0. DHCPv6 uses link-local multicast addresses. During socket joining multicast, interface name has to be specified. The whole thing is a stub implementation before proper interface detection is implemented. To run most of the remaining tests in a portable (on both Linux that use lo and BSD that use lo0), I need to know loopback interface name. Important point here is that by end of November, we are planning to have proper interface detection implemented. By that time this stub code will go away.

comment:5 in reply to: ↑ 2 Changed 8 years ago by jinmei

Replying to tomek:

Fix implemented. It works on Linux, Mac OS X and NetBSD.

Code on branch trac1361. Please review.

I already gave it a go for urgent care.

Some additional comments

  • s/or/nor/?
    +        cout << "Failed to detect loopback interface. Neither "
    +             << "lo or lo0 worked. I give up." << endl;
    
  • the use of sprintf is risky. In this case we only use constant string, so we should be fine with char*:
    namespace {  // btw I suggest hiding this stuff in an unnamed namedpace
    const char* LOOPBACK = "lo0";
    
    ....
        if (if_nametoindex("lo")>0) {
            LOOPBACK = "lo";
    ...
    
  • according to BIND 9 coding guideline we add spaces around '>', etc.
        if (if_nametoindex("lo") > 0) {
    
  • this seems to be an ugly hack.
    +        ASSERT_TRUE(false);
    
  • when you complete this branch, please delete it from the public repo.

comment:6 Changed 8 years ago by jinmei

  • Owner changed from UnAssigned to tomek

comment:7 Changed 8 years ago by tomek

  • Component changed from dhcp to dhcp6

comment:8 Changed 8 years ago by tomek

Vorner suggested that much better would be to use FAIL() instead of ASSERT_TRUE(false).

comment:9 Changed 8 years ago by tomek

Changes:

  • or => nor
  • spaces around >
  • ASSERT_TRUE(false) replaced by FAIL()
  • removed trac1361 branch

Checked in to #992 (d6e33479365c8f8f62ef2b9aa5548efe6b194601) as 1361 was already merged.

Last edited 8 years ago by tomek (previous) (diff)

comment:10 Changed 8 years ago by tomek

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.