SCP: Update NAT support to libslirp 4.8.0 #437
Conversation
Add ASan [address], MSan [memory], TSan [thread] and UBSan [undefined behavior] to the cmake-builder scripts. These are intended for developer usage only to aid in debugging and general code improvement/diagnosis. Linux, macOS: ASan, MSan, TSan and UBSan. Microsoft: ASan only.
Make Ethernet debugging (DBG_ETH, DEBUG_ETHER) conditional on the simulator's debugging bits. User must explicitly enable ETH debugging, whereas the default polluted the debug output unconditionaly.
Rip-n-replace the older slirp implementation with libslirp 4.8.0, which
includes a minimal glib stub implementation. No SIMH-specific
modifications made to libslirp so that future updates are easier to
apply.
Other changes:
- ETHER: Move UNIT service activation earlier
Move the UNIT service activation to after the packet is placed in the
Ethernet interface's read queue, vs where the "wakeup" was in the
reader thread's loop. If AIO ism't pending for the UNIT, the UNIT gets
scheduled via sim_activate_abs(). Uses sim_unit_aio_pending()
Solves a mystery stall when trying to FTP NetBSD's pkgsrc archive.
- SCP: sim_unit_aio_pending()
Add a predicate function that returns TRUE if the UNIT has pending
asynchronous I/O work to be done. Used in _eth_callback (sim_ether.c)
to schedule the UNIT's service function if necessary (no need to
schedule the service function if it's already going to be called.)
Does not depend on being in the main SIM thread.
- SCP: sim_misc_debug()
sim_misc_debug() enables debug output from non-devices, e.g., libslirp's
debug output, for consistent output formatting.
- CONSOLE: On Win32, don't repeatedly call GetConsoleMode() each time
sim_console_write() is called. Use an output function pointer to
invoke WriteConsoleA (console output) or WriteFile (output is not
a console).
- CMake
- Clang updates for Windows (CLANG64 environment from MinGW64).
- MinGW64 environment: Place binaries under BIN/$MSYSTEM, keep
BIN/Win32/_config_ reserved for MS-compiled simulators.
- Fix printf() formats for MinGW64-related compiles.
- makefile
- SDL2_ttf: Only generate info messages iff BESM6 is going to be
compiled. Do no generate SDL TTF or missing font file warnings when
compiling pdp11 or any other simulator.
- SYS_LDFLAGS: System (non-feature) libraries that should be appended to
the compiler's command line, such as -lm, -lpthread, -lrt and -ldl.
- Detect features required by the libslirp.a library by compiling a
small configuration program. (NOTE: This technique can be expanded
to detect other simulator-required or SIMH-required features.)
- Use a library, BIN/libslirp.dir/libslirp.a, to provide libslirp
support rather than compiling libslirp/src/*.c for each simulator that
uses networking support.
- Per-simulator "_LDFLAGS" so that only only the libraries needed are
provided to the compiler. Only added to simulators that use video and
networking. No real point in dragging SDL2_ttf into simulators other
than BESM6, for example.
|
Yes, this is a BF merge request, where most of the bulk comes from adding the Tested with the
Did not touch or alter the Removes the restrictions that prevented Win64 support. CMake produces |
|
"It's a lovely day to be a Turist." :-) |
|
You've got a massive set of changes in a single commit here. Sure, libslirp is a big change, by itself, but most of the other changes belong in separate commits! |
|
What are the advantages of using libslirp vs the original slirp code in simh? |
|
You said:
Absolutely true, but you broke the builds for simulators with network support.
That may indeed be true, but what is the advantage of Win64 support vs Win32 support for simh simulators? |
|
@markpizz: All checks passed on Github CI/CD (makefile and CMake). I'm sorry if that makes it more difficult for you to reintegrate this MR into your source tree. The Visual Studio Projects directory is obsolete and has been for a while. CMake is happy to produce a VS solution file that VS is quite happy to use for a variety of VS versions. World+Dog(tm) has since moved on to VS code. Re Win64: Imagine a project environment with strict hardware and software configuration management, and a distinct preference for Win64 executables instead of Win32. Well, more than a preference. A hard requirement. There are project environments like that, which use SIMH. SIMH actually does have an interesting coterie of users outside of the GitHub project space. |
|
So, this is merely another change for the sake of change without any actual benefits. Out of the other various, unrelated to slirp, changes in this single commit, maybe some are actual fixes or improvements to simh functionality, but again these belong, at least, in separate commits and more likely separate PRs. |
An assertion without a basis in reality, or in Navy-speak, "BRAVO SIERRA". The SLiRP project refactored the code base to be more useful than just to the QEMU project. QEMU converted over and now uses the library version, There are two benefits: (a) libslirp's interface is simpler than the previous SLiRP, (b) libslirp is consistently compatible across platforms. SIMH still needed to provide glib stubs to work around creating a dependency on glib-2.0. However, those stubs do not prevent compiling for Win64 and do not include SYSTEM HEADER FILE NAMES THAT PREVENT UPGRADES AND FORWARD PROGRESS. I'd personally love to congratulate the rocket scientist (was that you, Mark?) who decided that including header files with the same name as system header files was a superb idea. Technically, the simplified Additionally, in the 7 elapsed years since SLiRP was introduced, there have been bug fixes. Bug fixes are useful. |
|
Sorry for the delay responding. Patch looks ok to me. I would only ask that you verify that AIO works on PDP10 simulators. I had a problem with it and had to remove it. Right now I do not have much time for development, so will not have time to test this. |
I've been having fun being a Turist on the PDP10-KL and PDP10-KA and haven't encountered an issue with AIO (I owe you a MR for the ITS repo to fix the netmask in config). If you have suggestions for a test case that's network-heavy, I'm happy to do some debugging (idea: The 87M tar.gz file from http://cdn.netbsd.org/pub/pkgsrc/stable/pkgsrc.tar.gz). But, so far, so good with Putty SUPDUP and tools/supdup/supdup. |
|
What I do to test things is run my OS build scripts. Also try and operate Panda and ITS with network support. In this case I would make sure you test Slirp NATing. For ITS under Slirp use address outside the range of 10.0.0.0, ITS seems to have problems routing to these addresses. If I get some time tomorrow I will see what state my local repo is in. If it is good, you can submit the changes to my repository. Just be careful with scp.c as I have moved the prompt printing for vm_read into the vm_read function. I need to push these changes out to open-simh repository. |
That's primarily how I've tested NAT with the pdp10-kl simulator; I had to hack the Other than a nit with PuTTY's supdup not emitting CLREOL when it output a newline, I haven't run into any stalls or network hangs. (Note: I included some configuration info for ITS in the 0readme_ethernet.txt file for successful Turism. :-)
There should be no conflict between your changes and mine. |
bscottm
force-pushed
the
libslirp-rel-1
branch
from
8625fb4 to
70120dd
Compare
- Manage poll() file descriptors more efficiently. Both Linux() poll and Windows WSAPoll() ignore file descriptors that are negative in the pollfd array. Set the pollfd.fd file descriptor to INVALID_SOCKET when no longer needed. Reuse the pollfd array element if have_valid_socket() returns false. - sim_slirp_dispatch(): Removed, empty function no longer required. - sim_deb: Remove code that NULLs this FILE pointer when "flushing" the debug output. Presumably, closing the file and re-opening it flushes the debug output. Synchronizing when this operation occurs with other code in other threads also write to sim_deb needs to be revisited. For the time being, just fflush(sim_deb). Otherwise, _sim_debug_write_all() gets stuck in an infinite loop writing zero bytes.
Avoid spinning in the reader thread executing sim_slirp_select() when libslirp isn't managing any sockets. Waits on a condition variable until libslirp registers a socket via register_poll_socket(), at which point, the condition variable is signaled. - sim_atomic.h: Add atomic value primitives to ensure value synchronization across threads. Required for ARM-based platforms, useful on Intel. At the moment, these primitives are used by NAT(libslirp). However, there are many other places in SIMH where they would be useful if used. Does not (yet) replace the AIO intrinsics or synchronization code in sim_defs.h. - sim_slirp: Add no_sockets_cv condvar, no_sockets_lock mutex. If libslirp hasn't registered any sockets (which includes the TCP and UDP redirections), sim_slirp_select() waits on the condvar until awakened when register_poll_socket() encounters its first socket. Simulators that don't use AIO, but end up calling sim_slirp_select(): Sleep for 5 ms and return 0. It's a punt. - sim_ether: Close and cleanup the underlying Ethernet device before cleaning up the other reader thread synchronization primitives. Otherwise, the NAT(libslirp) reader thread will not exit.
bscottm
force-pushed
the
libslirp-rel-1
branch
2 times, most recently
from
76c13ba to
e66f393
Compare
|
@markpizz, @pkoning2, @rcornwell: For what it's worth, the XP executables top out at ~500kB/sec (select()-based polling) whereas the Win32 and Win64 executables top out at 1.8MB/sec (poll() and WSAPoll()-based polling) when downloading http://cdn.netbsd.org/pub/pkgsrc/stable/pkgsrc.tar.gz on the Vax emulator. ARM64 Ubuntu tops out around 1.9-2.0 MB/sec; Linux and macOS both use poll(). Also, it's painfully obvious that the XP executables were never used or downloaded. They could never execute on an XP system because a TARGET_WINVER (target Windows version) was not available to be set low enough. I'm starting to think that continuing to support XP is a "juice not worth the squeeze" exercise in futility. (No one has ever issued a bug report that they couldn't use the prebuilt XP binaries.) |
What mechanism measured the numbers you are quoting here and how/why isn't it part of the CI testing? Also, I once again mention the inconsistent line ending issue in your inclusion of libslirp which is more inconsistent than it simply has LF line endings. Specifically, some of the libslirp files have LF line endings and others have CRLF line endings. In addition to that, you've made changes to several libslirp files. Have you submitted these changes to the upstream source of libslirp? |
It's a well-established in the Linux and BSD communities that Informally measured at home using NetBSD 10.0 on the VAX/uVax 3900 simulator,
The With respect to line endings, I'm happy to convert the |
ETHER: - Remove the spaghetti code that was _eth_reader, replace with a simpler state machine loop that invokes a network-specific reader function. - Implement an orderly shutdown of the underlying network emulation when needed, partcularly for libslirp. Leverages atomic primitives to ensure proper cross-thread communication, which is important on non-x86 platforms. CONSOLE: - Add the "DBGSIGNAL" (also "DBGSIG") option to send the interrupt to the debugger (presumably, gdb.) This permits debugging optimized release code, i.e., simulator was compiled with "-O2 -g" flags. By default, DBGSIGNAL is off. - DBGSIGNAL is separate from the interrupt character, DBGINT. The two work together. FIO: - Work around spurious GCC Link Time Optimization messages when it tries to inline sim_byte_swap_data(). LTO tries to inline sim_byte_swap_data and comes the conclusion that 8 bytes are being stored in a 4 byte area (sim_tape.c, fread()-ing meta, type t_mtrlnt.) As the result of these LTO warnings, compiling with the makefile becomes a losing proposition because all warnings are errors. - Use byte swapping intrinsics on GCC, Clang and MSVC, dropping through to the old for loop if not a known or an irregular size. CMake: - Add option to build libslirp with glib-2.0 vs. the default "minimal" glib replacement.
|
@mark: Following transcript using my current code (which I need to repackage for this release), on Win64 using
|
bscottm
force-pushed
the
libslirp-rel-1
branch
from
e66f393 to
f79cf17
Compare
|
Temporarily withdrawing this PR -- found multiple NPCAP issues exposed by the code movement in this patch. Cannot (and should not) release code with known problems (although this really tells you how much
Needs testing now that I've found the problem and adapted an easily found solution on StackOverflow. |
Rip-n-replace the older slirp implementation with libslirp 4.8.0 and a minimal glib stub implementation. No SIMH-specific modifications were made to libslirp so that future updates are easier to apply.
Other changes:
ETHER: Move UNIT service activation earlier
Move the UNIT service activation to after the packet is placed in the
Ethernet interface's read queue, vs where the "wakeup" was in the
reader thread's loop. If AIO ism't pending for the UNIT, the UNIT gets
scheduled via sim_activate_abs(). Uses sim_unit_aio_pending()
Solves a mystery stall when trying to FTP NetBSD's pkgsrc archive.
SCP: sim_unit_aio_pending()
Add a predicate function that returns TRUE if the UNIT has pending
asynchronous I/O work to be done. Used in _eth_callback (sim_ether.c)
to schedule the UNIT's service function if necessary (no need to
schedule the service function if it's already going to be called.)
Does not depend on being in the main SIM thread.
SCP: sim_misc_debug()
sim_misc_debug() enables debug output from non-devices, e.g., libslirp's
debug output, for consistent output formatting.
This change includes a separate debugging mutex, which removes the
dependency (dual use) on the AIO mutex.
CONSOLE: Don't repeatedly call GetConsoleMode() on Win32 each time sim_console_write() is called. Use an output function pointer to invoke WriteConsoleA (console output) or WriteFile (output is not a console).
CMake
Clang updates for Windows (CLANG64 environment from MinGW64).
MinGW64 environment: Place binaries under BIN/$MSYSTEM, keep BIN/Win32/config reserved for MS-compiled simulators.
Fix printf() formats for MinGW64-related compiles.
makefile
SDL2_ttf: Only generate info messages iff BESM6 is going to be compiled. Do no generate SDL TTF or missing font file warnings when compiling pdp11 or any other simulator.
SYS_LDFLAGS: System (non-feature) libraries that should be appended to the compiler's command line, such as -lm, -lpthread, -lrt and -ldl.
Detect features required by the libslirp.a library by compiling a small configuration program. (NOTE: This technique can be expanded to detect other simulator-required or SIMH-required features.)
Use a library, BIN/libslirp.dir/libslirp.a, to provide libslirp support rather than compiling libslirp/src/*.c for each simulator that uses networking support.
Per-simulator "_LDFLAGS" so that only only the libraries needed are provided to the compiler. Only added to simulators that use video and networking. No real point in dragging SDL2_ttf into simulators other than BESM6, for example.