DEV9: Improve logic for getting MacAddress #10937

fjtrujy · 2024-03-16T16:31:36Z

Description of Changes

Improving logic for getting MacAddress, now it is working for Apple and FreeBSD

Previously on MacOS:

[   12.2374] DEV9: Opening adapter 'en0'...
[   12.2377] DEV9: Device uses DLT 1: EN10MB
[   12.2379] DEV9: Adapter Ok.
[   12.2418] DEV9: Unsupported OS, can't get MAC address
[   12.2420] DEV9: Failed to get MAC address for adapter
[   12.2422] DEV9: Failed to GetNetAdapter()

Now:

[  443.7172] DEV9: Opening adapter 'en0'...
[  443.7173] DEV9: Device uses DLT 1: EN10MB
[  443.7175] DEV9: Adapter Ok.

pcsx2/DEV9/AdapterUtils.cpp

lightningterror · 2024-03-24T21:34:46Z

@fjtrujy Status?

TheLastRar · 2024-04-10T15:13:18Z

Status?

fjtrujy · 2024-04-10T15:14:21Z

I will try to address these issues soon.
Thanks for the review

fjtrujy · 2024-04-10T23:04:29Z

Hello @lightningterror @TheLastRar and @stenzek, could you take a look now? not sure if I did how you wanted

TheLastRar

I note that you are iterating though all interfaces, returning the first MAC address found

This function should return the MAC address specific to the adapter passed into it.
On non-windows platforms, Adapter is typedefed to ifaddrs

I also noticed a few formatting errors, you should be able to use clang format to correct those.

pcsx2/DEV9/AdapterUtils.cpp

fjtrujy · 2024-04-11T07:02:26Z

I note that you are iterating though all interfaces, returning the first MAC address found

This function should return the MAC address specific to the adapter passed into it. On non-windows platforms, Adapter is typedefed to ifaddrs

I also noticed a few formatting errors, you should be able to use clang format to correct those.

Understood!!
I have applied that condition too!

pcsx2/DEV9/AdapterUtils.cpp

fjtrujy · 2024-04-11T23:34:41Z

I have done some improvements, I think that now is cleaner.
I was putting some Console.WriteLn to check results:

DEV9: Found adapter lo0, requested en0
DEV9: Found adapter lo0, requested en0
DEV9: Found adapter lo0, requested en0
DEV9: Found adapter lo0, requested en0
DEV9: Found adapter gif0, requested en0
DEV9: Found adapter stf0, requested en0
DEV9: Found adapter XHC20, requested en0
DEV9: Found adapter en0, requested en0
DEV9: Found MAC address F4:5C:89:BF:7D:99

Which fit with the values in the OS

Cheers

TheLastRar

Looking better now overall

pcsx2/DEV9/AdapterUtils.cpp

TheLastRar · 2024-04-12T01:23:37Z

pcsx2/DEV9/AdapterUtils.cpp

+	return macAddr;
+#elif defined(AF_LINK)
+	struct ifaddrs *tempifs, *po;
+	getifaddrs(&tempifs);


You will need to call freeifaddrs() once you are done with tempifs

Alternatively, you can use a std::unique_ptr with a custom deleter to handle this for you, like is done in the posix version of AdapterUtils::GetAdapter()

You may also want to check the return value of getifaddrs(), as it could return -1 indicating an error.

pcsx2/DEV9/AdapterUtils.cpp

fjtrujy · 2024-04-12T17:18:27Z

Once again I have done some improvements, sorry for that, please take a look again

pcsx2/DEV9/AdapterUtils.cpp

fjtrujy · 2024-04-16T20:59:47Z

Let me know if I need to something else.
Thanks!

F0bes · 2024-04-23T03:23:19Z

This is good to go if you're happy with it @stenzek

stenzek

Sorry, I should've given this a proper review earlier.

pcsx2/DEV9/AdapterUtils.cpp

stenzek · 2024-04-23T04:58:18Z

pcsx2/DEV9/AdapterUtils.cpp

+		Console.Error("DEV9: Failed to get MAC address for adapter using SIOCGENADDR");
+		return std::nullopt;
+	}
+	memcpy(&macAddr, &ifr.ifr_enaddr, 6);


Suggested change

memcpy(&macAddr, &ifr.ifr_enaddr, 6);

std::memcpy(&macAddr, &ifr.ifr_enaddr, sizeof(&macAddr));

pcsx2/DEV9/AdapterUtils.cpp

fjtrujy · 2024-04-23T09:43:52Z

Hello @stenzek !
First of all thanks for the review, I have applied all your suggestions, please take a look again 🙏

Cheers

stenzek

Otherwise LGTM

pcsx2/DEV9/AdapterUtils.cpp

fjtrujy · 2024-04-23T14:26:26Z

I have applied all your recommendations, it is way cleaner now!
Thanks

stenzek

LGTM, untested

TheLastRar

I did test this on linux (which hit the SIOCGIFHWADDR path) and it worked

However, when testing I noticed a small issue with how you had setup the pre processor conditions

pcsx2/DEV9/AdapterUtils.cpp

github-actions bot added the DEV9 label Mar 16, 2024

fjtrujy force-pushed the networkMacOS branch from 85b8167 to d56d50c Compare March 16, 2024 16:36

stenzek reviewed Mar 17, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

lightningterror reviewed Mar 17, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

lightningterror changed the title ~~Improve logic for getting MacAddress~~ DEV9: Improve logic for getting MacAddress Mar 17, 2024

lightningterror added the Hold Merge label Apr 8, 2024

fjtrujy force-pushed the networkMacOS branch 3 times, most recently from 2adc08a to 34507bf Compare April 10, 2024 22:46

