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

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

Closed
wants to merge 1 commit into from

Conversation

TellowKrinkle
Copy link
Member

Description of Changes

Adds a function to read MAC addresses on macOS

With this, I can successfully ping my emulated PS2 with adapters in bridge mode, but not switched. Better than nothing at least.

Rationale behind Changes

Networking on macOS

Suggested Testing Steps

Test networking things on macOS. I've only tested pinging the ps2sdk's tcpip-basic sample code.

Note: I was unable to ping from the computer running PCSX2, but other devices on the network worked fine.

Note: You'll need write access to the /dev/bpf* system files. If you have wireshark (or homebrew's wireshark-chmodbpf package) installed, that should handle it for you. Otherwise, chgrp/chmod those so the user you're running PCSX2 as has access.

@kokroo

@github-actions github-actions bot added the DEV9 label Sep 11, 2022
@kokroo
Copy link

kokroo commented Sep 14, 2022

Description of Changes

Adds a function to read MAC addresses on macOS

With this, I can successfully ping my emulated PS2 with adapters in bridge mode, but not switched. Better than nothing at least.

Rationale behind Changes

Networking on macOS

Suggested Testing Steps

Test networking things on macOS. I've only tested pinging the ps2sdk's tcpip-basic sample code.

Note: I was unable to ping from the computer running PCSX2, but other devices on the network worked fine.

Note: You'll need write access to the /dev/bpf* system files. If you have wireshark (or homebrew's wireshark-chmodbpf package) installed, that should handle it for you. Otherwise, chgrp/chmod those so the user you're running PCSX2 as has access.

@kokroo

Sweet, for some reason I cannot see this in my GitHub notifications.
How may I help you with the testing and development, and how do I send you money for a pizza and beers?

Cheers!

@TellowKrinkle
Copy link
Member Author

How may I help you with the testing and development

Can you test this PR with some actual online games and see if it works there?

Check the log viewer and make sure it says it successfully opened the interface before saying it doesn't work. If it fails it'll be printed in red in the log.

@@ -47,4 +47,5 @@ namespace AdapterUtils
std::vector<PacketReader::IP::IP_Address> GetGateways(ifaddrs* adapter);
std::vector<PacketReader::IP::IP_Address> GetDNS(ifaddrs* adapter);
#endif
bool GetAdapterMAC(u8 (&mac)[6], const char* adapter_name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other functions in this file take either PIP_ADAPTER_ADDRESSES or ifaddrs*, would it be possible to do the same here?
Although looking at PCAPAdapter::PCAPAdapter(), this could end up potentially duplicating some code.
It would however allow us to remove some code in SocketAdapter::SocketAdapter()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both the linux and BSD implementations don't use an ifaddrs here, and PCAPAdapter::PCAPAdapter() doesn't have one when it wants a MAC address.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetGateways() and GetDNS() also doesn't need ifaddrs over a string
It's more of a hold over from windows code, where PIP_ADAPTER_ADDRESSES, while containing most of the information needed, was used to identify a found/matched adapter. when porting to Linux ifaddrs was used as a drop in replacement.

It is, however, probably too much effort at this point to have GetAdapterMAC() conform to this, given the issue in PCAPAdapter::PCAPAdapter()

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, I think the easiest way will be to have a hacky solution where a few portions in each function are overridden if the code is running on macOS. But that leads to messy, unmaintainable code.

Do you have a strategy in mind to tackle this? I am new to this codebase and I'm still learning my way around.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably more nit picking at this point I think

I do want to give the pcap code another look separately, as I've gotten a report of a game behaving differently between this and my old plugin, that may, however, just be unrelated emulation change affecting things.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably more nit picking at this point I think

I do want to give the pcap code another look separately, as I've gotten a report of a game behaving differently between this and my old plugin, that may, however, just be unrelated emulation change affecting things.

What game was it and what changed?

@@ -31,7 +31,7 @@
namespace AdapterUtils
{
#ifdef _WIN32
bool GetWin32Adapter(const std::string& name, PIP_ADAPTER_ADDRESSES adapter, std::unique_ptr<IP_ADAPTER_ADDRESSES[]>* buffer);
bool GetWin32Adapter(const char* name, PIP_ADAPTER_ADDRESSES adapter, std::unique_ptr<IP_ADAPTER_ADDRESSES[]>* buffer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason from switching from std::string& to char*? Just curious

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that we don't have to create new std::strings from char*s just to call the method

We can't use a std::string_view because these get passed to c functions which expect c (null-terminated) strings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that wasn't an issue before as anywhere this was called before either already had the name as a std::string or needed to create a std::string for another purpose anyway.

I see in this PR that you now call this function in GetAdapterMAC(), which admittedly would be easier to have take a char* vs a std::string

@TheLastRar
Copy link
Contributor

Didn't think that failing to get the mac address would block pcap from working, seems I forgot it was needed for a pcap filter used in switched mode (and also a missing check that the getting the mac address succeeded)

I'll be interested to see if that was all that was needed to get network working on macOS

@kokroo
Copy link

kokroo commented Sep 15, 2022

Screen Shot 2022-09-15 at 11 36 45 PM

@TellowKrinkle No success. I am able to go to the last stage of the network configurator. I added the custom DNS entry "45.7.228.197" from ps2online.com and it shows the above. I can confirm the above server is working, and also accessible from my machine. My actual PS2 is on the same network and connects to this custom DNS server just fine.

Have I configured something wrong in the settings? If you want, you can see my screen on Teamviewer and experiment.

@kokroo
Copy link

kokroo commented Sep 15, 2022

@TellowKrinkle Okay, so I changed the settings to this (sockets, auto), and it worked! I have been able to enter the lobby! There are no players online on this game, so haven't been able to test multiplayer, but I guess it'll work. I will report back when I try multiplayer on another game. Cheers for this! Can I buy you a pizza now?

Screen Shot 2022-09-16 at 1 37 51 AM

@kokroo
Copy link

kokroo commented Sep 16, 2022

@TellowKrinkle So with sockets, I am able to enter lobbies on NFS Underground and Midnight Club 3 and see other games too. Whenever I try to join them, it fails. I have no other games to test online functionality with, but according to what AirGamer said on discord, PCAP Bridged mode will be needed for full functionality since socket connections don't have complete ethernet frames.

@ppamorim
Copy link

On Underground 2 I am getting this screen:

Screenshot 2022-09-16 at 21 00 30

@kokroo
Copy link

kokroo commented Sep 19, 2022

ppamorim

@ppamorim Is this PCAP Bridged mode or Sockets?

@ppamorim
Copy link

ppamorim

@ppamorim Is this PCAP Bridged mode or Sockets?

I tried both. No change.

@TheLastRar
Copy link
Contributor

ppamorim

@ppamorim Is this PCAP Bridged mode or Sockets?

I tried both. No change.

What are your settings?

@kokroo
Copy link

kokroo commented Sep 25, 2022

ppamorim

@ppamorim Is this PCAP Bridged mode or Sockets?

I tried both. No change.

What are your settings?

I think they used the ps2online.com DNS.

I can confirm that in over 3 games that I own that work with that server, all of them allow me to get to the lobby, and see rooms/lobbies/matches, but it never lets me enter.

This only works with Sockets. PCAP bridged mode doesn't even allow me to go online. The only improvement I see over the previous versions of PCSX2, is that network configuration mode inside the emulator didn't even show the PS2 Network adapter in older builds, but now it does show that.

@kokroo
Copy link

kokroo commented Nov 8, 2022

@TellowKrinkle Hello, is there any way I could help you with this feature? I am available for testing any builds against the online games I possess.

@fjtrujy
Copy link
Contributor

fjtrujy commented Dec 21, 2022

Hello @TellowKrinkle ,
any chance of rebasing or resuming this work?

Cheers

@kokroo
Copy link

kokroo commented Jan 2, 2023

Hello @TellowKrinkle , any chance of rebasing or resuming this work?

Cheers

I think no one is working on this right now. I tried to take a crack at it but looks like it's beyond my capabilities. It's quite unfortunate since we do have online PS2 servers these days, but Mac users cannot join them :(

I hope somebody capable takes this up in the future if they can contribute their time. Wishing for the best!

@Uzarkis
Copy link

Uzarkis commented Oct 25, 2023

I hope somebody capable takes this up in the future if they can contribute their time. Wishing for the best!

Me wishes the best too. We need more improvements for mac.

@checktext00
Copy link

First of all, thank you for working on and making PCSX2, it's really great to have game emulation on a computer.

After installing on macOS, I got this scary warning in Little Snitch with the Endpoint Security System Extension installed

PCSX2_bpf0

and I found this thread. According to Little Snitch firewall documentation about the Berkeley Packet Filter:

BPF allows injection of data packets at the network interface. This means that a (privileged) app which opens a BPF device can send any data packet to any destination. The packet is injected directly at the network interface layer, circumventing all firewalls.

I even have the automatic update check disabled, and this throws up a scary warning in Little Snitch

(this only seems to happen after opening the PCSX2 settings menu window the first time after launching the app)

after running sudo opensnoop -n PCSX2 I saw that PCSX2 tried to access both /dev/bpf and /dev/bpf0

using
macOS Big Sur 11.7.10
PCSX2-v1.7.5637.app

Is there a way of making networking work without this bpf access? Thanks again for making PCSX2

@TellowKrinkle
Copy link
Member Author

Is there a way of making networking work without this bpf access?

Only the (not currently working) pcap-based backends should require bpf access

If you're getting that warning without a pcap adapter enabled, that's a bug and you can report it in a separate issue

@checktext00
Copy link

Is there a way of making networking work without this bpf access?

Only the (not currently working) pcap-based backends should require bpf access

If you're getting that warning without a pcap adapter enabled, that's a bug and you can report it in a separate issue

Ok, I don't think I have a pcap adapter enabled as I don't really know what that is, I have made a new issue here with some extra info at the bottom

@TheLastRar
Copy link
Contributor

TheLastRar commented Apr 29, 2024

I believe this pr can be closed now that #10937 is merged, which added the same functionality

@fjtrujy
Copy link
Contributor

fjtrujy commented Apr 29, 2024

I believe this pr can be closed now that #10937 is merged, which added the same functionality

I was actually double checking it, but I saw how this PR was also changing other files, not sure if it is because it was a legacy implementation

@TheLastRar
Copy link
Contributor

TheLastRar commented Apr 29, 2024

I believe this pr can be closed now that #10937 is merged, which added the same functionality

I was actually double checking it, but I saw how this PR was also changing other files, not sure if it is because it was a legacy implementation

That was mostly because the code for getting the mac address was duplicated over different files.
That code was unified into AdapterUtils sometime between this pr and yours

@lightningterror
Copy link
Contributor

Aight gonna close if it's already integrated.

@TellowKrinkle TellowKrinkle deleted the MacDEV9 branch April 30, 2024 01:15
@kokroo
Copy link

kokroo commented May 2, 2024

Does this bring us a step closer to online play on PCSX2 for macOS?
Thanks!

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

Successfully merging this pull request may close these issues.

None yet

10 participants