Home page logo
/

snort logo Snort mailing list archives

Re: [PATCH 1/1] daq_nfq: fix cfg->timeout usage and remove extra select call
From: Russ Combs <rcombs () sourcefire com>
Date: Fri, 17 Dec 2010 09:45:07 -0500

Thanks Florian.  We'll have a look.

Is this against daq-0.4?

On Fri, Dec 17, 2010 at 8:50 AM, Florian Westphal <fwestphal () astaro com>wrote:

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

------------------------------------------------------------------------------
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

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]
AlienVault