From b076a26cdc029a54b7f0fdec30b9bb508424a79e Mon Sep 17 00:00:00 2001 From: Ken Steele Date: Wed, 10 Jul 2013 16:47:52 -0400 Subject: [PATCH] Replace ReleaseData function on Packet Structure with ReleasePacket. This commit allows handling Packets allocated by different methods. The ReleaseData function pointer in the Packet structure is replaced with ReleasePacket function pointer, which is then always called to release the memory associated with a Packet. Currently, the only usage of ReleaseData is in AF Packet. Previously ReleaseData was only called when it was not NULL. To implement the same functionality as before in AF Packet, a new function is defined in AF Packet to first call the AFP specific ReleaseData function and then releases the Packet structure. Three new general functions are defined for releasing packets in the default case: 1) PacketFree() - To release a packet alloced with SCMalloc() 2) PacketPoolReturnPacket() - For packets allocated from the Packet Pool. Calls RECYCLE_PACKET(p) 3) PacketFreeOrRelease() - Calls PacketFree() or PacketPoolReturnPacket() based on the PKT_ALLOC flag. Having these functions removes the need to check the PKT_ALLOC flag when releasing a packet in most cases, since the ReleasePacket function encodes how the Packet was allocated. The PKT_ALLOC flag is still set and is needed when AF Packet releases a packet, since it replaces the ReleasePacket function pointer with its own function and then calls PacketFreeOfRelease(), which uses the PKT_ALLOC flag. --- src/decode.c | 39 +++++++++++++++++++++++++++++-------- src/decode.h | 7 ++++--- src/source-af-packet.c | 16 +++++++++------ src/tmqh-packetpool.c | 44 ++++++++++++++---------------------------- src/tmqh-packetpool.h | 4 ++-- 5 files changed, 61 insertions(+), 49 deletions(-) diff --git a/src/decode.c b/src/decode.c index 4a574ac064..e790f47078 100644 --- a/src/decode.c +++ b/src/decode.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2012 Open Information Security Foundation +/* Copyright (C) 2007-2013 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -58,6 +58,7 @@ #include "util-print.h" #include "tmqh-packetpool.h" #include "util-profiling.h" +#include "pkt-var.h" void DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, uint16_t len, PacketQueue *pq, uint8_t proto) @@ -77,6 +78,15 @@ void DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, } } +/** + * \brief Return a malloced packet. + */ +void PacketFree(Packet *p) +{ + PACKET_CLEANUP(p); + SCFree(p); +} + /** * \brief Get a malloced packet. * @@ -91,6 +101,7 @@ Packet *PacketGetFromAlloc(void) memset(p, 0, SIZE_OF_PACKET); PACKET_INITIALIZE(p); + p->ReleasePacket = PacketFree; p->flags |= PKT_ALLOC; SCLogDebug("allocated a new packet only using alloc..."); @@ -99,6 +110,17 @@ Packet *PacketGetFromAlloc(void) return p; } +/** + * \brief Return a packet to where it was allocated. + */ +void PacketFreeOrRelease(Packet *p) +{ + if (p->flags & PKT_ALLOC) + PacketFree(p); + else + PacketPoolReturnPacket(p); +} + /** * \brief Get a packet. We try to get a packet from the packetpool first, but * if that is empty we alloc a packet that is free'd again after @@ -182,8 +204,6 @@ inline int PacketCopyData(Packet *p, uint8_t *pktdata, int pktlen) return PacketCopyDataOffset(p, 0, pktdata, pktlen); } - - /** * \brief Setup a pseudo packet (tunnel) * @@ -248,7 +268,8 @@ Packet *PacketPseudoPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t * * \retval p the pseudo packet or NULL if out of memory */ -Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto) { +Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto) +{ SCEnter(); /* get us a packet */ @@ -381,7 +402,8 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv) * * \todo IPv6 */ -void AddressDebugPrint(Address *a) { +void AddressDebugPrint(Address *a) +{ if (a == NULL) return; @@ -397,7 +419,8 @@ void AddressDebugPrint(Address *a) { } /** \brief Alloc and setup DecodeThreadVars */ -DecodeThreadVars *DecodeThreadVarsAlloc() { +DecodeThreadVars *DecodeThreadVarsAlloc() +{ DecodeThreadVars *dtv = NULL; @@ -412,7 +435,6 @@ DecodeThreadVars *DecodeThreadVarsAlloc() { return dtv; } - /** * \brief Set data for Packet and set length when zeo copy is used * @@ -432,7 +454,8 @@ inline int PacketSetData(Packet *p, uint8_t *pktdata, int pktlen) return 0; } -const char *PktSrcToString(enum PktSrcEnum pkt_src) { +const char *PktSrcToString(enum PktSrcEnum pkt_src) +{ char *pkt_src_str = ""; switch (pkt_src) { case PKT_SRC_WIRE: diff --git a/src/decode.h b/src/decode.h index e5d74028ec..e3c2e61a39 100644 --- a/src/decode.h +++ b/src/decode.h @@ -406,8 +406,8 @@ typedef struct Packet_ int debuglog_flowbits_names_len; const char **debuglog_flowbits_names; - /** The release function for packet data */ - TmEcode (*ReleaseData)(ThreadVars *, struct Packet_ *); + /** The release function for packet structure and data */ + void (*ReleasePacket)(struct Packet_ *); /* pkt vars */ PktVar *pktvar; @@ -686,7 +686,6 @@ typedef struct DecodeThreadVars_ (p)->prev = NULL; \ (p)->root = NULL; \ (p)->livedev = NULL; \ - (p)->ReleaseData = NULL; \ PACKET_RESET_CHECKSUMS((p)); \ PACKET_PROFILING_RESET((p)); \ } while (0) @@ -777,6 +776,8 @@ Packet *PacketPseudoPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto); Packet *PacketGetFromQueueOrAlloc(void); Packet *PacketGetFromAlloc(void); +void PacketFree(Packet *p); +void PacketFreeOrRelease(Packet *p); int PacketCopyData(Packet *p, uint8_t *pktdata, int pktlen); int PacketSetData(Packet *p, uint8_t *pktdata, int pktlen); int PacketCopyDataOffset(Packet *p, int offset, uint8_t *data, int datalen); diff --git a/src/source-af-packet.c b/src/source-af-packet.c index 3803427b15..b7a7725b94 100644 --- a/src/source-af-packet.c +++ b/src/source-af-packet.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2011,2012 Open Information Security Foundation +/* Copyright (C) 2011-2013 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -665,13 +665,12 @@ TmEcode AFPWritePacket(Packet *p) return TM_ECODE_OK; } -TmEcode AFPReleaseDataFromRing(ThreadVars *t, Packet *p) +void AFPReleaseDataFromRing(Packet *p) { - int ret = TM_ECODE_OK; /* Need to be in copy mode and need to detect early release where Ethernet header could not be set (and pseudo packet) */ if ((p->afp_v.copy_mode != AFP_COPY_MODE_NONE) && !PKT_IS_PSEUDOPKT(p)) { - ret = AFPWritePacket(p); + AFPWritePacket(p); } if (AFPDerefSocket(p->afp_v.mpeer) == 0) @@ -685,7 +684,12 @@ TmEcode AFPReleaseDataFromRing(ThreadVars *t, Packet *p) cleanup: AFPV_CLEANUP(&p->afp_v); - return ret; +} + +void AFPReleasePacket(Packet *p) +{ + AFPReleaseDataFromRing(p); + PacketFreeOrRelease(p); } /** @@ -788,7 +792,7 @@ int AFPReadFromRing(AFPThreadVars *ptv) SCReturnInt(AFP_FAILURE); } else { p->afp_v.relptr = h.raw; - p->ReleaseData = AFPReleaseDataFromRing; + p->ReleasePacket = AFPReleasePacket; p->afp_v.mpeer = ptv->mpeer; AFPRefSocket(ptv->mpeer); diff --git a/src/tmqh-packetpool.c b/src/tmqh-packetpool.c index d8f3cc3c30..0b2b862940 100644 --- a/src/tmqh-packetpool.c +++ b/src/tmqh-packetpool.c @@ -99,8 +99,9 @@ void PacketPoolStorePacket(Packet *p) { /* Clear the PKT_ALLOC flag, since that indicates to push back * onto the ring buffer. */ p->flags &= ~PKT_ALLOC; + p->ReleasePacket = PacketPoolReturnPacket; + PacketPoolReturnPacket(p); - RingBufferMrMwPut(ringbuffer, (void *)p); SCLogDebug("buffersize %u", RingBufferSize(ringbuffer)); } @@ -115,6 +116,15 @@ Packet *PacketPoolGetPacket(void) { return p; } +/** \brief Return packet to Packet pool + * + */ +void PacketPoolReturnPacket(Packet *p) +{ + PACKET_RECYCLE(p); + RingBufferMrMwPut(ringbuffer, (void *)p); +} + void PacketPoolInit(intmax_t max_pending_packets) { /* pre allocate packets */ SCLogDebug("preallocating packets... packet size %" PRIuMAX "", (uintmax_t)SIZE_OF_PACKET); @@ -253,33 +263,14 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) FlowDeReference(&p->root->flow); /* if p->root uses extended data, free them */ - if (p->root->ReleaseData) { - if (p->root->ReleaseData(t, p->root) == TM_ECODE_FAILED) { - SCLogWarning(SC_ERR_INVALID_ACTION, - "Unable to release packet data"); - } - } if (p->root->ext_pkt) { if (!(p->root->flags & PKT_ZERO_COPY)) { SCFree(p->root->ext_pkt); } p->root->ext_pkt = NULL; } - if (p->root->flags & PKT_ALLOC) { - PACKET_CLEANUP(p->root); - SCFree(p->root); - p->root = NULL; - } else { - PACKET_RECYCLE(p->root); - RingBufferMrMwPut(ringbuffer, (void *)p->root); - } - - } - - if (p->ReleaseData) { - if (p->ReleaseData(t, p) == TM_ECODE_FAILED) { - SCLogWarning(SC_ERR_INVALID_ACTION, "Unable to release packet data"); - } + p->root->ReleasePacket(p->root); + p->root = NULL; } /* if p uses extended data, free them */ @@ -292,14 +283,7 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) PACKET_PROFILING_END(p); - SCLogDebug("getting rid of tunnel pkt... alloc'd %s (root %p)", p->flags & PKT_ALLOC ? "true" : "false", p->root); - if (p->flags & PKT_ALLOC) { - PACKET_CLEANUP(p); - SCFree(p); - } else { - PACKET_RECYCLE(p); - RingBufferMrMwPut(ringbuffer, (void *)p); - } + p->ReleasePacket(p); SCReturn; } diff --git a/src/tmqh-packetpool.h b/src/tmqh-packetpool.h index b4138324f8..a248c037a8 100644 --- a/src/tmqh-packetpool.h +++ b/src/tmqh-packetpool.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2010 Open Information Security Foundation +/* Copyright (C) 2007-2013 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -33,7 +33,7 @@ Packet *PacketPoolGetPacket(void); uint16_t PacketPoolSize(void); void PacketPoolStorePacket(Packet *); void PacketPoolWait(void); - +void PacketPoolReturnPacket(Packet *p); void PacketPoolInit(intmax_t max_pending_packets); void PacketPoolDestroy(void);