Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MacAddressUtil prefers wrong MAC #5522

Open
CodingFabian opened this issue Jul 11, 2016 · 10 comments
Open

MacAddressUtil prefers wrong MAC #5522

CodingFabian opened this issue Jul 11, 2016 · 10 comments
Assignees

Comments

@CodingFabian
Copy link
Contributor

When using MacAddressUtil on a Windows server in Amazon, it selects the "wrong" best match.

Have a look at this output from ifconfig /all

Ethernet adapter Ethernet:
​
   Connection-specific DNS Suffix  . : us-west-2.compute.internal
   Description . . . . . . . . . . . : AWS PV Network Device #0
   Physical Address. . . . . . . . . : 02-57-3F-D8-7A-E9
   DHCP Enabled. . . . . . . . . . . : Yes
   Autoconfiguration Enabled . . . . : Yes
   Link-local IPv6 Address . . . . . : fe80::91d4:6238:2922:64e9%12(Preferred)
   IPv4 Address. . . . . . . . . . . : 172.31.38.68(Preferred)
   Subnet Mask . . . . . . . . . . . : 255.255.240.0
   Lease Obtained. . . . . . . . . . : Monday, June 20, 2016 11:22:08 AM
   Lease Expires . . . . . . . . . . : Monday, July 11, 2016 11:53:20 AM
   Default Gateway . . . . . . . . . : 172.31.32.1
   DHCP Server . . . . . . . . . . . : 172.31.32.1
   DHCPv6 IAID . . . . . . . . . . . : 203554827
   DHCPv6 Client DUID. . . . . . . . : 00-01-00-01-1E-96-7D-CB-02-57-3F-D8-7A-E9
   DNS Servers . . . . . . . . . . . : 172.31.0.2
   NetBIOS over Tcpip. . . . . . . . : Enabled
​
Tunnel adapter isatap.us-west-2.compute.internal:
​
   Media State . . . . . . . . . . . : Media disconnected
   Connection-specific DNS Suffix  . : us-west-2.compute.internal
   Description . . . . . . . . . . . : Microsoft ISATAP Adapter
   Physical Address. . . . . . . . . : 00-00-00-00-00-00-00-E0
   DHCP Enabled. . . . . . . . . . . : No
   Autoconfiguration Enabled . . . . : Yes
​
Tunnel adapter Local Area Connection* 12:
​
   Connection-specific DNS Suffix  . :
   Description . . . . . . . . . . . : Teredo Tunneling Pseudo-Interface
   Physical Address. . . . . . . . . : 00-00-00-00-00-00-00-E0
   DHCP Enabled. . . . . . . . . . . : No
   Autoconfiguration Enabled . . . . : Yes
   IPv6 Address. . . . . . . . . . . : 2001:0:5ef5:79fb:2cf4:f2b:53e0:d9bb(Preferred)
   Link-local IPv6 Address . . . . . : fe80::2cf4:f2b:53e0:d9bb%13(Preferred)
   Default Gateway . . . . . . . . . : ::
   DHCPv6 IAID . . . . . . . . . . . : 385875968
   DHCPv6 Client DUID. . . . . . . . : 00-01-00-01-1E-96-7D-CB-02-57-3F-D8-7A-E9
   NetBIOS over Tcpip. . . . . . . . : Disabled

It is pretty obvious that 172.31.38.68+ 02-57-3F-D8-7A-E9 is the best match. But MacAddressUtil disagrees.

What happens is that netty first considers this as the best, but then it checks the tunnel devices:
and compares 02:57:3f:d8:7a:e9 with 00:00:00:00:00:00:00:e0.

        // Prefer globally unique address.
        if ((current[0] & 2) == 0) {
            if ((candidate[0] & 2) == 0) {
                // Both current and candidate are globally unique addresses.
                return 0;
            } else {
                // Only current is globally unique.
                return 1;
            }
        } else {
            if ((candidate[0] & 2) == 0) {
                // Only candidate is globally unique.
                return -1;
            } else {
                // Both current and candidate are non-unique.
                return 0;
            }
        }

It turns out that 00:00:00:00:00:00:00:e0 is globally unique (which unfortunately is not true...)

Even if that would be fixed, then it still would be choosen via:

                } else if (res == 0) {
                    // Cannot tell the difference.  Choose the longer one.
                    if (bestMacAddr.length < macAddr.length) {
                        replace = true;
                    }
                }

Imho the best would be to enhance

            if (iface.isVirtual()) {
                continue;
            }

to exclude these synthetic devices?

Any other ideas? Opinions?

@normanmaurer
Copy link
Member

@CodingFabian isVirtual() sounds ok. Interested in submitting a PR ?

@CodingFabian
Copy link
Contributor Author

@normanmaurer the isVirtual check is already in place, but these two interfaces are not considered virtual by java.

@CodingFabian
Copy link
Contributor Author

I wonder if it might be better to score interfaces first and only use mac scoring as tie breaker.
currently the mac looks better but the interface scoring would score the first interface much better

@normanmaurer
Copy link
Member

@trustin thoughts ?

@CodingFabian
Copy link
Contributor Author

isPointToPoint() returns true for those interfaces. maybe that could be used to rank them down?

@normanmaurer
Copy link
Member

@CodingFabian sounds good... Can you provide a pr ?

@normanmaurer normanmaurer self-assigned this Jul 14, 2016
@normanmaurer normanmaurer added this to the 4.0.39.Final milestone Jul 14, 2016
@CodingFabian
Copy link
Contributor Author

ok, the change would be that we first rank interfaces, and use mac ranking as tie breaker.
additionally we rank p2p worse than non-p2p

Sounds for me like a possibly large impact change. what is the significance of the chosen mac in netty?
I guess i have to write some test cases :)

@normanmaurer
Copy link
Member

@CodingFabian are you still interested in fixing this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants