From 3b2853e91056586dda1bb9ff313b09aebd55bc5d Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 15 Aug 2023 10:43:57 +1000 Subject: [PATCH 1/3] ctdb-common: Improve error handling Factor out a failure label, which will get more use in subsequent commits, and only set private_data when success is certain. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15451 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit d87041d8968e91db9d257445321b85693303f95e) --- ctdb/common/system_socket.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ctdb/common/system_socket.c b/ctdb/common/system_socket.c index 06dc558eb22..3a7b6eb41de 100644 --- a/ctdb/common/system_socket.c +++ b/ctdb/common/system_socket.c @@ -988,7 +988,6 @@ int ctdb_sys_open_capture_socket(const char *iface, void **private_data) errbuf); return -1; } - *((pcap_t **)private_data) = pt; pcap_packet_type = pcap_datalink(pt); switch (pcap_packet_type) { @@ -1005,8 +1004,7 @@ int ctdb_sys_open_capture_socket(const char *iface, void **private_data) #endif /* DLT_LINUX_SLL2 */ default: DBG_ERR("Unknown pcap packet type %d\n", pcap_packet_type); - pcap_close(pt); - return -1; + goto fail; } fd = pcap_get_selectable_fd(pt); @@ -1014,7 +1012,12 @@ int ctdb_sys_open_capture_socket(const char *iface, void **private_data) t, fd); + *((pcap_t **)private_data) = pt; return fd; + +fail: + pcap_close(pt); + return -1; } int ctdb_sys_close_capture_socket(void *private_data) -- 2.39.2 From 4130971e9627f132c3581e207acdc1fb095a839d Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 15 Aug 2023 10:57:59 +1000 Subject: [PATCH 2/3] ctdb-common: Replace pcap_open_live() by lower level calls A subsequent commit will insert an additional call before pcap_activate(). This sequence of calls is taken from the source for pcap_open_live(), so there should be no change in behaviour. Given the defaults set by pcap_create_common(), it would be possible to omit the calls to pcap_set_promisc() and pcap_set_timeout(). However, those defaults don't seem to be well documented, so continue to explicitly set everything that was set before. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15451 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit ffc2ae616d8fab7528fbdfd8c6b94c5b9a0e3a7c) --- ctdb/common/system_socket.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/ctdb/common/system_socket.c b/ctdb/common/system_socket.c index 3a7b6eb41de..16463af5a33 100644 --- a/ctdb/common/system_socket.c +++ b/ctdb/common/system_socket.c @@ -980,14 +980,38 @@ int ctdb_sys_open_capture_socket(const char *iface, void **private_data) int pcap_packet_type; const char *t = NULL; int fd; + int ret; - pt = pcap_open_live(iface, 100, 0, 0, errbuf); + pt = pcap_create(iface, errbuf); if (pt == NULL) { DBG_ERR("Failed to open pcap capture device %s (%s)\n", iface, errbuf); return -1; } + /* + * pcap isn't very clear about defaults... + */ + ret = pcap_set_snaplen(pt, 100); + if (ret < 0) { + DBG_ERR("Failed to set snaplen for pcap capture\n"); + goto fail; + } + ret = pcap_set_promisc(pt, 0); + if (ret < 0) { + DBG_ERR("Failed to unset promiscuous mode for pcap capture\n"); + goto fail; + } + ret = pcap_set_timeout(pt, 0); + if (ret < 0) { + DBG_ERR("Failed to set timeout for pcap capture\n"); + goto fail; + } + ret = pcap_activate(pt); + if (ret < 0) { + DBG_ERR("Failed to activate pcap capture\n"); + goto fail; + } pcap_packet_type = pcap_datalink(pt); switch (pcap_packet_type) { -- 2.39.2 From 265a31b9274b62743ab36d7569f7b15d0acc6fb2 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 15 Aug 2023 12:34:20 +1000 Subject: [PATCH 3/3] ctdb-common: Set immediate mode for pcap capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a problem where ctdb_killtcp (almost always) fails to capture packets with --enable-pcap and libpcap ≥ 1.9.1. The problem is due to a gradual change in libpcap semantics when using pcap_get_selectable_fd(3PCAP) to get a file descriptor and then using that file descriptor in non-blocking mode. pcap_set_immediate_mode(3PCAP) says: pcap_set_immediate_mode() sets whether immediate mode should be set on a capture handle when the handle is activated. In immediate mode, packets are always delivered as soon as they arrive, with no buffering. and On Linux, with previous releases of libpcap, capture devices are always in immediate mode; however, in 1.5.0 and later, they are, by default, not in immediate mode, so if pcap_set_immediate_mode() is available, it should be used. However, it wasn't until libpcap commit 2ade7676101366983bd4f86bc039ffd25da8c126 (before libpcap 1.9.1) that it became a requirement to use pcap_set_immediate_mode(), even with a timeout of 0. More explanation in this libpcap issue comment: https://github.com/the-tcpdump-group/libpcap/issues/860#issuecomment-541204548 Do a configure check for pcap_set_immediate_mode() even though it has existed for 10 years. It is easy enough. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15451 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs Autobuild-User(master): Amitay Isaacs Autobuild-Date(master): Tue Aug 15 10:53:52 UTC 2023 on atb-devel-224 (cherry picked from commit dc7b48c404337891b5105df4d6751cf549a533eb) --- ctdb/common/system_socket.c | 7 +++++++ ctdb/wscript | 1 + 2 files changed, 8 insertions(+) diff --git a/ctdb/common/system_socket.c b/ctdb/common/system_socket.c index 16463af5a33..273b9c3400e 100644 --- a/ctdb/common/system_socket.c +++ b/ctdb/common/system_socket.c @@ -1007,6 +1007,13 @@ int ctdb_sys_open_capture_socket(const char *iface, void **private_data) DBG_ERR("Failed to set timeout for pcap capture\n"); goto fail; } +#ifdef HAVE_PCAP_SET_IMMEDIATE_MODE + ret = pcap_set_immediate_mode(pt, 1); + if (ret < 0) { + DBG_ERR("Failed to set immediate mode for pcap capture\n"); + goto fail; + } +#endif ret = pcap_activate(pt); if (ret < 0) { DBG_ERR("Failed to activate pcap capture\n"); diff --git a/ctdb/wscript b/ctdb/wscript index 88e42439f5a..a7b04541014 100644 --- a/ctdb/wscript +++ b/ctdb/wscript @@ -221,6 +221,7 @@ def configure(conf): if not conf.CHECK_FUNCS_IN('pcap_open_live', 'pcap', headers='pcap.h'): Logs.error('Need libpcap') sys.exit(1) + conf.CHECK_FUNCS_IN('pcap_set_immediate_mode', 'pcap', headers='pcap.h') if not conf.CHECK_FUNCS_IN('backtrace backtrace_symbols', 'execinfo', checklibc=True, headers='execinfo.h'): -- 2.39.2