search

uweseimet / scsi2pi

Advanced performant SCSI/SASI emulation and tools for the PiSCSI/RaSCSI board
https://www.scsi2pi.net
BSD 3-Clause "New" or "Revised" License
9 stars 2 forks source link

piscsi_bridge is not removed when daynaport is detached #45

Closed fdanapfel closed 7 months ago

fdanapfel commented 7 months ago

When creating a new ticket please provide information on your environment.

Describe the issue

After attaching the daynaport the piscsi0 device and the piscsi bridge are automatically created:

$ ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host noprefixroute 
       valid_lft forever preferred_lft forever
2: wlan0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether e4:5f:01:78:10:a2 brd ff:ff:ff:ff:ff:ff
    inet 192.168.178.23/24 brd 192.168.178.255 scope global dynamic noprefixroute wlan0
       valid_lft 863902sec preferred_lft 863902sec
    inet6 2003:c9:d701:1200:470a:d09a:e629:147e/64 scope global dynamic noprefixroute 
       valid_lft 7189sec preferred_lft 3589sec
    inet6 fe80::8d9b:2326:42e9:e441/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever
$ /opt/scsi2pi/bin/s2pctl -i 6 -c a -t daynaport
$ /opt/scsi2pi/bin/s2pctl -l
+----+-----+------+-------------------------------------
| ID | LUN | TYPE | IMAGE FILE
+----+-----+------+-------------------------------------
|  6 |   0 | SCDP | DaynaPort SCSI/Link
+----+-----+------+-------------------------------------
$ ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host noprefixroute 
       valid_lft forever preferred_lft forever
2: wlan0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether e4:5f:01:78:10:a2 brd ff:ff:ff:ff:ff:ff
    inet 192.168.178.23/24 brd 192.168.178.255 scope global dynamic noprefixroute wlan0
       valid_lft 863892sec preferred_lft 863892sec
    inet6 2003:c9:d701:1200:470a:d09a:e629:147e/64 scope global dynamic noprefixroute 
       valid_lft 7179sec preferred_lft 3579sec
    inet6 fe80::8d9b:2326:42e9:e441/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever
7: piscsi0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master piscsi_bridge state UNKNOWN group default qlen 1000
    link/ether 42:56:e1:91:a2:7d brd ff:ff:ff:ff:ff:ff
    inet6 fe80::4056:e1ff:fe91:a27d/64 scope link 
       valid_lft forever preferred_lft forever
8: piscsi_bridge: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 42:56:e1:91:a2:7d brd ff:ff:ff:ff:ff:ff
    inet 10.10.20.1/24 brd 10.10.20.255 scope global piscsi_bridge
       valid_lft forever preferred_lft forever
    inet6 fe80::78c0:dfff:fe69:3170/64 scope link 
       valid_lft forever preferred_lft forever

But when detaching the daynaport only the piscsi0 device is removed, but not the piscsi_bridge:

frank@bookworm:~ $ /opt/scsi2pi/bin/s2pctl -i 6 -c d -t daynaport
frank@bookworm:~ $ /opt/scsi2pi/bin/s2pctl -l
No devices currently attached.
frank@bookworm:~ $ ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host noprefixroute 
       valid_lft forever preferred_lft forever
2: wlan0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether e4:5f:01:78:10:a2 brd ff:ff:ff:ff:ff:ff
    inet 192.168.178.23/24 brd 192.168.178.255 scope global dynamic noprefixroute wlan0
       valid_lft 863864sec preferred_lft 863864sec
    inet6 2003:c9:d701:1200:470a:d09a:e629:147e/64 scope global dynamic noprefixroute 
       valid_lft 7151sec preferred_lft 3551sec
    inet6 fe80::8d9b:2326:42e9:e441/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever
8: piscsi_bridge: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN group default qlen 1000
    link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
    inet 10.10.20.1/24 brd 10.10.20.255 scope global piscsi_bridge
       valid_lft forever preferred_lft forever
    inet6 fe80::78c0:dfff:fe69:3170/64 scope link 
       valid_lft forever preferred_lft forever

I would expect that the bridge should be removed as well.

