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

IOS/Network: Emulate socket fd table #5432

Merged
merged 9 commits into from Jun 4, 2017
Merged

Conversation

@sepalani
Copy link
Contributor

@sepalani sepalani commented May 15, 2017

This PR tries to emulate a Wii fd table. Dolphin was using the host's fd when emulating sockets, while it shouldn't be a problem, it seems that in some cases (like the Netflix channel) there is a check against that fd value.

Regarding the Netflix channel, if the fd value is greater than 23 it returns a EBADF error (which consequently is sent to SO commands like CONNECT). I assume, it's probably using select/poll to monitor sockets and that it does a sanity check against a specified limit (max_fd?).

Currently, the fd values are computed empirically. It will need a hardware test to confirm that it matches the Wii's behaviour. It also assumes that it has a separate table for other (non-socket) fd generated. Otherwise, the fd table will need to be implemented somewhere else in IOS code. Not the case anymore, thanks to @shuffle2.

TODO

Based on @shuffle2's gist: https://gist.github.com/shuffle2/4541e4e96ba3e04a34bd36b0e07d3013#file-so_socket-c-L71

  • Create the socket fd table
  • Replace logs using host socket fd with wii fd
  • Handle SO_ENETRESET (39)
  • Handle SO_EAFNOSUPPORT (5)
  • Handle SO_EPROTONOSUPPORT (68)
  • Handle SO_EMFILE (33)
  • Handle SO_EPROTOTYPE (69)
  • Handle SO_ENOMEM (49)

Ready to be reviewed & merged.

