Hello, `kill_tcp_connections` in util script `functions` misuses the binary `ctdb_killtcp`: 1/ It passes connections to `ctdb_killatcp`'s stdin instead of as argument 2/ `ss` may shows connections under IPv6 format with [] ([::ffff:172.16.0.27]:794) and `ctdb_killtcp` does not accept it. We must remove the `[]` Here's the diff ``` @@ -430,8 +430,8 @@ kill_tcp_connections () get_tcp_connections_for_ip "$_ip" | { _killcount=0 _connections="" - _nl=" -" + _nl="" + _kill_res=0 while read _dst _src; do _destport="${_dst##*:}" __oneway=$_oneway @@ -444,19 +444,23 @@ kill_tcp_connections () if ! $__oneway ; then _connections="${_connections}${_nl}${_dst} ${_src}" fi + _src=$(echo "$_src" | sed 's/\[//g;s/\]//g') + _dst=$(echo "$_dst" | sed 's/\[//g;s/\]//g') + "${CTDB_HELPER_BINDIR}/ctdb_killtcp" "$_iface" "$_src" "$_dst" + _conn_kill=$? + if [ $_conn_kill != 0 ] ; then + echo "Failed to kill TCP connection $_src $_dst" + fi + + _kill_res=$_conn_kill || $_kill_res _killcount=$((_killcount + 1)) done - if [ $_killcount -eq 0 ] ; then + if [ $_killcount -eq 0 ] || [ $_kill_res != 0 ] ; then return fi - echo "$_connections" | \ - "${CTDB_HELPER_BINDIR}/ctdb_killtcp" "$_iface" || { - echo "Failed to kill TCP connections" - return - } _connections=$(get_tcp_connections_for_ip "$_ip") ``` Can you integrate that into the next release? Thanks,
Hello Viet, the ctdb_killtcp tool can work with both - arguments and reading from stdin. The kill_tcp_connections() function creates a newline separated ($_nl) list of connections and passes the list to the ctdb_killtcp tool, what makes sense in this case, in my opinion. It should work fine. The handling of the IPv6 connections does not work with the [::]:port notation, that's right. Since this notation is very common (https://datatracker.ietf.org/doc/html/rfc5952#section-6), this issue should be fixed in the code which parses the address (ctdb/protocol/protocol_util.c). Best regards, Björn
Yes, passing connection information to stdin of ctdb_killtcp is much faster than repeatedly invoking it. We should fix the socket parsing code used by ctdb_killtcp. However, for now, why not just apply a workaround like commit 693080abe4d8bec96280af5a6aa668251a98ec5d to shell function get_tcp_connections_for_ip()? It is simple and should work in this case.
Created attachment 17064 [details] Patch Attached find a patch that should make ctdb_killtcp accept the [ipv6]:port notation
Thanks for that Volker! Please see https://gitlab.com/samba-team/devel/samba/-/commits/martins/ctdb-ipv6 for a change on top of your patch that changes printing IPv6 sockets to the same style (you might be able to improve on this) and changes tests accordingly.
(In reply to Martin Schwenke from comment #4) Note that my patch doesn't remove any of the existing [] stripping in scripts. I'm not in a position to test that right now...
Created attachment 17069 [details] Fix ipv6-encoded ipv4 addresses
FYI, i've tested the patch and it works Thanks
https://gitlab.com/samba-team/samba/-/merge_requests/2317 contains this and other commits. For this bug we only really need to backport the actual fix.
This bug was referenced in samba master: 224e99804efef960ef4ce2ff2f4f6dced1e74146
Created attachment 17085 [details] Patch for v4-15-test, v4-14-test Cherry-picks cleanly, applies to both branches, builds. Tests still pass (though they don't test the new behaviour, since tests not backported - that has been tested in master).
Jule, please push the second path to 4-14 and 4-15. Thanks, Volker
Pushed to autobuild-v4-{15,14}-test.
This bug was referenced in samba v4-15-test: 6c28c948a493eed8efd0591fd3d52d6779b74f13
This bug was referenced in samba v4-14-test: 870991a12c5a826141cf3f821a7fc8cd654bdc93
Closing out bug report. Thanks!
This bug was referenced in samba v4-15-stable (Release samba-4.15.4): 6c28c948a493eed8efd0591fd3d52d6779b74f13
This bug was referenced in samba v4-14-stable (Release samba-4.14.13): 870991a12c5a826141cf3f821a7fc8cd654bdc93