BTW. as I've mentioned in on bookworm brctl is not needed to remove a bridge, it can also be done directly with ip: $ sudo /sbin/ip link delete piscsi_bridge (it is not necessary to "down" the piscsi_bridge first)

uweseimet commented 7 months ago

@fdanapfel Does the logfile show any messages regarding the removal? CTapDriver.cpp should log this.

Regarding bookworm, I don't intend to do something OS-specific in the C++ code, i.e I am not going to check whether s2p is running on bookworm or bullseye. I just added the C++ code because before I did, the setup required some manual work. The idea is to reduce the manual work required, as long as this can be done the same way for each OS. The less C++ code is needed for that, the better. The logfile on trace level reflects each step the C++ code takes, by logging the equivalent shell commands that once had to be executed.

fdanapfel commented 7 months ago

@uweseimet This is what I see in the log after setting the log level to trace when detatching the daynaport:

Jan 24 19:13:32 bookworm s2p[7335]: [2024-01-24 19:13:32.148] [trace] Executing DETACH command
Jan 24 19:13:32 bookworm s2p[7335]: [2024-01-24 19:13:32.148] [info] Validating: operation=DETACH, command parameters='locale=en_GB.UTF-8', device=6:0, type=SCDP, vendor='', product='', revision='', block size=0
Jan 24 19:13:32 bookworm s2p[7335]: [2024-01-24 19:13:32.148] [info] Executing: operation=DETACH, command parameters='locale=en_GB.UTF-8', device=6:0, type=SCDP, vendor='', product='', revision='', block size=0
Jan 24 19:13:32 bookworm s2p[7335]: [2024-01-24 19:13:32.148] [trace] >brctl delif piscsi_bridge piscsi0
Jan 24 19:13:32 bookworm s2p[7335]: [2024-01-24 19:13:32.173] [info] Detached SCDP 6:0
Jan 24 19:13:37 bookworm s2p[7335]: [2024-01-24 19:13:37.523] [trace] Executing DEVICES_INFO command

So it looks like it is trying to run brctl to remove the bridge, but brctl is not installed by default on bookworm.

The ip command I've mentioned in my previous command works on both bullseye and bookworm btw.

uweseimet commented 7 months ago

@fdanapfel If it works on both systems I'm fine with changing this. Please stay tuned ;-).

Note that s2p does not run brctl. s2p uses OS calls for this. The log just shows the command line equvalent, like it was before this was coded in C++. This is essentially what s2p does:

    ifreq ifr;
    ifr.ifr_ifindex = if_nametoindex("piscsi0");
    if (!ifr.ifr_ifindex) {
        return "Can't if_nametoindex " + ifname;
    }
    strncpy(ifr.ifr_name, "piscsi_bridge", IFNAMSIZ - 1);
    if (ioctl(br_socket_fd, SIOCBRDELIF, &ifr) < 0) {
        return ERROR;
    }
    return SUCCESS;

Whether brctl is installed or not does not matter., because ioctl is used. But ioctl does not report any error, so s2p assumes the operation has worked ...

fdanapfel commented 7 months ago

@uweseimet Ah, thanks for the explanation on how this actually works. I wasn't sure what you meant by CTapDriver.cpp (remember I'm not a code developer), and then got confused because I saw brctl mentioned in the log and also remembered that there was some discussion in the piscsi repo about brctl no longer being available on bookworm by default.

If it is possible to get adding and removing the bridge working by using code instead of relying on external commands then I'm fine with it. Using the ip command was just a suggestion since I thought s2p was still trying to use brctl (which made me even more confused because I could not figure out how the bridge is created if brctl isn't even installed).

uweseimet commented 7 months ago

@fdanapfel But the current code, which is the equivalent of the brctl command, does not seem to work, does it? The OS reports no error, but the brige has not been removed if I understand you correctly. The same code is used to create the brige, by the way. In this case you just pass TRUE instead of FALSE to the OS, using the same system call. (add bridge = true, remove bridge = false). I wonder why creating the bridge works, but deleting it does not appear to wirk.

fdanapfel commented 7 months ago

@uweseimet Did some reading up on this, and as far as I understand your code seems to be only for adding/removing an interface from the bridge.

Running an strace on brctl delbr piscsi_bridge shows the following ioctl being used to remove the bridge:

