-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
piscsi_bridge is not removed when daynaport is detached #45
Comments
@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. |
@uweseimet This is what I see in the log after setting the log level to trace when detatching the daynaport:
So it looks like it is trying to run The |
@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++.
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 ... |
@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). |
@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. |
@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:
Maybe this helps. |
@fdanapfel Thank you, I will get back to you when working on this ticket. |
@fdanapfel This appears to work: After terminating s2p the bridge is now gone (branch issue_45). |
@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 PiSCSI/piscsi#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. |
@fdanapfel Currently the bridge is only removed when s2p is terminated. Can you please test this first? Anything else may come later. |
@fdanapfel I just moved the instructions from the website to the respective page on the wiki. |
@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:
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 Guess the Pi Zero 2 W is not a suitable system for compiling. |
@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. |
@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. 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. |
@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. |
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 ;-).
This disables the respective command line commands that you can see in the trace statements. |
@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:
|
@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. |
@fdanapfel Ensure that you run "make clean" first, in order to delete everything that might have been created by g++ before. |
@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. |
@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. |
@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):
So the clang error seems to have been caused by running If compiling now works I'll try to switch to the development branch and see if this works as well. |
@fdanapfel This sounds as if something in the previously checked out data was corrupted.
Check your other settings, like MAKEFLAGS for "g++". |
@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
Thanks for getting this fixed! |
@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 PiSCSI/piscsi#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. |
When creating a new ticket please provide information on your environment.
The Pi type:
Zero 2 W
The Pi OS version (output of 'lsb_release -a'):
Distributor ID: Debian
Description: Debian GNU/Linux 12 (bookworm)
Release: 12
Codename: bookworm
The SCSI2Pi release or git revision:
scsi2pi_1.2_devel_50d3589_arm64-1.deb
The computer/sampler connected to the RaSCSI/PiSCSI board:
N/A
Describe the issue
After attaching the daynaport the piscsi0 device and the piscsi bridge are automatically created:
But when detaching the daynaport only the piscsi0 device is removed, but not the piscsi_bridge:
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)
The text was updated successfully, but these errors were encountered: