[TeleTubby666]

…net_write

On the Wii, connecting to an SMB share hangs after the tree connect, and opening a directory with many entries fails (the share root works, a large subfolder does not). A packet capture shows the server side is fine.

• smb2_net_read(): libsmb2 reads in a loop until the socket reports EAGAIN, but net_read() does not return EAGAIN when no data is waiting, so smb2_service() never returns. Only read when select() reports data, otherwise return EAGAIN.
• smb2_net_read()/smb2_net_write(): libogc's net_read()/net_write() copy the whole buffer through a fixed 64 KB IOS heap. A large reply (a directory listing is up to ~64 KB) fails with IPC_ENOMEM and breaks the connection. Cap a single call to 8 KB; libsmb2 resumes partial reads and writes.

Only affects wii

[dborth]

I think this is fixing this at the wrong layer. libsmb2 shouldn't need to work around this. I've submitted a PR for libogc2 - extremscorner/libogc2#31

[TeleTubby666]

Following up on this: I tested libogc2 PR 31 (block splitting) together with
an unmodified libsmb2 on a real Wii, with trace logging around net_read().
The large-reply case is addressed there, but the connect hang is not.

After the last PDU of the connect handshake has been read successfully
(net_read(len=16) = 16), the next read libsmb2 does (net_read(len=4)) blocks
instead of returning EAGAIN, so smb2_service() never returns. In my log
that call did not return for about a minute. Excerpt:

net_read(len=16) calling
net_read(len=16) = 16
net_read(len=4) calling
(no result)

That part is independent of the 64 KB heap limit, so it will remain even if
the libogc2 change is merged. The check in this PR (net_select() with a zero
timeout before net_read(), return EAGAIN if nothing is readable) fixes it
on my hardware. If you prefer to fix the 64 KB problem only in libogc2, this
part could still be applied here on its own; I can split it out into a
separate PR if that helps.

[dborth]

Got it. So with the large reply case handled, yes let's split off this to the remainder.

[dborth]

Let me update - any libsmb2 changes need to be tested on and also work for libogc too - not just libogc2.

[dborth]

The select-before-read check is a reasonable Wii-only workaround. But as written, a hangup or error that IOS reports without POLLIN becomes EAGAIN forever and the dead connection never surfaces. Watching the exception set as well fixes that. It costs one extra IOS call per read. I've added that change.

Before merging though (and submitting upstream to libsmb2) - to find the real cause, log what net_ioctl(fd, FIONBIO, &one) returns, what net_fcntl(fd, F_GETFL, 0) returns afterwards, and how long a net_read takes on an idle connected socket. If it blocks with the flag set, the fix belongs in libogc2 and this libsmb2 workaround can go.

[TeleTubby666]

Thanks for the pointers. I tested on real Wii hardware (libogc2 master incl. 8cad2e7 + libsmb2 with the change below, stock snes9xgx otherwise).

Root cause of the hang: with the toolchain I use, FIONBIO evaluates to 0x1 in libsmb2, but libogc2's net_ioctl() only accepts its own _IOW('f', 126, unsigned long) = 0x8004667e. So net_ioctl(fd, FIONBIO, &one) in set_nonblocking() returns -22 (EINVAL). The return value is ignored, the socket stays blocking, and net_read() blocks as soon as nothing is pending.

Trace (before): F_GETFL = 0x0, IOS_O_NONBLOCK (0x04) not set, read blocks -> hang at "connecting".
Trace (with explicit 0x8004667e): ioctl returns 0, F_GETFL = 0x4, net_read() returns -1/EAGAIN when drained, as libsmb2 expects.

With that change connect works, a large directory listing works (a 65424-byte read comes back as 32696 + 32728 thanks to 8cad2e7), loading a ROM works, and so do mkdir, stat and writes (8 KB, 32 KB and 48 KB writes all return the full length). The select-before-read workaround is not needed on this setup.

Change: TeleTubby666@b0ca245 (branch wii-fionbio-fix, based on current master).

Caveats: Wii only (Wii U is not affected). I have no GameCube to test. For libogc I could only check that it compiles in CI; I haven't verified what FIONBIO expands to there, so you may want to guard the constant accordingly.

Should I turn PR #2 into this change, or open a new PR from wii-fionbio-fix?

[dborth]

good job. it's the right problem, wrong solution. I've refined it for you and submitted it directly to sahlberg#495

you can test https://github.com/dborth/libsmb2 again and see if it's fixed. I've updated the changelog on the emulators which should kick off builds that include the libogc2 + libsmb2 changes