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

EXI: Refactor Windows BBA-TAP interface to a read thread, crash fixes, cleanups #3432

Merged
merged 4 commits into from Feb 26, 2016

Conversation

stenzek
Copy link
Contributor

@stenzek stenzek commented Jan 2, 2016

The old interface apparently had performance issues, and was prone to crashes (one of them due to the fact that writes to the tap interface were done asynchronously, using an OVERLAPPED structure on the stack, which obviously does not exist once the method returned)

I don't have many ways of testing this, I've tried with sockettest.dol, and this works okay. Keep in mind, some people have observed that this is slower to get an address on Windows over Linux.

I believe this is correct behavior, because Windows runs spanning tree on its bridge interface, dropping all packets for 20 seconds or so (L2 loop protection). Apparently there's a registry tweak to disable STP, but I had no luck with that as I only got the bridge to work once (seems it's flaky in W10, or my install is hosed). Running NAT on the interface instead brings the time down to around 4-5 seconds (comparable to Linux?)

Review on Reviewable

@stenzek stenzek force-pushed the bba-tap-win branch 2 times, most recently from be8aa54 to ad2ff21 Compare January 2, 2016 13:12
@JMC47
Copy link
Contributor

JMC47 commented Jan 6, 2016

I need to retest this; I first thought I couldn't connect to tap, but then I realized I somehow broke my taps. I'll have to reinstall I guess.

@ThatInvaderGuy
Copy link

This fixes the constant crashing on my desktop pc. My laptop that had no problems before works too. Tested Kirby Air Ride and everything was working fine. 1080 Avalanche still disconnects when selecting a course, but it always did that. MK:DD is next on my list to test.

@@ -27,7 +28,7 @@ void GenerateMacAddress(const MACConsumer type, u8* mac)
break;
}

srand((unsigned int)time(nullptr));
srand(Common::Timer::GetTimeMs());

This comment was marked as off-topic.

@stenzek
Copy link
Contributor Author

stenzek commented Jan 7, 2016

Regarding the casting/buffer issue, I've changed it slightly, it allocates a single struct with an inline buffer for small frames, for larger frames it'll do a second allocation for the buffer. So no casting to/from u8, just to the write class to call the destructor, and not needing a second allocation at all in most cases.

Regarding the random MAC change, the only disadvantage to this that I see is if there are any games that still work online (I saw the list on the wiki, but are the servers still online for these games?), and you're bridging to LAN, you'll be creating DHCP leases each time you boot the emulator.

I suppose another way around this could be replacing the last byte with the index of the adapter chosen or something, but this would require more refactoring, as the MAC address is set at boot, not reset time, which is when the adapter is actually opened.

static const size_t SMALL_FRAME_BUFFER_SIZE = 200;
struct AsyncIOWritePacket final : public AsyncIOHeader
{
u8 small_frame_buffer[SMALL_FRAME_BUFFER_SIZE];

This comment was marked as off-topic.

@stenzek stenzek changed the title [Testing required] EXI: Refactor Windows BBA-TAP interface to use IO completion ports EXI: Refactor Windows BBA-TAP interface to a read thread, crash fixes, cleanups Jan 31, 2016
This also has the added benefit of not crashing under most circumstances.
Includes a few other changes, including replacing the atomic<bool> with a
Flag, as well as adding a flag for indicating read thread shutdown.
@JMC47
Copy link
Contributor

JMC47 commented Jan 31, 2016

Requesting a merge on this: it fixes a ton of issues/crashes and makes BBA actually useable for Phantasy Star Online on Windows.

@delroth delroth added this to the Dolphin Release 5.0 milestone Feb 1, 2016
@lioncash lioncash removed this from the Dolphin Release 5.0 milestone Feb 26, 2016
@delroth delroth added this to the Dolphin Release 5.0 milestone Feb 26, 2016
delroth added a commit that referenced this pull request Feb 26, 2016
EXI: Refactor Windows BBA-TAP interface to a read thread, crash fixes, cleanups
@delroth delroth merged commit 1d07fee into dolphin-emu:master Feb 26, 2016
@stenzek stenzek deleted the bba-tap-win branch November 13, 2016 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants