RFC: Revised patchset to fix Bug 7929 (C&C 3 network does not work)

Mike Kaplinskiy mike.kaplinskiy at gmail.com
Sat Sep 18 22:59:47 CDT 2010


A few broad comments:
 - There's enough #ifdef's to make this mostly unreadable. It's hard
to think of the cases of no IP_PKTINFO availability, only compile time
availability or full availability. Please just write a few wrapper
functions for the #ifdef's that return sensible values if IP_PKTINFO
is not available and then use them. I understand that in some places
(i.e. WS2_send/WS2_recv) are supposed to be #ifdef'y but it may be
easier to comment if you minimize the sheer number of combinations.
 - Pick a place to do the filtering and stick with it. You do some
filtering in the server, some filtering in wine-userspace and some
calls aren't filtered at all (see blocking recv's). I would suggest
forwarding ReadFile/WriteFile to WSARecv/WSASend for sockets and then
taking care of it in ws2_32 (we might end up needing this soon anyway
for other annoying reasons). Even if your async reads are woken, you
should be lenient enough in WS2_recv to return EWOULDBLOCK if no
packets matching the interface are available. Since you don't do this
now blocking recv's are broken. Also, FYI our *nix sockets are always
non-blocking, you don't need MSG_DONTWAIT.
 - So what if an application likes to bind to an interface AND use IP_PKTINFO?
 - I don't think this is the right approach. Alexandre's comments from
last year still apply. We would have to change the way we do sockets
entirely; instead of just passing through to similar functions in *nix
we would need to emulate a network stack (which is really what your
patches crudely do). For example, now we wouldn't be able to use poll
anywhere reliably on sockets. I don't think that's a direction
Alexandre wants to go in. I think only one of two things will help us
in this case - the kernel folks appease us or wineserver becomes a
kernel module and we just do I/O from wineserver. Both of which are on
schedule for around the time hell freezes over.
 - If you really want to get this to work consider writing something
to LD_PRELOAD that emulates most of the network syscalls (+ read) with
windows-style bind'ing correctly. I'd argue this may be easier and
faster than putting the fix it into wine. This may be more
user-friendly than having people compile wine themselves.

Mike.


On Sat, Sep 18, 2010 at 10:44 PM, Erich Hoover <ehoover at mines.edu> wrote:
> I realize it's been some time since I've submitted something for the
> networking problems with the C&C games, but I believe I've come up
> with something that resolves the major show-stopping issue that was
> discussed previously.  Here is the original thread, for reference:
>    http://www.winehq.org/pipermail/wine-devel/2009-October/079099.html
>
> Specifically, this set of patches is revised to address the problem of
> throwing away packets destined for the incorrect interface in a
> multi-threaded/multi-process environment.  The approach used here is
> to create a global (named) POSIX.1-2001 semaphore for each socket with
> the form
>    wine-ifacelock-FD
> , where "FD" is the Unix file descriptor corresponding to the socket
> at hand.  These semaphores are only created for UDP sockets bound to a
> specific interface and are locked/unlocked by both the Wine user-space
> and the Wine server.
>
> These patches are against wine-1.3.3 and are split apart according to
> function.  The most revised portions since the original set are 2/6
> (server-side creation of semaphore), 3/6 (locking and unlocking), and
> 6/6 (server-side locking and unlocking).  If I could get some feedback
> on these patches then I would greatly appreciate it, it'd be really
> nice to finally get this issue resolved.  Thanks in advance!
>
> [1/6] ws2_32/tests: Add UDP broadcast tests:
>        http://www.compholio.com/wine-kane/patches/2010-09-18/0001-ws2_32-tests-Add-UDP-broadcast-tests.patch
> [2/6] server: Add mechanism for storing an interface ID with a socket:
>        http://www.compholio.com/wine-kane/patches/2010-09-18/0002-server-Add-mechanism-for-storing-an-interface-ID-wit.patch
> [3/6] ws2_32: Patch to selectively bind to interfaces while still
> allowing broadcast packets:
>        http://www.compholio.com/wine-kane/patches/2010-09-18/0003-ws2_32-Patch-to-selectively-bind-to-interfaces-while.patch
> [4/6] ws2_32: Ensure select does not wake up on packets with an
> interface mismatch:
>        http://www.compholio.com/wine-kane/patches/2010-09-18/0004-ws2_32-Ensure-select-does-not-wake-up-on-packets-wit.patch
> [5/6] ws2_32: Ensure Async WSARecv does not wake up on packets with an
> interface mismatch:
>        http://www.compholio.com/wine-kane/patches/2010-09-18/0005-ws2_32-Ensure-Async-WSARecv-does-not-wake-up-on-pack.patch
> [6/6] server: Ensure Async ReadFile does not wake up on packets with
> an interface mismatch:
>        http://www.compholio.com/wine-kane/patches/2010-09-18/0006-server-Ensure-Async-ReadFile-does-not-wake-up-on-pac.patch
>
> Erich Hoover
> ehoover at mines.edu
>
>
>



More information about the wine-devel mailing list