# strace brctl delbr piscsi_bridge
...
ioctl(3, SIOCBRDELBR, "piscsi_bridge")         = 0
...

Maybe this helps.

uweseimet commented 7 months ago

@fdanapfel Thank you, I will get back to you when working on this ticket.

uweseimet commented 7 months ago

@fdanapfel This appears to work: After terminating s2p the bridge is now gone (branch issue_45). It would be good if you could also test this, but in this case you should compile the issue_45 branch. Sooner or later compiling cannot be avoided in order to verify ticket resolutions ;-).

fdanapfel commented 7 months ago

@uweseimet Thanks for creating a fix for this. Is the bridge only removed when s2p is terminated. or also when the daynaport is detached?

I would prefer if the bridge is also removed when the daynaport is detached.

It would even be better if there was an option to select if the bridge should be created or not (to stay compatible with piscsi the default should be that the bridge is created) when the daynaport is attached. Because in a setup where the wlan interface is used the bridge is actually not needed as far as I know (since NAT is used instead of bridging in that case).

And having an option to not have the bridge created would make testing the new setup for connecting the emulated daynaport to the real network which I proposed in https://github.com/PiSCSI/piscsi/issues/1387 a bit easier.

Regarding compilimg the code: maybe you could add the instructions from https://www.scsi2pi.net/en/downloads.html on how to build from source to https://github.com/uweseimet/scsi2pi/wiki/Compilation-Instructions as well, or even on the main page of the github repo. Most people (including me) wouldn't expect them on a page titled "Downloads and Installation" on a completely different web-site when looking at a github repo.

uweseimet commented 7 months ago

@fdanapfel Currently the bridge is only removed when s2p is terminated. Can you please test this first? Anything else may come later.

uweseimet commented 7 months ago

@fdanapfel I just moved the instructions from the website to the respective page on the wiki.

fdanapfel commented 7 months ago

@uweseimet I've now tried compiling the code from branch issue_45 on the Pi Zero 2 W, but it looks like it is not working and causing the Pi to hang.

This is the output I get:

$ make
make -C cpp 
make[1]: Entering directory '/home/frank/source/scsi2pi/cpp'
g++ -O3 -Wall -DNDEBUG -std=c++20 -iquote . -D_FILE_OFFSET_BITS=64 -DFMT_HEADER_ONLY -DSPDLOG_FMT_EXTERNAL -MD -MP -Wall -Wextra -Wno-psabi  -DBOARD_FULLSPEC -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCHD -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCMO -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCCD -DBUILD_SCDP -DBUILD_SCLP -DBUILD_MODE_PAGE_DEVICE -DBUILD_SCHS -DBUILD_DISK -DBUILD_SAHD -c shared_command/command_dispatcher.cpp -o obj/command_dispatcher.o

After that noting more happens, even if I wait for half an hour.

I can still ping the Pi at that point, but ssh'ing to it to see what is going just hangs.

I also tried setting export MAKEFLAGS="-j1", which I found in one of the issues over on piscsi, but this doesn't seem to help either.

Guess the Pi Zero 2 W is not a suitable system for compiling.

uweseimet commented 7 months ago

@fdanapfel How much memory does you Pi have? Only 512 MB? In such as case g++ does not really work, it needs too much memory. You can use clang++ instead, which is also faster. After installing it (with apt install clang) you can run, for instance "make CXX=clang++", or add clang++ to the MAKEFLAGS. And set DEBUG=1, that's faster for compiling (but slower during runtime). Edit: 2W, so of course 512 MB. Then using -j1 and clang++ is the only option.

uweseimet commented 7 months ago

@fdanapfel Compiling on a Pi Zero (not Zero 2) takes several hours, by the way. Can you time how long it takes on the Pi 2, with "time make"? A Pi 4 only takes some minutes for a debug build. I am using a Pi 4 and a Pi Zero, but for the Pi Zero I always cross-compile . The nightly debug builds are built with cross-compilers.

If compiling takes too long I can trigger a cross-compiled build of the issue_45 branch. But I cannot do this every time. The cross compilers are running on a rather slow cloud VM, this is why I usually only use them for nightly builds.

uweseimet commented 7 months ago