{
if (fd >= 0)
{
WiiSocket& sock = WiiSockets[fd];
s32 wii_fd = 3;

This comment has been minimized.

@leoetlino

leoetlino May 15, 2017
Member

Is there a limit in IOS?

This comment has been minimized.

@sepalani

sepalani May 15, 2017
Author Contributor

No idea. That's why it needs a hardware test to know at least the following:

  • the max_fd value (if any)
  • the maximum number of open sockets the Wii can support
  • how are fd values given and does it have a separate table for sockets

This comment has been minimized.

@shuffle2

shuffle2 May 16, 2017
Contributor

You should just be able to look at IOS disasm to figure this out, which would give a better understanding anyways.

@@ -231,6 +231,7 @@ void WiiSocket::Update(bool read, bool write, bool except)
WiiSockMan::Convert(*wii_name, local_name);

int ret = connect(fd, (sockaddr*)&local_name, sizeof(local_name));
ret = WiiSockMan::GetInstance().AddSocket(ret);

This comment has been minimized.

@BhaaLseN

BhaaLseN May 15, 2017
Member

connect return 0 on success or -1 on error; it doesn't return a socket. Did you want to call this on fd instead of ret?

This comment has been minimized.

@sepalani

sepalani May 15, 2017
Author Contributor

Good catch, that's an oversight. It shouldn't be called at all.

return ret;
}

s32 WiiSockMan::GetHostSocket(s32 wii_fd)

This comment has been minimized.

@lioncash

lioncash May 15, 2017
Member

this can be a const member function using return WiiSockets.at(wii_fd).fd; in your if statement body.

@sepalani sepalani force-pushed the sepalani:fd-table branch from 87a8fa7 to 3c5a469 May 15, 2017
@sepalani sepalani force-pushed the sepalani:fd-table branch 2 times, most recently from 30d0cc1 to c6f8d5d May 24, 2017
@sepalani sepalani mentioned this pull request May 31, 2017
3 of 3 tasks complete
Copy link
Contributor

@riking riking left a comment

Error handling seems a bit suspect.

{
if (WiiSockets.count(wii_fd) > 0)
return WiiSockets.at(wii_fd).fd;
return EBADF;

This comment has been minimized.

@riking

riking Jun 1, 2017
Contributor

This should be return -EBADF;, no?

$ cat > test.c
#include <errno.h>
#include <stdio.h>
int main() { printf("%d\n", EBADF); }
$ gcc test.c ; ./a.out
9

This comment has been minimized.

@sepalani

sepalani Jun 1, 2017
Author Contributor

I borrowed this value from the DeleteSocket() member function, so I'm not sure. You might be right.

}

WiiSockMan::GetInstance().AddSocket(ReturnValue);
ret = WiiSockMan::GetInstance().AddSocket(ret);

This comment has been minimized.

@riking

riking Jun 1, 2017
Contributor

The return value should be checked for -1/error before being added to the socket table.

This comment has been minimized.

@sepalani

sepalani Jun 1, 2017
Author Contributor

It's checked in the AddSocket() method and is the behaviour Dolphin had before this PR.

@shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Jun 1, 2017

Here is the related SO code from IOS56 which is used to lookup the object via fd:

  v15 = irq_disable();
  entry = 0;
  if ( fd <= 23 )
  {
    entry = &sock_fd_table[fd];
    if ( entry->refcount > 0 && (obj = entry->obj) != 0 )
    {
      ++entry->refcount;
      if ( pobj )
        *pobj = obj;
    }
    else
    {
      entry = 0;
    }
  }
  irq_restore(v15);
  return entry;

So the hardcoded size check seems correct.

@shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Jun 1, 2017

Here is how the fd is selected by IOCTL_SO_SOCKET: https://gist.github.com/shuffle2/4541e4e96ba3e04a34bd36b0e07d3013#file-so_socket-c-L101
basically, SO keeps it's own fd table, and it returns the first currently free (refcount==0) index into the table as the fd.

@shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Jun 1, 2017

I updated the above gist to clear up confusion around the "protocol" arg for SOSocket()...the params follow standard posix socket() https://gist.github.com/shuffle2/4541e4e96ba3e04a34bd36b0e07d3013#file-so_socket-c-L71

@sepalani sepalani force-pushed the sepalani:fd-table branch from c6f8d5d to 71d9b64 Jun 4, 2017
@sepalani sepalani changed the title [WIP] IOS/Network: Emulate fd table IOS/Network: Emulate fd table Jun 4, 2017
@sepalani sepalani changed the title IOS/Network: Emulate fd table IOS/Network: Emulate socket fd table Jun 4, 2017
@sepalani sepalani force-pushed the sepalani:fd-table branch from 30f30b5 to 3b17d68 Jun 4, 2017
@sepalani
Copy link
Contributor Author

@sepalani sepalani commented Jun 4, 2017

@riking you were correct concerning the error values. It was a mistake from the original code because DeleteSocket returns the Wii error code via the CloseFd() member function. So I addressed your comment.

I tested this PR with games using NintendoWFC (with altwfc) and other channels and they don't hit the fd maximum limit. That is to say, they apparently don't forget to close sockets.

Otherwise, should I change the constants used with their respective define value such as AF_INET and so on or leave a comment about them?

Regardless, this PR is ready to be reviewed & merged.

@shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Jun 4, 2017

Otherwise, should I change the constants used with their respective define value such as AF_INET and so on or leave a comment about them?

What do you mean by that?

@sepalani
Copy link
Contributor Author

@sepalani sepalani commented Jun 4, 2017

@shuffle2
Replace this:

if (type != 1 && type != 2)

With this or something along those lines:

if (type != 1 && type != 2)  // SOCK_STREAM && SOCK_DGRAM

Ditto, for the other constants.

@shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Jun 4, 2017

A comment would be fine - I'm not sure how much we have to care about those values differing on host platforms, best to be on the safe side I guess.

@sepalani sepalani force-pushed the sepalani:fd-table branch from d7e64de to 1409690 Jun 4, 2017
@shuffle2 shuffle2 merged commit a2bd95a into dolphin-emu:master Jun 4, 2017
10 checks passed
10 checks passed
default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
@sepalani sepalani deleted the sepalani:fd-table branch Jun 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.