TheLastRar suggested changes Apr 11, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

fjtrujy force-pushed the networkMacOS branch from 34507bf to 4005350 Compare April 11, 2024 07:02

fjtrujy force-pushed the networkMacOS branch from 4005350 to fec416c Compare April 11, 2024 07:18

stenzek reviewed Apr 11, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

fjtrujy force-pushed the networkMacOS branch from fec416c to 066bcfa Compare April 11, 2024 07:58

TheLastRar reviewed Apr 11, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

fjtrujy force-pushed the networkMacOS branch 3 times, most recently from ded282a to b26ddf9 Compare April 11, 2024 21:31

fjtrujy marked this pull request as draft April 11, 2024 22:25

fjtrujy force-pushed the networkMacOS branch from b26ddf9 to 7cce419 Compare April 11, 2024 23:28

fjtrujy marked this pull request as ready for review April 11, 2024 23:32

fjtrujy force-pushed the networkMacOS branch from 7cce419 to 5b496f3 Compare April 11, 2024 23:34

fjtrujy force-pushed the networkMacOS branch from 5b496f3 to b23973b Compare April 11, 2024 23:35

TheLastRar suggested changes Apr 12, 2024

View reviewed changes

fjtrujy force-pushed the networkMacOS branch from b23973b to 97d4e06 Compare April 12, 2024 17:17

TheLastRar reviewed Apr 12, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

fjtrujy force-pushed the networkMacOS branch from 97d4e06 to 6d92f58 Compare April 12, 2024 19:09

TheLastRar approved these changes Apr 12, 2024

View reviewed changes

lightningterror requested a review from stenzek April 14, 2024 09:15

fjtrujy force-pushed the networkMacOS branch from 6d92f58 to a70d603 Compare April 16, 2024 20:58

stenzek reviewed Apr 23, 2024

View reviewed changes

fjtrujy force-pushed the networkMacOS branch from a70d603 to 2edf250 Compare April 23, 2024 09:24

stenzek reviewed Apr 23, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

pcsx2/DEV9/AdapterUtils.cpp Show resolved Hide resolved

fjtrujy force-pushed the networkMacOS branch from 2edf250 to a082ec3 Compare April 23, 2024 13:26

stenzek approved these changes Apr 23, 2024

View reviewed changes

lightningterror removed the Hold Merge label Apr 23, 2024

TheLastRar suggested changes Apr 25, 2024

View reviewed changes

pcsx2/DEV9/AdapterUtils.cpp Outdated Show resolved Hide resolved

fjtrujy added 2 commits April 26, 2024 22:49

Change error to normal message

9d90fa1

DEV9: Improve logic for getting MacAddress

48fd502

fjtrujy force-pushed the networkMacOS branch from a082ec3 to 48fd502 Compare April 26, 2024 20:49

TheLastRar approved these changes Apr 26, 2024

View reviewed changes

stenzek merged commit d63966b into PCSX2:master Apr 28, 2024
12 checks passed

TheLastRar mentioned this pull request Apr 29, 2024

DEV9: MAC address fetch implementation for MacOS/FreeBSD #7035

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEV9: Improve logic for getting MacAddress #10937

DEV9: Improve logic for getting MacAddress #10937

fjtrujy commented Mar 16, 2024 •

edited

lightningterror commented Mar 24, 2024

TheLastRar commented Apr 10, 2024

fjtrujy commented Apr 10, 2024

fjtrujy commented Apr 10, 2024

TheLastRar left a comment •

edited

fjtrujy commented Apr 11, 2024

fjtrujy commented Apr 11, 2024 •

edited

TheLastRar left a comment •

edited

TheLastRar Apr 12, 2024

fjtrujy commented Apr 12, 2024

fjtrujy commented Apr 16, 2024

F0bes commented Apr 23, 2024

stenzek left a comment

stenzek Apr 23, 2024

fjtrujy commented Apr 23, 2024

stenzek left a comment

fjtrujy commented Apr 23, 2024 •

edited

stenzek left a comment

TheLastRar left a comment •

edited

	memcpy(&macAddr, &ifr.ifr_enaddr, 6);
	std::memcpy(&macAddr, &ifr.ifr_enaddr, sizeof(&macAddr));

DEV9: Improve logic for getting MacAddress #10937

DEV9: Improve logic for getting MacAddress #10937

Conversation

fjtrujy commented Mar 16, 2024 • edited

Description of Changes

lightningterror commented Mar 24, 2024

TheLastRar commented Apr 10, 2024

fjtrujy commented Apr 10, 2024

fjtrujy commented Apr 10, 2024

TheLastRar left a comment • edited

Choose a reason for hiding this comment

fjtrujy commented Apr 11, 2024

fjtrujy commented Apr 11, 2024 • edited

TheLastRar left a comment • edited

Choose a reason for hiding this comment

TheLastRar Apr 12, 2024

Choose a reason for hiding this comment

fjtrujy commented Apr 12, 2024

fjtrujy commented Apr 16, 2024

F0bes commented Apr 23, 2024

stenzek left a comment

Choose a reason for hiding this comment

stenzek Apr 23, 2024

Choose a reason for hiding this comment

fjtrujy commented Apr 23, 2024

stenzek left a comment

Choose a reason for hiding this comment

fjtrujy commented Apr 23, 2024 • edited

stenzek left a comment

Choose a reason for hiding this comment

TheLastRar left a comment • edited

Choose a reason for hiding this comment

fjtrujy commented Mar 16, 2024 •

edited

TheLastRar left a comment •

edited

fjtrujy commented Apr 11, 2024 •

edited

TheLastRar left a comment •

edited

fjtrujy commented Apr 23, 2024 •

edited

TheLastRar left a comment •

edited