@fdanapfel The bridge is also removed when detaching the DaynaPort. I missed that this is implicit, because the lifecyle of the bridge is identical with the lifecycle of the DaynaPort emulation. I just successfully tested this.

uweseimet commented 7 months ago

daynaport is attached. Because in a setup where the wlan interface is used the bridge is actually not needed as far as I know (since NAT is used instead of bridging in that case).

In such a case I would definitely need to know whether the bridge is needed or not, i.e. "as far as I know" is not sufficient I'm afraid ;-). Provided that you manage to compile the sources you can quite easily prevent the bridge from being created (for testing). Just comment out these lines in ctapdriver.cpp and run "make" again:

        trace(">ip link set dev " + BRIDGE_NAME + " up");
        if (const string error = ip_link(ip_fd, BRIDGE_NAME.c_str(), true); !error.empty()) {
            return cleanUp(error);
        }

...
    trace(">brctl addif " + BRIDGE_NAME + " " + DEFAULT_BRIDGE_IF);
    if (const string error = br_setif(br_socket_fd, BRIDGE_NAME, DEFAULT_BRIDGE_IF, true); !error.empty()) {
        return cleanUp(error);
    }

This disables the respective command line commands that you can see in the trace statements.

fdanapfel commented 7 months ago

@uweseimet Thanks for all the guidance on how to compile the code.

I've now installed clang, and running make with -j1, CXX=clang++ and DEBUG=1 doesn't seem to cause the Pi Zero to hang, but make now fails with an error:

