sceNetInet socket remap

[hrydgard]

As adviced by @anr2me in anr2me#21

Works great on Windows! Managed to play races in Wipeout Pulse.

But breaks other platforms currently, heh. EDIT: Was fixed eventually.

[anr2me]

My SceNetInetSelect implementation was mostly a work around for Windows and only works for 1 socket (the last socket)
Also, Windows' fd_set struct only supports up to 64 sockets as i remembered, because it use array of socket's fd instead of bitmap of socket's id where each bit represent 1 socket id like what PSP (and non-Windows use),
So we probably need to make sure the fd_set iteration doesn't exceed 64 sockets on Windows to avoid unexpected behavior (most games use less than 64 sockets anyway), or may be set the FD_SETSIZE to 256 by defining the preprocessor symbol FD_SETSIZE before including winsock.h (not sure whether this will affects other parts of the code that use fd_set too or not)

[hrydgard]

I also don't think more than 64 sockets is necessary to support, and since as you say it's an array of sockets and not a bitset, so even if a game passes in a number of sockets to listen to that's beyond 64, it's unlikely that that many are in use. The code in this PR makes use of that fact, and in fact it now works great on Windows!

Still debugging a mysterious failure on non-Windows though!

[hrydgard]

OK, mysterious failure fixed, I just start allocating socket numbers from 20 instead of from 1.

This must be clashing with some socket created by that adhoc thread that we start up? Will need to hook all socket access up to this socket allocator. But, let's commit in the meantime.

EDIT: Only worked on Mac, heh.

[hrydgard]

Rebased, still working on Windows/Mac.

Next step I think I'll have all the AdHoc stuff also use the socket mapper, so there's no risk of clashing.

[anr2me]

OK, mysterious failure fixed, I just start allocating socket numbers from 20 instead of from 1.

This must be clashing with some socket created by that adhoc thread that we start up? Will need to hook all socket access up to this socket allocator. But, let's commit in the meantime.

EDIT: Only worked on Mac, heh.

adhoc threads doesn't creates PSP socket unless the game is using AdhocMatching/GameMode(another adhoc library), the socket being created on friendfinder thread is only for communication with adhoc server, which use native TCP socket, thus doesn't consumes PSP's socket id.

[anr2me]

Rebased, still working on Windows/Mac.

Next step I think I'll have all the AdHoc stuff also use the socket mapper, so there's no risk of clashing.

Currently we use a separated id generation for both adhoc sockets and infra sockets, where adhoc sockets are stored in AdhocSocket* adhocSockets[MAX_SOCKET]; and the socket's id is the index number + 1;

As i remembered during my test on JPCSP with official prx files, adhoc sockets and infrastructure sockets seems to share the same id manager/generator.

For example:

1. create PDP socket, got id = 1
2. create UDP socket got id = 2
3. create PTP socket got id = 3
4. create TCP socket got id = 4

But this probably need to be confirmed on a real PSP too.

[anr2me]

We should store non-blocking state here too, and probably some other value that can be set using SetSockOpt too just like non-blocking state, but non-blocking state alone should be sufficient (at least for TCP/UDP socket, while PDP/PTP sockets might need some additional info to be stored, you can checked the AdhocSocket struct for this), since we will need to retrieve the non-blocking state in order to simulate blocking mode later.

PS: Sockets are in blocking mode by default after creation, thus the default value should be nonblocking = false.

[anr2me]

Also, since we have stored the socket protocol here, we can replace these code at sceNetInet.cpp by forwarding only TCP or UDP instead of forwarding both protocol:

// Enable Port-forwarding
	// TODO: Check the socket type/protocol for SOCK_STREAM/SOCK_DGRAM or IPPROTO_TCP/IPPROTO_UDP instead of forwarding both protocol
	// InetSocket* sock = pspSockets.Get<InetSocket>(socket, error);
	// UPnP_Add((sock->type == SOCK_STREAM)? IP_PROTOCOL_TCP: IP_PROTOCOL_UDP, port, port);	
	unsigned short port = ntohs(saddr.in.sin_port);
	UPnP_Add(IP_PROTOCOL_UDP, port, port);
	UPnP_Add(IP_PROTOCOL_TCP, port, port);

I opened both TCP and UDP because i use the native socket directly and there is noway to retrieve the socket protocol (at least the easy way as i remembered).

Edit: oops, you already did this >.<

[hrydgard]

Yeah I've concluded that the problem must be something different and the offset is fixing it by accident somehow :/ since indeed while joining infrastructure mode, we don't generate any other game-visible socket IDs.

I'm mystified.. but I'll crack this somehow heh.

[anr2me]

Yeah I've concluded that the problem must be something different and the offset is fixing it by accident somehow :/ since indeed while joining infrastructure mode, we don't generate any other game-visible socket IDs.

I'm mystified.. but I'll crack this somehow heh.

May be it didn't get initialized properly, and somehow works on those platforms because it was coincidentally zeroed.

[anr2me]

Shouldn't we returned -1 in the case of failure to create a socket? just like a posix socket syscall:

On success, a file descriptor for the new socket is returned. On
error, -1 is returned, and errno is set to indicate the error.

[hrydgard]

The caller is responsible for the actual syscall return value. But yeah this will be made clearer in an upcoming commit.

(either way, haven't hit this yet)

[hrydgard]

Added some ImDebugger UI where you can see some info about the open sockets and Np.

Still haven't figured out the linux/mac problem, or why increasing the minimum helps somewhat.

[anr2me]

May i know why we need the minimum value? as i remembered the id is always started from 1.

[hrydgard]

I don't know! It shouldn't be there, but it somehow fixes it on Mac and also on Linux sometimes...

I want to remove it (or set it to 1) before I merge this.

[anr2me]

Hmm.. may be during socket creation instead of searching for an unused slot below the last id, we should use the slot after the last id, and if the socket with the last id got deleted we decrease the last id, similar to what we did on adhoc socket.

For example:

1. create socket1, got id =1
2. create socket2 got id = 2
3. delete socket1
4. create socket3 got id = 3
5. delete socket3
6. create socket4 got id = 3
7. delete socket4
8. delete socket3
9. create socket5, got id = 2

[hrydgard]

If that was the cause, I'd expect it to fail on all platforms equally? However, it might still be worth trying, thanks for the idea.

[anr2me]

Btw, which game are you testing this with? the one that have issue on Linux/Android

[hrydgard]

Wipeout Pulse. Notably, without this PR, it works on Mac/Linux (but not on Windows, of course). Haven't really tested on Android yet, but not much point until it works on Linux...

[anr2me]

Well without id remapping, socket's ids on non-windows are usually below 256 and already compatible with PSP socket's id limit, unlike Windows socket id that usually have value more than 1k.

If the game use select the issue could also be on the select implementation, since Windows and non-Windows have a different fd_set structure, may be some ids didn't get set/isset correctly.

[hrydgard]

I do want to perform the same ID remapping on all platforms, and there's no reason that shouldn't work.

select is definitely the main suspect... But I can't understand why it won't work :/

[hrydgard]

I tried your ID assignment algorithm, made no difference.

What I'm seeing makes no sense at all :/

On Windows everything is fine with MIN_VALID_INET_SOCKET=1.

On Linux/Mac you have to increase MIN_VALID_NET_SOCKET, on Linux by at least 50, otherwise select never returns any pending reads bits. Then it works for me (but not for a tester). I don't see anything else interesting in the log.

I don't think I'm mixing up the IDs in sceNetInetSelect()..

[anr2me]

Hmm.. on Windows i can only get to this endless searching

[hrydgard]

Really? with this PR it's fully playable in multiplayer for me on Windows. EU version. Primary DNS in settings = 67.222.156.250.

[ChaCheeChoo]

You have the NTSC version. The server only works on PAL version.

[anr2me]

Hmm.. may be the issue is at

ppsspp/Core/HLE/sceNetInet.cpp

Line 678 in f84ec05

int retval = accept(inetSock->sock, (struct sockaddr*)&saddr.addr, srclen);

accept supposed to create a new socket (the input socket is the listening socket, which is different than the accepted socket of new connection), so the returned id should be mapped to PSP socket id by creating a new PSP socket.

On success, these system calls return a file descriptor for the
accepted socket (a nonnegative integer). On error, -1 is
returned, errno is set to indicate the error, and addrlen is left
unchanged.

[hrydgard]

Oh, that's a good catch! I'll fix that.

But it turns out that the problem was the first parameter to select, which I forgot to change according to the host socket numbers .. sigh. Anyway, it's solved now, working great on all three platforms (well, I'll test Mac now, but Linux works)

[anr2me]

You have the NTSC version. The server only works on PAL version.

Btw, is there a way to make the game cross-region? they probably use the same packet for communication, right?

[hrydgard]

I'll fix accept separately, along with a few other little things.

[ChaCheeChoo]

Making the game cross-region is on the to do list. Currently the server isn't set up to be able to have two different AppId's communicate with each other.