Skip to content

Conversation

@wfurt
Copy link
Member

@wfurt wfurt commented Sep 30, 2020

This is regression to 3.1 caused by dotnet/corefx#41490. Somehow the lack of MAC address got unnoticed as GetPhysicalAddress() simple returns empty address instead of failing.

I modified tests to ensure valid length at least for Ethernet type.
Provided repro now shows correct output:

Selected interface:enp0s5
MAC:001C42E4302E
False
Bytes:6
Hash code:-465423824

fixes #42870

@wfurt wfurt added area-System.Net os-linux Linux OS (any supported distro) labels Sep 30, 2020
@wfurt wfurt added this to the 5.0.0 milestone Sep 30, 2020
@wfurt wfurt requested review from a team and danmoseley September 30, 2020 00:19
@wfurt wfurt self-assigned this Sep 30, 2020
@ghost
Copy link

ghost commented Sep 30, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@geoffkizer
Copy link
Contributor

This is regression to 3.1 caused by #41490

Looks like wrong issue?

@wfurt
Copy link
Member Author

wfurt commented Sep 30, 2020

updated. correct number wrong repo - this was done back in corefx before the consolidation.

@geoffkizer
Copy link
Contributor

Can you help me understand what in that change broke this?

@wfurt
Copy link
Member Author

wfurt commented Sep 30, 2020

In the past, we would walk the interface list and then for each address type we would invoke appropriate managed callback which would be building the resulting structure. That was inefficient as well as we could not handle certain cases. So with the change we put everything to single array passed from native side and we processes it in two linear passes on managed side.
The issue is that we never processed returned AddressBytes (effectively ignoring) them) and that created PhysicalAddress with no content. It got missed since we did not have test to validate that besides existence of the object. The fix is to create PhysicalAddress from returned data.

I hope this makes some sense @geoffkizer. Updated test would be sufficient to catch this e.g. test for some specific data.

@danmoseley
Copy link
Member

Thanks for quick fix @wfurt. Let's get a 5.0 PR up with the template?

Are there any other missing test cases you see around this API?

@wfurt wfurt merged commit d52ee3a into dotnet:master Sep 30, 2020
@wfurt
Copy link
Member Author

wfurt commented Sep 30, 2020

/backport release/5.0

@wfurt
Copy link
Member Author

wfurt commented Sep 30, 2020

/backport to release/5.0

@github-actions
Copy link
Contributor

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/279989116

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@karelz karelz modified the milestones: 5.0.0, 6.0.0 Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net os-linux Linux OS (any supported distro)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression - NET5.0 RC1 returns empty MAC address for network interface on Linux

4 participants