Suricata

The logic that triggers this lives in detect-engine.c, look at the BreakCapture function and how it is called here https://github.com/OISF/suricata/blob/14c2b6e445b3e8b5f802c4538397c12bfd3f831d/src/detect-engine.c#L1505

The goal is to make sure that all packet threads wake up within a reasonable time, even if the capture threads see no packets for a while. So it appears that the capture threads are not responding quickly enough and thus the BreakCapture is called which in turn calls the pcap_breakloop() function (indirectly).

The issue is that the breakloop result is considered an error. Could you try this patch?

diff --git a/src/source-pcap.c b/src/source-pcap.c
index 9c7388b96..a16e42d05 100644
--- a/src/source-pcap.c
+++ b/src/source-pcap.c
@@ -237,6 +237,10 @@ static void PcapCallbackLoop(char *user, struct pcap_pkthdr *h, u_char *pkt)
     SCReturn;
 }

+#ifndef PCAP_ERROR_BREAK
+#define PCAP_ERROR_BREAK -2
+#endif
+
 /**
  *  \brief Main PCAP reading Loop function
  */
@@ -264,15 +268,15 @@ TmEcode ReceivePcapLoop(ThreadVars *tv, void *data, void *slot)
         /* Right now we just support reading packets one at a time. */
         r = pcap_dispatch(ptv->pcap_handle, packet_q_len,
                           (pcap_handler)PcapCallbackLoop, (u_char *)ptv);
-        if (unlikely(r < 0)) {
+        if (unlikely(r == 0 || r == PCAP_ERROR_BREAK)) {
+            if (r == PCAP_ERROR_BREAK && ptv->cb_result == TM_ECODE_FAILED) {
+                SCReturnInt(TM_ECODE_FAILED);
+            }
+            TmThreadsCaptureHandleTimeout(tv, ptv->slot, NULL);
+        } else if (unlikely(r < 0)) {
             int dbreak = 0;
             SCLogError(SC_ERR_PCAP_DISPATCH, "error code %" PRId32 " %s",
                        r, pcap_geterr(ptv->pcap_handle));
-#ifdef PCAP_ERROR_BREAK
-            if (r == PCAP_ERROR_BREAK) {
-                SCReturnInt(ptv->cb_result);
-            }
-#endif
             do {
                 usleep(PCAP_RECONNECT_TIMEOUT);
                 if (suricata_ctl_flags != 0) {
@@ -287,8 +291,6 @@ TmEcode ReceivePcapLoop(ThreadVars *tv, void *data, void *slot)
         } else if (ptv->cb_result == TM_ECODE_FAILED) {
             SCLogError(SC_ERR_PCAP_DISPATCH, "Pcap callback PcapCallbackLoop failed");
             SCReturnInt(TM_ECODE_FAILED);
-        } else if (unlikely(r == 0)) {
-            TmThreadsCaptureHandleTimeout(tv, ptv->slot, NULL);
         }

         StatsSyncCountersIfSignalled(tv);

Thank you for the patch. I was planning on applying this to the code from the 4.1.4 branch we are running and putting it on one of our redundant sensors to test. However, I then noticed that the 4.1.4 branch calls TmThreadsCaptureInjectPacket instead of TmThreadsCaptureHandleTimeout in the else-if from the patch hunk 3 for lines 287-291, which the patch moves to the lines 264-268 in hunk 2. It seems we would need to test this on the master branch instead of the current version we are running.

You wrote that "The issue is that the breakloop result is considered an error". What I am wondering is if this means this error can be ignored or are we losing visibility momentarily due to the capture threads not responding quickly?

Sorry for the delay. I just applied a modified version of the patch on an instance where we have seen this error on 12 of the last 30 days. I am hoping the modifications I made still have the same effect. I took the patch provided by Victor above and replaced TmThreadsCaptureHandleTimeout with TmThreadsCaptureInjectPacket, since that was the function previously used in the 4.1.4 version in the final else-if condition of patch and TmThreadsCaptureHandleTimeout didn't exist there.

If that invalidates the patch, please let me know. Otherwise I will do my best to get back to you once we have a good idea of the results from the change.

A modified version of the patch (described in last comment) has been in place on one of our 4.1.4 version sensors since 2019-07-09. Since applying the patch, we have not encountered the error.