[PATCH 1/1] daq_nfq: fix cfg->timeout usage and remove extra select call

[Florian Westphal <fwestphal () astaro com>]

cfg->timeout is in milliseconds in snort, but was assigned
to tv_sec; i.e. 16 minutes.

Moreover, the call to select() is not needed at all, we can use
RCV_TIMEOUT setsockopt to prevent recv() calls from blocking the daq caller for
too long.

Also, nfnetlink may return -ENOBUFS (to inform userspace that
messages have been lost).
daq_nfq does not need to care about this, so ignore ENOBUFS
and tell the kernel to not notify us in the first place.
---

 os-daq-modules/daq_nfq.c |   75 ++++++++++++++++++++++++---------------------
 1 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/os-daq-modules/daq_nfq.c b/os-daq-modules/daq_nfq.c
index 204dcd0..cdf6b7e 100644
--- a/os-daq-modules/daq_nfq.c
+++ b/os-daq-modules/daq_nfq.c
@@ -74,7 +74,6 @@ typedef struct
     volatile int count;
     int passive;
     uint32_t snaplen;
-    unsigned timeout;
 
     char error[DAQ_ERRBUF_SIZE];
     DAQ_State state;
@@ -182,7 +181,6 @@ static int nfq_daq_get_setup (
     }
 
     impl->snaplen = cfg->snaplen ? cfg->snaplen : IP_MAXPACKET;
-    impl->timeout = cfg->timeout;
     impl->passive = ( cfg->mode == DAQ_MODE_PASSIVE );
 
     return DAQ_SUCCESS;
@@ -193,6 +191,8 @@ static int nfq_daq_get_setup (
 static int nfq_daq_initialize (
     const DAQ_Config_t* cfg, void** handle, char* errBuf, size_t errMax)
 {
+    struct timeval rcv_timeo = { 0 };
+
     if(cfg->name && *(cfg->name))
     {
         snprintf(errBuf, errMax, "The nfq DAQ module does not support interface or readback mode!");
@@ -333,6 +333,26 @@ static int nfq_daq_initialize (
         }
     }
 
+    if (cfg->timeout >= 1000) {
+      rcv_timeo.tv_sec = cfg->timeout / 1000;
+    } else {
+       rcv_timeo.tv_usec = cfg->timeout * 1000;
+    }
+
+    if (cfg->timeout && setsockopt(impl->sock, SOL_SOCKET, SO_RCVTIMEO,
+                   &rcv_timeo, sizeof(rcv_timeo))) {
+            snprintf(errBuf, errMax,
+                "%s: can't set receive timeout (%s)\n",
+                    __FUNCTION__, strerror(errno));
+                return DAQ_ERROR;
+    }
+
+    // tell kernel that we do not need to know about queue overflows.
+    // Without this, read operations on the netlink socket will return
+    // ENOBUFS on queue overrun.
+    setsockopt(impl->sock, SOL_NETLINK, NETLINK_NO_ENOBUFS, &(int){1},
+                                                         sizeof(int));
+
     impl->state = DAQ_STATE_INITIALIZED;
 
     *handle = impl;
@@ -468,9 +488,6 @@ static int nfq_daq_acquire (
     NfqImpl *impl = (NfqImpl*)handle;
 
     int n = 0;
-    fd_set fdset;
-    struct timeval tv;
-    tv.tv_usec = 0;
 
     // If c is <= 0, don't limit the packets acquired.  However,
     // impl->count = 0 has a special meaning, so interpret accordingly.
@@ -480,42 +497,30 @@ static int nfq_daq_acquire (
 
     while ( (impl->count < 0) || (n < impl->count) )
     {
-        FD_ZERO(&fdset);
-        FD_SET(impl->sock, &fdset);
+        int len = recv(impl->sock, impl->buf, MSG_BUF_SIZE, 0);
 
-        // set this per call
-        tv.tv_sec = impl->timeout;
-        tv.tv_usec = 0;
-
-        // at least ipq had a timeout!
-        if ( select(impl->sock+1, &fdset, NULL, NULL, &tv) < 0 )
+        if ( len > 0)
         {
-            if ( errno == EINTR )
-                break;
-            DPE(impl->error, "%s: select = %s",
-                __FUNCTION__, strerror(errno));
-            return DAQ_ERROR;
-        }
+            int stat = nfq_handle_packet(
+                impl->nf_handle, (char*)impl->buf, len);
 
-        if (FD_ISSET(impl->sock, &fdset))
-        {
-            int len = recv(impl->sock, impl->buf, MSG_BUF_SIZE, 0);
+            impl->stats.hw_packets_received++;
 
-            if ( len > 0)
+            if ( stat < 0 )
             {
-                int stat = nfq_handle_packet(
-                    impl->nf_handle, (char*)impl->buf, len);
-
-                impl->stats.hw_packets_received++;
-
-                if ( stat < 0 )
-                {
-                    DPE(impl->error, "%s: nfq_handle_packet = %s",
-                        __FUNCTION__, strerror(errno));
-                    return DAQ_ERROR;
-                }
-                n++;
+                DPE(impl->error, "%s: nfq_handle_packet = %s",
+                    __FUNCTION__, strerror(errno));
+                return DAQ_ERROR;
             }
+            n++;
+        } else {
+           if ( errno == EINTR || errno == EAGAIN )
+                break;
+           if ( errno == ENOBUFS )
+                continue; /* should not happen, we set NO_ENOBUFS sockopt */
+            DPE(impl->error, "%s: read = %s",
+                  __FUNCTION__, strerror(errno));
+            return DAQ_ERROR;
         }
     }
     return 0;
-- 
1.7.2.2


------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel