diff --git a/src/decode.h b/src/decode.h index 92ae217c82..ca44cacbf7 100644 --- a/src/decode.h +++ b/src/decode.h @@ -870,10 +870,8 @@ void CaptureStatsSetup(ThreadVars *tv, CaptureStats *s); ((p)->action |= a)); \ } while (0) -#define TUNNEL_INCR_PKT_RTV(p) do { \ - SCMutexLock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \ +#define TUNNEL_INCR_PKT_RTV_NOLOCK(p) do { \ ((p)->root ? (p)->root->tunnel_rtv_cnt++ : (p)->tunnel_rtv_cnt++); \ - SCMutexUnlock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \ } while (0) #define TUNNEL_INCR_PKT_TPR(p) do { \ @@ -882,16 +880,6 @@ void CaptureStatsSetup(ThreadVars *tv, CaptureStats *s); SCMutexUnlock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \ } while (0) -#define TUNNEL_DECR_PKT_TPR(p) do { \ - SCMutexLock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \ - ((p)->root ? (p)->root->tunnel_tpr_cnt-- : (p)->tunnel_tpr_cnt--); \ - SCMutexUnlock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \ - } while (0) - -#define TUNNEL_DECR_PKT_TPR_NOLOCK(p) do { \ - ((p)->root ? (p)->root->tunnel_tpr_cnt-- : (p)->tunnel_tpr_cnt--); \ - } while (0) - #define TUNNEL_PKT_RTV(p) ((p)->root ? (p)->root->tunnel_rtv_cnt : (p)->tunnel_rtv_cnt) #define TUNNEL_PKT_TPR(p) ((p)->root ? (p)->root->tunnel_tpr_cnt : (p)->tunnel_tpr_cnt) @@ -1132,5 +1120,37 @@ int DecoderParseDataFromFileSerie(char *fileprefix, DecoderFunc Decoder); #define PKT_SET_SRC(p, src_val) ((p)->pkt_src = src_val) +/** \brief return true if *this* packet needs to trigger a verdict. + * + * If we have the root packet, and we have none outstanding, + * we can verdict now. + * + * If we have a upper layer packet, it's the only one and root + * is already processed, we can verdict now. + * + * Otherwise, a future packet will issue the verdict. + */ +static inline bool VerdictTunnelPacket(Packet *p) +{ + bool verdict = true; + SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex; + SCMutexLock(m); + const uint16_t outstanding = TUNNEL_PKT_TPR(p) - TUNNEL_PKT_RTV(p); + SCLogDebug("tunnel: outstanding %u", outstanding); + + /* if there are packets outstanding, we won't verdict this one */ + if (IS_TUNNEL_ROOT_PKT(p) && !IS_TUNNEL_PKT_VERDICTED(p) && !outstanding) { + // verdict + SCLogDebug("root %p: verdict", p); + } else if (!IS_TUNNEL_ROOT_PKT(p) && outstanding == 1 && p->root && IS_TUNNEL_PKT_VERDICTED(p->root)) { + // verdict + SCLogDebug("tunnel %p: verdict", p); + } else { + verdict = false; + } + SCMutexUnlock(m); + return verdict; +} + #endif /* __DECODE_H__ */ diff --git a/src/source-ipfw.c b/src/source-ipfw.c index 40742afc55..6821d69456 100644 --- a/src/source-ipfw.c +++ b/src/source-ipfw.c @@ -623,30 +623,12 @@ TmEcode VerdictIPFW(ThreadVars *tv, Packet *p, void *data, PacketQueue *pq, Pack * if this is a tunnel packet we check if we are ready to verdict * already. */ if (IS_TUNNEL_PKT(p)) { - char verdict = 1; - - SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex; - SCMutexLock(m); - - /* if there are more tunnel packets than ready to verdict packets, - * we won't verdict this one - */ - if (TUNNEL_PKT_TPR(p) > TUNNEL_PKT_RTV(p)) { - SCLogDebug("VerdictIPFW: not ready to verdict yet: " - "TUNNEL_PKT_TPR(p) > TUNNEL_PKT_RTV(p) = %" PRId32 - " > %" PRId32 "", TUNNEL_PKT_TPR(p), TUNNEL_PKT_RTV(p)); - verdict = 0; - } - - SCMutexUnlock(m); + bool verdict = VerdictTunnelPacket(p); /* don't verdict if we are not ready */ - if (verdict == 1) { + if (verdict == true) { SCLogDebug("Setting verdict on tunnel"); retval = IPFWSetVerdict(tv, ptv, p->root ? p->root : p); - - } else { - TUNNEL_INCR_PKT_RTV(p); } } else { /* no tunnel, verdict normally */ diff --git a/src/source-nfq.c b/src/source-nfq.c index 2a8a288489..4c13f5a737 100644 --- a/src/source-nfq.c +++ b/src/source-nfq.c @@ -943,7 +943,8 @@ static void NFQRecvPkt(NFQQueueVars *t, NFQThreadVars *tv) NFQMutexUnlock(t); if (ret != 0) { - SCLogWarning(SC_ERR_NFQ_HANDLE_PKT, "nfq_handle_packet error %" PRId32 "", ret); + SCLogWarning(SC_ERR_NFQ_HANDLE_PKT, "nfq_handle_packet error %"PRId32" %s", + ret, strerror(errno)); } } } @@ -1145,37 +1146,21 @@ TmEcode VerdictNFQ(ThreadVars *tv, Packet *p, void *data, PacketQueue *pq, Packe /* if this is a tunnel packet we check if we are ready to verdict * already. */ if (IS_TUNNEL_PKT(p)) { - char verdict = 1; - //printf("VerdictNFQ: tunnel pkt: %p %s\n", p, p->root ? "upper layer" : "root"); - - SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex; - SCMutexLock(m); - - /* if there are more tunnel packets than ready to verdict packets, - * we won't verdict this one */ - if (TUNNEL_PKT_TPR(p) > TUNNEL_PKT_RTV(p)) { - SCLogDebug("not ready to verdict yet: TUNNEL_PKT_TPR(p) > " - "TUNNEL_PKT_RTV(p) = %" PRId32 " > %" PRId32, - TUNNEL_PKT_TPR(p), TUNNEL_PKT_RTV(p)); - verdict = 0; - } - - SCMutexUnlock(m); - + SCLogDebug("tunnel pkt: %p/%p %s", p, p->root, p->root ? "upper layer" : "root"); + bool verdict = VerdictTunnelPacket(p); /* don't verdict if we are not ready */ - if (verdict == 1) { - //printf("VerdictNFQ: setting verdict\n"); + if (verdict == true) { ret = NFQSetVerdict(p->root ? p->root : p); - if (ret != TM_ECODE_OK) + if (ret != TM_ECODE_OK) { return ret; - } else { - TUNNEL_INCR_PKT_RTV(p); + } } } else { /* no tunnel, verdict normally */ ret = NFQSetVerdict(p); - if (ret != TM_ECODE_OK) + if (ret != TM_ECODE_OK) { return ret; + } } return TM_ECODE_OK; } diff --git a/src/stream-tcp.c b/src/stream-tcp.c index c62e539f75..4c2dc44e7f 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -5844,6 +5844,7 @@ static void StreamTcpPseudoPacketCreateDetectLogFlush(ThreadVars *tv, #endif } + SCLogDebug("np %p", np); PacketEnqueue(pq, np); StatsIncr(tv, stt->counter_tcp_pseudo); diff --git a/src/tmqh-packetpool.c b/src/tmqh-packetpool.c index 913cb3e78c..a94ce20c4e 100644 --- a/src/tmqh-packetpool.c +++ b/src/tmqh-packetpool.c @@ -443,7 +443,7 @@ Packet *TmqhInputPacketpool(ThreadVars *tv) void TmqhOutputPacketpool(ThreadVars *t, Packet *p) { - int proot = 0; + bool proot = false; SCEnter(); SCLogDebug("Packet %p, p->root %p, alloced %s", p, p->root, p->flags & PKT_ALLOC ? "true" : "false"); @@ -458,17 +458,19 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) if (IS_TUNNEL_ROOT_PKT(p)) { SCLogDebug("IS_TUNNEL_ROOT_PKT == TRUE"); - if (TUNNEL_PKT_TPR(p) == 0) { - SCLogDebug("TUNNEL_PKT_TPR(p) == 0, no more tunnel packet " - "depending on this root"); + const uint16_t outstanding = TUNNEL_PKT_TPR(p) - TUNNEL_PKT_RTV(p); + SCLogDebug("root pkt: outstanding %u", outstanding); + if (outstanding == 0) { + SCLogDebug("no tunnel packets outstanding, no more tunnel " + "packet(s) depending on this root"); /* if this packet is the root and there are no - * more tunnel packets, return it to the pool */ - - /* fall through */ + * more tunnel packets to consider + * + * return it to the pool */ } else { - SCLogDebug("tunnel root Packet %p: TUNNEL_PKT_TPR(p) > 0, so " + SCLogDebug("tunnel root Packet %p: outstanding > 0, so " "packets are still depending on this root, setting " - "p->tunnel_verdicted == 1", p); + "SET_TUNNEL_PKT_VERDICTED", p); /* if this is the root and there are more tunnel * packets, return this to the pool. It's still referenced * by the tunnel packets, and we will return it @@ -482,34 +484,34 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) } else { SCLogDebug("NOT IS_TUNNEL_ROOT_PKT, so tunnel pkt"); - /* the p->root != NULL here seems unnecessary: IS_TUNNEL_PKT checks - * that p->tunnel_pkt == 1, IS_TUNNEL_ROOT_PKT checks that + - * p->root == NULL. So when we are here p->root can only be - * non-NULL, right? CLANG thinks differently. May be a FP, but - * better safe than sorry. VJ */ - if (p->root != NULL && IS_TUNNEL_PKT_VERDICTED(p->root) && - TUNNEL_PKT_TPR(p) == 1) + TUNNEL_INCR_PKT_RTV_NOLOCK(p); + const uint16_t outstanding = TUNNEL_PKT_TPR(p) - TUNNEL_PKT_RTV(p); + SCLogDebug("tunnel pkt: outstanding %u", outstanding); + /* all tunnel packets are processed except us. Root already + * processed. So return tunnel pkt and root packet to the + * pool. */ + if (outstanding == 0 && + p->root && IS_TUNNEL_PKT_VERDICTED(p->root)) { - SCLogDebug("p->root->tunnel_verdicted == 1 && TUNNEL_PKT_TPR(p) == 1"); - /* the root is ready and we are the last tunnel packet, - * lets enqueue them both. */ - TUNNEL_DECR_PKT_TPR_NOLOCK(p); + SCLogDebug("root verdicted == true && no outstanding"); - /* handle the root */ + /* handle freeing the root as well*/ SCLogDebug("setting proot = 1 for root pkt, p->root %p " "(tunnel packet %p)", p->root, p); - proot = 1; + proot = true; /* fall through */ - } else { - /* root not ready yet, so get rid of the tunnel pkt only */ - SCLogDebug("NOT p->root->tunnel_verdicted == 1 && " - "TUNNEL_PKT_TPR(p) == 1 (%" PRIu32 ")", TUNNEL_PKT_TPR(p)); + } else { + /* root not ready yet, or not the last tunnel packet, + * so get rid of the tunnel pkt only */ - TUNNEL_DECR_PKT_TPR_NOLOCK(p); + SCLogDebug("NOT IS_TUNNEL_PKT_VERDICTED (%s) || " + "outstanding > 0 (%u)", + (p->root && IS_TUNNEL_PKT_VERDICTED(p->root)) ? "true" : "false", + outstanding); - /* fall through */ + /* fall through */ } } SCMutexUnlock(m); @@ -520,7 +522,7 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) FlowDeReference(&p->flow); /* we're done with the tunnel root now as well */ - if (proot == 1) { + if (proot == true) { SCLogDebug("getting rid of root pkt... alloc'd %s", p->root->flags & PKT_ALLOC ? "true" : "false"); FlowDeReference(&p->root->flow);