$ make -j1 CXX=clang++ Debug=1
make -C cpp 
make[1]: Entering directory '/home/frank/source/scsi2pi/cpp'
mkdir -p lib
mkdir -p obj
mkdir -p generated
protoc -I ../proto --cpp_out=generated ../proto/s2p_interface.proto
mv generated/s2p_interface.pb.cc generated/s2p_interface.pb.cpp
clang++ -O3 -Wall -DNDEBUG -std=c++20 -iquote . -D_FILE_OFFSET_BITS=64 -DFMT_HEADER_ONLY -DSPDLOG_FMT_EXTERNAL -MD -MP -Wall -Wextra -Wno-psabi  -DBOARD_FULLSPEC -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCHD -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCMO -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCCD -DBUILD_SCDP -DBUILD_SCLP -DBUILD_MODE_PAGE_DEVICE -DBUILD_SCHS -DBUILD_DISK -DBUILD_SAHD -c shared_command/command_dispatcher.cpp -o obj/command_dispatcher.o
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.  Program arguments: clang++ -O3 -Wall -DNDEBUG -std=c++20 -iquote . -D_FILE_OFFSET_BITS=64 -DFMT_HEADER_ONLY -DSPDLOG_FMT_EXTERNAL -MD -MP -Wall -Wextra -Wno-psabi -DBOARD_FULLSPEC -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCHD -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCMO -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCCD -DBUILD_SCDP -DBUILD_SCLP -DBUILD_MODE_PAGE_DEVICE -DBUILD_SCHS -DBUILD_DISK -DBUILD_SAHD -c shared_command/command_dispatcher.cpp -o obj/command_dispatcher.o
1.  <eof> parser at end of file
2.  Code generation
3.  Running pass 'Function Pass Manager' on module 'shared_command/command_dispatcher.cpp'.
4.  Running pass 'AArch64 Assembly Printer' on function '@_ZN6spdlog6logger5cloneENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE'
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamEi+0x44)[0x7f7a3be4dc]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm3sys17RunSignalHandlersEv+0x70)[0x7f7a3bc4b4]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(+0xdcb1c0)[0x7f7a2eb1c0]
linux-vdso.so.1(__kernel_rt_sigreturn+0x0)[0x7f82c2f7bc]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(+0x132a9f0)[0x7f7a84a9f0]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm11raw_ostream5writeEPKcm+0x164)[0x7f7a39b49c]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm9MCContext16createTempSymbolERKNS_5TwineEb+0xb4)[0x7f7b751048]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm9MCContext16createTempSymbolEv+0x3c)[0x7f7b7513f0]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm16MCObjectStreamer18emitCFIEndProcImplERNS_16MCDwarfFrameInfoE+0x1c)[0x7f7b775c6c]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(+0x165f4c0)[0x7f7ab7f4c0]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm10AsmPrinter16emitFunctionBodyEv+0x264c)[0x7f7ab60b6c]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(+0x2764ba4)[0x7f7bc84ba4]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm19MachineFunctionPass13runOnFunctionERNS_8FunctionE+0x140)[0x7f7a71122c]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm13FPPassManager13runOnFunctionERNS_8FunctionE+0x26c)[0x7f7a4f4390]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm13FPPassManager11runOnModuleERNS_6ModuleE+0x3c)[0x7f7a4faf70]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm6legacy15PassManagerImpl3runERNS_6ModuleE+0x7b4)[0x7f7a4f4d98]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZN5clang17EmitBackendOutputERNS_17DiagnosticsEngineERKNS_19HeaderSearchOptionsERKNS_14CodeGenOptionsERKNS_13TargetOptionsERKNS_11LangOptionsEN4llvm9StringRefEPNSE_6ModuleENS_13BackendActionESt10unique_ptrINSE_17raw_pwrite_streamESt14default_deleteISK_EE+0xad0)[0x7f80ffc680]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(+0x1b06f20)[0x7f812b6f20]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZN5clang8ParseASTERNS_4SemaEbb+0x210)[0x7f801f6f1c]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZN5clang14FrontendAction7ExecuteEv+0x80)[0x7f81bb4444]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZN5clang16CompilerInstance13ExecuteActionERNS_14FrontendActionE+0x328)[0x7f81b27d28]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZN5clang25ExecuteCompilerInvocationEPNS_16CompilerInstanceE+0x218)[0x7f81c27d20]
clang++(_Z8cc1_mainN4llvm8ArrayRefIPKcEES2_Pv+0x810)[0x413608]
clang++[0x411df4]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(+0x207c5fc)[0x7f8182c5fc]
/lib/aarch64-linux-gnu/libLLVM-14.so.1(_ZN4llvm20CrashRecoveryContext9RunSafelyENS_12function_refIFvvEEE+0xcc)[0x7f7a2eae94]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZNK5clang6driver10CC1Command7ExecuteEN4llvm8ArrayRefINS2_8OptionalINS2_9StringRefEEEEEPNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPb+0x120)[0x7f8182c000]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZNK5clang6driver11Compilation14ExecuteCommandERKNS0_7CommandERPS3_+0x2b0)[0x7f81800f50]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZNK5clang6driver11Compilation11ExecuteJobsERKNS0_7JobListERN4llvm15SmallVectorImplISt4pairIiPKNS0_7CommandEEEE+0x7c)[0x7f8180118c]
/lib/aarch64-linux-gnu/libclang-cpp.so.14(_ZN5clang6driver6Driver18ExecuteCompilationERNS0_11CompilationERN4llvm15SmallVectorImplISt4pairIiPKNS0_7CommandEEEE+0xd8)[0x7f818150c4]
clang++(main+0x23d4)[0x411648]
/lib/aarch64-linux-gnu/libc.so.6(+0x27780)[0x7f79097780]
/lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0x98)[0x7f79097858]
clang++(_start+0x30)[0x40eef0]
clang: error: clang frontend command failed with exit code 139 (use -v to see invocation)
Debian clang version 14.0.6
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: /tmp/command_dispatcher-5a81c0.cpp
clang: note: diagnostic msg: /tmp/command_dispatcher-5a81c0.sh
clang: note: diagnostic msg: 

********************
make[1]: *** [Makefile:282: obj/command_dispatcher.o] Error 139
make[1]: Leaving directory '/home/frank/source/scsi2pi/cpp'
make: *** [Makefile:10: cpp] Error 2
uweseimet commented 7 months ago

@fdanapfel It must be "DEBUG", not "Debug". If this does not work then I don't know what's wrong. Compiling (PiSCSi and SCSi2Pi) definitely works fine on numerous platforms. And on a Pi anyway. The fact that the compiler crashes indicates that there is a basic issue with your environment, which is not related to the sources.

uweseimet commented 7 months ago

@fdanapfel Ensure that you run "make clean" first, in order to delete everything that might have been created by g++ before.

fdanapfel commented 7 months ago

