From 1fafb83fed2b03c9f8730f6cd191a6b4299ff489 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 21 Oct 2022 21:22:23 +0200 Subject: [PATCH] packet: turn tunnel lock into spinlock Lock is only held to update/check ints, so spin lock will be more efficient. Place the member of Packet in a new "persistent" area to make it clear this is not touched by the PacketReinit logic. Ticket: #5592. --- src/decode.h | 33 ++++++++++++++++++++------------- src/detect-mark.c | 4 ++-- src/packet.c | 4 ++-- src/source-nfq.c | 4 ++-- src/tmqh-packetpool.c | 8 ++++---- 5 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/decode.h b/src/decode.h index 84d2bdbaf9..92ba3a2926 100644 --- a/src/decode.h +++ b/src/decode.h @@ -617,11 +617,6 @@ typedef struct Packet_ * It should always point to the lowest * packet in a encapsulated packet */ - /** mutex to protect access to: - * - tunnel_rtv_cnt - * - tunnel_tpr_cnt - */ - SCMutex tunnel_mutex; /* ready to set verdict counter, only set in root */ uint16_t tunnel_rtv_cnt; /* tunnel packet ref count */ @@ -638,6 +633,16 @@ typedef struct Packet_ #ifdef PROFILING PktProfiling *profile; #endif + /* things in the packet that live beyond a reinit */ + struct { + /** lock to protect access to: + * - tunnel_rtv_cnt + * - tunnel_tpr_cnt + * - nfq_v.mark + * - flags + */ + SCSpinlock tunnel_lock; + } persistent; } Packet; /** highest mtu of the interfaces we monitor */ @@ -773,11 +778,13 @@ void CaptureStatsSetup(ThreadVars *tv, CaptureStats *s); ((p)->root ? (p)->root->tunnel_rtv_cnt++ : (p)->tunnel_rtv_cnt++); \ } while (0) -#define TUNNEL_INCR_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) +static inline void TUNNEL_INCR_PKT_TPR(Packet *p) +{ + Packet *rp = p->root ? p->root : p; + SCSpinLock(&rp->persistent.tunnel_lock); + rp->tunnel_tpr_cnt++; + SCSpinUnlock(&rp->persistent.tunnel_lock); +} #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) @@ -1093,8 +1100,8 @@ static inline void DecodeSetNoPacketInspectionFlag(Packet *p) static inline bool VerdictTunnelPacket(Packet *p) { bool verdict = true; - SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex; - SCMutexLock(m); + SCSpinlock *lock = p->root ? &p->root->persistent.tunnel_lock : &p->persistent.tunnel_lock; + SCSpinLock(lock); const uint16_t outstanding = TUNNEL_PKT_TPR(p) - TUNNEL_PKT_RTV(p); SCLogDebug("tunnel: outstanding %u", outstanding); @@ -1108,7 +1115,7 @@ static inline bool VerdictTunnelPacket(Packet *p) } else { verdict = false; } - SCMutexUnlock(m); + SCSpinUnlock(lock); return verdict; } diff --git a/src/detect-mark.c b/src/detect-mark.c index 553a1eb70e..f058ffeaae 100644 --- a/src/detect-mark.c +++ b/src/detect-mark.c @@ -233,11 +233,11 @@ static int DetectMarkPacket(DetectEngineThreadCtx *det_ctx, Packet *p, * are fine. */ if (p->flags & PKT_REBUILT_FRAGMENT) { Packet *tp = p->root ? p->root : p; - SCMutexLock(&tp->tunnel_mutex); + SCSpinLock(&tp->persistent.tunnel_lock); tp->nfq_v.mark = (nf_data->mark & nf_data->mask) | (tp->nfq_v.mark & ~(nf_data->mask)); tp->flags |= PKT_MARK_MODIFIED; - SCMutexUnlock(&tp->tunnel_mutex); + SCSpinUnlock(&tp->persistent.tunnel_lock); } } } diff --git a/src/packet.c b/src/packet.c index 2c654879f0..b85e3dcff0 100644 --- a/src/packet.c +++ b/src/packet.c @@ -60,7 +60,7 @@ bool PacketCheckAction(const Packet *p, const uint8_t a) */ void PacketInit(Packet *p) { - SCMutexInit(&p->tunnel_mutex, NULL); + SCSpinInit(&p->persistent.tunnel_lock, 0); p->alerts.alerts = PacketAlertCreate(); PACKET_RESET_CHECKSUMS(p); p->livedev = NULL; @@ -181,7 +181,7 @@ void PacketDestructor(Packet *p) } PacketAlertFree(p->alerts.alerts); PACKET_FREE_EXTDATA(p); - SCMutexDestroy(&p->tunnel_mutex); + SCSpinDestroy(&p->persistent.tunnel_lock); AppLayerDecoderEventsFreeEvents(&p->app_layer_events); PACKET_PROFILING_RESET(p); } diff --git a/src/source-nfq.c b/src/source-nfq.c index dc41a7927d..0e8b1bdce6 100644 --- a/src/source-nfq.c +++ b/src/source-nfq.c @@ -496,11 +496,11 @@ static int NFQBypassCallback(Packet *p) * work for those. Rebuilt packets from IP fragments are fine. */ if (p->flags & PKT_REBUILT_FRAGMENT) { Packet *tp = p->root ? p->root : p; - SCMutexLock(&tp->tunnel_mutex); + SCSpinLock(&tp->persistent.tunnel_lock); tp->nfq_v.mark = (nfq_config.bypass_mark & nfq_config.bypass_mask) | (tp->nfq_v.mark & ~nfq_config.bypass_mask); tp->flags |= PKT_MARK_MODIFIED; - SCMutexUnlock(&tp->tunnel_mutex); + SCSpinUnlock(&tp->persistent.tunnel_lock); return 1; } return 0; diff --git a/src/tmqh-packetpool.c b/src/tmqh-packetpool.c index fe0b016f16..7e36e0fef9 100644 --- a/src/tmqh-packetpool.c +++ b/src/tmqh-packetpool.c @@ -365,8 +365,8 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) p,p->root ? "upper layer" : "tunnel root"); /* get a lock to access root packet fields */ - SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex; - SCMutexLock(m); + SCSpinlock *lock = p->root ? &p->root->persistent.tunnel_lock : &p->persistent.tunnel_lock; + SCSpinLock(lock); if (IS_TUNNEL_ROOT_PKT(p)) { SCLogDebug("IS_TUNNEL_ROOT_PKT == TRUE"); @@ -390,7 +390,7 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) SET_TUNNEL_PKT_VERDICTED(p); PACKET_PROFILING_END(p); - SCMutexUnlock(m); + SCSpinUnlock(lock); SCReturn; } } else { @@ -426,7 +426,7 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p) /* fall through */ } } - SCMutexUnlock(m); + SCSpinUnlock(lock); SCLogDebug("tunnel stuff done, move on (proot %d)", proot); }