Closed alexeys85 closed 2 years ago
Before you force-pushed the follow-up changes, the tests passed. Now they no longer pass, you'll have to look into why your PR introduces a regression.
Other than that it looks good in general. Some minor coding style issues, but those I can fix up myself later. Only caveat I have is that I'd like ipc_receive()
to return -1
instead of 0 on error, as we talked about earlier, i.e., change to ssize_t
return type and fix the behavior of the callee. Returning zero is fine on timeout when the client disconnects.
Before force-push PR wasn't quite complete and didn't support receiving arbitrary number of commands.
ipc_receive()
should fixed now!
Failed CI tests run fine locally, weird...
By the way, locally test gre.sh
was skipped:
[7m>> Checking dependencies ... [0m
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 22.04.1 LTS
Release: 22.04
Codename: jammy
Linux scuderia-pc 5.15.0-48-generic #54-Ubuntu SMP Fri Aug 26 13:26:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
[7m>> TEST: SKIP [0m
grep -q ip_gre /proc/modules is not supported on this system.
SKIP gre.sh (exit status: 77)
However, after modprobe ip_gre
:
enzo@scuderia-pc:~$ grep ip_gre /proc/modules
ip_gre 28672 0 - Live 0x0000000000000000
ip_tunnel 32768 1 ip_gre, Live 0x0000000000000000
gre 16384 1 ip_gre, Live 0x0000000000000000
enzo@scuderia-pc:~$ lsmod | grep gre
ip_gre 28672 0
ip_tunnel 32768 1 ip_gre
gre 16384 1 ip_gre
There's some memory corruption being introduced. See near the end of the last run:
>> Phase 3: Join group from multiple sources (IPC)
malloc(): unsorted double linked list corrupted
Re: GRE. Yeah, I've set up my tests using unshare
so it's possible to do networking ops unprivileged. The gre.sh
test however cannot load a module, that has to be done by the real root of the system, hence it's set to go to SKIP (not FAIL) if the kernel doesn't support IP-GRE.
In last force-push I got rid of malloc
usage but seems still no go, same as broken pipe
errors :(
Should be good now!
Looks much better, thank you for hanging in there! :smiley::+1:
I'll do a deep review of the changes probably sometime tomorrow morning (CET). But I don't expect anything major to pop up at this stage.
Merged this morning. Now, before the next release it'd be great to have support for
$ smcroutectl --batch - << EOF
...
EOF
And a tiny little test to verify the same.
Hop to get to it soon!
Cool thanks! I created a couple of issues to track it so we don't forget.
Add support to read multiple commands from client and invoke them. Example of possible (but not yet implemented) usage of smcroutectl:
smcroutectl --batch - << EOF join eth0 225.1.2.3 add eth0 192.168.1.42 225.1.2.3 eth1 eth2 rem eth1 225.3.4.5 eth3 leave eth1 225.3.4.5 EOF
Signed-off-by: Alexey Smirnov s.alexey@gmail.com