@uweseimet Did "make clean" and then ran "make -j1 CXX=clang++" but it still crashes with the same error.

I'm using the standard Raspberry Pi OS 12 (bookworm) arm64 image, and the only modifications that I've done is to install the devel packages for scsi2p and add all the packages you have listed that are required for compiling scsi2p (including their dependencies) and then added clang (plus its dependencies) later on as you mentioned.

No modifications besides that have been done to the setup. So it is either an issue with the clang provided by bookworm, or the Pi Zero 2 W is simply not powerful enough for compiling stuff.

I don't have any more time until next week to look into getting the compilation to work. So unless you are able to provide a development version of the scsi2pi .deb that includes the changes I won't be able to do any testing to verify that your changes work for me as well at the moment.

uweseimet commented 7 months ago

@fdanapfel Because I could test this myself I will commit these changes later, so that tomorrow there will be a new develop build with them. But, of course, as soon as your tickets require changes for problems that only you can reproduce it would definitely be required that you have a working compiler environment. Not only for SCSI2Pi, but also for PiSCSI. On my Pi 4 I am also using the standard bookworm arm64 image, by the way.

fdanapfel commented 7 months ago

@uweseimet I've now tried to just start over from scratch by removing the cloned scsi2pi directory, and then clone it again and just run make without switching to the branch, and this now seems to work without the clang error (the compilation is still running after 15 mins, whereas before it almost immediately failed):

$ rm -rf scsi2pi/
$ git clone https://github.com/uweseimet/scsi2pi
Cloning into 'scsi2pi'...
remote: Enumerating objects: 1611, done.
remote: Counting objects: 100% (1374/1374), done.
remote: Compressing objects: 100% (778/778), done.
remote: Total 1611 (delta 969), reused 783 (delta 596), pack-reused 237
Receiving objects: 100% (1611/1611), 544.72 KiB | 1.73 MiB/s, done.
Resolving deltas: 100% (1072/1072), done.
$ cd scsi2pi/
$ export CXXFLAGS="clang++"
$ make -j1
make -C cpp 
make[1]: Entering directory '/home/frank/source/scsi2pi/cpp'
mkdir -p lib
mkdir -p obj
mkdir -p generated
protoc -I ../proto --cpp_out=generated ../proto/s2p_interface.proto
mv generated/s2p_interface.pb.cc generated/s2p_interface.pb.cpp
g++ clang++ -O3 -Wall -DNDEBUG -std=c++20 -iquote . -D_FILE_OFFSET_BITS=64 -DFMT_HEADER_ONLY -DSPDLOG_FMT_EXTERNAL -MD -MP -Wall -Wextra -Wno-psabi  -DBOARD_FULLSPEC -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCHD -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCMO -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCCD -DBUILD_SCDP -DBUILD_SCLP -DBUILD_MODE_PAGE_DEVICE -DBUILD_SCHS -DBUILD_DISK -DBUILD_SAHD -c shared_command/command_dispatcher.cpp -o obj/command_dispatcher.o

So the clang error seems to have been caused by running git checkout issue_45 before running make.

If compiling now works I'll try to switch to the development branch and see if this works as well.

uweseimet commented 7 months ago

@fdanapfel This sounds as if something in the previously checked out data was corrupted. I see that you are compiling an optimized version again (no DEBUG=1). Remember, this makes compiling even slower than it is anyway. But, of course, the perfomance of the result (transfer rate) is significantly better. I also see that something is still wrong "g++ clang++" is not valid, this must be "clang++" only, because the idea is to replace g++ by clang++. This apperas to be caused by a bug in the Makefile, I can reproduce it. As a work-around, just replace g++ by clang++ directly in the Makefile. Edit: This is not a bug in the Makefile, just checked it:

>make CXX=clang++
clang++ -O0 -g -DDEBUG -std=c++20 -iquote . -D_FILE_OFFSET_BITS=64 -DFMT_HEADER_ONLY -DSPDLOG_FMT_EXTERNAL -MD -MP -Wall -Wextra -Wno-psabi  -DBOARD_FULLSPEC -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCHD -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCMO -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCCD -DBUILD_SCDP -DBUILD_SCLP -DBUILD_MODE_PAGE_DEVICE -DBUILD_SCHS -DBUILD_DISK -DBUILD_SAHD -c shared_command/command_dispatcher.cpp -o obj/command_dispatcher.o

Check your other settings, like MAKEFLAGS for "g++".

fdanapfel commented 7 months ago

@uweseimet Thanks for the suggestion, but I gave up for now on figuring out why compiling won't work. I'll look into this again next week.

But I was now able to verify that after installing scsi2pi_2.0_devel_9881ef7_bookworm_arm64-1.deb the piscsi_bridge is now removed as well when the daynaport is detached:

$ ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host noprefixroute 
       valid_lft forever preferred_lft forever
2: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether e4:5f:01:78:10:a2 brd ff:ff:ff:ff:ff:ff
    inet 192.168.178.23/24 brd 192.168.178.255 scope global dynamic noprefixroute wlan0
       valid_lft 863943sec preferred_lft 863943sec
    inet6 2003:c9:d701:1200:470a:d09a:e629:147e/64 scope global dynamic noprefixroute 
       valid_lft 7146sec preferred_lft 3546sec
    inet6 fe80::8d9b:2326:42e9:e441/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever
$ /opt/scsi2pi/bin/s2pctl -l
No devices currently attached.
$ /opt/scsi2pi/bin/s2pctl -i 6 -c a -t daynaport
$ /opt/scsi2pi/bin/s2pctl -l
+----+-----+------+-------------------------------------
| ID | LUN | TYPE | IMAGE FILE
+----+-----+------+-------------------------------------
|  6 |   0 | SCDP | DaynaPort SCSI/Link
+----+-----+------+-------------------------------------
$ ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host noprefixroute 
       valid_lft forever preferred_lft forever
2: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether e4:5f:01:78:10:a2 brd ff:ff:ff:ff:ff:ff
    inet 192.168.178.23/24 brd 192.168.178.255 scope global dynamic noprefixroute wlan0
       valid_lft 863884sec preferred_lft 863884sec
    inet6 2003:c9:d701:1200:470a:d09a:e629:147e/64 scope global dynamic noprefixroute 
       valid_lft 7165sec preferred_lft 3565sec
    inet6 fe80::8d9b:2326:42e9:e441/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever
3: piscsi0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master piscsi_bridge state UNKNOWN group default qlen 1000
    link/ether 0a:a9:99:b7:0b:58 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::8a9:99ff:feb7:b58/64 scope link 
       valid_lft forever preferred_lft forever
4: piscsi_bridge: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 0a:a9:99:b7:0b:58 brd ff:ff:ff:ff:ff:ff
    inet 10.10.20.1/24 brd 10.10.20.255 scope global piscsi_bridge
       valid_lft forever preferred_lft forever
    inet6 fe80::4434:6eff:fe1a:711/64 scope link 
       valid_lft forever preferred_lft forever
$ /opt/scsi2pi/bin/s2pctl -i 6 -c d -t daynaport
$ ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host noprefixroute 
       valid_lft forever preferred_lft forever
2: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether e4:5f:01:78:10:a2 brd ff:ff:ff:ff:ff:ff
    inet 192.168.178.23/24 brd 192.168.178.255 scope global dynamic noprefixroute wlan0
       valid_lft 863943sec preferred_lft 863943sec
    inet6 2003:c9:d701:1200:470a:d09a:e629:147e/64 scope global dynamic noprefixroute 
       valid_lft 7146sec preferred_lft 3546sec
    inet6 fe80::8d9b:2326:42e9:e441/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever

Thanks for getting this fixed!

uweseimet commented 7 months ago

@fdanapfel Thank you for testing. I am going to close this ticket, because the issue in the subject has been resolved. For any further tickets on the network/bridge setup (unless there is a bug) I would like to wait until https://github.com/PiSCSI/piscsi/issues/1387 has been resolved, i.e. until there is a potentially new final setup for PiSCSI. If this setup requires changes or triggers improvements in SCSI2Pi these should be addressed in a new ticket. Note that the next development build will contain one more change for this ticket: IMO s2p should not remove the bridge if it has not also created it. If the bridge was present before s2p was lauched, this is an external setup and s2p should not interfere with it.