From: Victor Julien <vjulien@oisf.net>
Date: Wed, 27 Mar 2024 08:15:59 +0000 (+0100)
Subject: decode/tcp: optimize SACKOK storage
X-Git-Tag: suricata-8.0.0-beta1~1400
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9632c2f570981577c04af0c9dabc0752ac4b2b41;p=thirdparty%2Fsuricata.git

decode/tcp: optimize SACKOK storage

Take less space in the TCPVars for tracking if SACKOK is set.

Reduces size by 16 bytes.

Ticket: #6938.
---

diff --git a/src/decode-tcp.c b/src/decode-tcp.c
index 4935b446c2..14acc3f4ba 100644
--- a/src/decode-tcp.c
+++ b/src/decode-tcp.c
@@ -111,10 +111,10 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen)
                     if (olen != TCP_OPT_SACKOK_LEN) {
                         ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN);
                     } else {
-                        if (p->tcpvars.sackok.type != 0) {
+                        if (TCP_GET_SACKOK(p)) {
                             ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE);
                         } else {
-                            SET_OPTS(p->tcpvars.sackok, tcp_opts[tcp_opt_cnt]);
+                            p->tcpvars.sack_ok = true;
                         }
                     }
                     break;
@@ -266,11 +266,12 @@ int DecodeTCP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         StatsIncr(tv, dtv->counter_tcp_rst);
     }
 #ifdef DEBUG
-    SCLogDebug("TCP sp: %" PRIu32 " -> dp: %" PRIu32 " - HLEN: %" PRIu32 " LEN: %" PRIu32 " %s%s%s%s%s%s",
-        GET_TCP_SRC_PORT(p), GET_TCP_DST_PORT(p), TCP_GET_HLEN(p), len,
-        TCP_HAS_SACKOK(p) ? "SACKOK " : "", TCP_HAS_SACK(p) ? "SACK " : "",
-        TCP_HAS_WSCALE(p) ? "WS " : "", TCP_HAS_TS(p) ? "TS " : "",
-        TCP_HAS_MSS(p) ? "MSS " : "", TCP_HAS_TFO(p) ? "TFO " : "");
+    SCLogDebug("TCP sp: %" PRIu32 " -> dp: %" PRIu32 " - HLEN: %" PRIu32 " LEN: %" PRIu32
+               " %s%s%s%s%s%s",
+            GET_TCP_SRC_PORT(p), GET_TCP_DST_PORT(p), TCP_GET_HLEN(p), len,
+            TCP_GET_SACKOK(p) ? "SACKOK " : "", TCP_HAS_SACK(p) ? "SACK " : "",
+            TCP_HAS_WSCALE(p) ? "WS " : "", TCP_HAS_TS(p) ? "TS " : "",
+            TCP_HAS_MSS(p) ? "MSS " : "", TCP_HAS_TFO(p) ? "TFO " : "");
 #endif
 
     FlowSetupPacket(p);
diff --git a/src/decode-tcp.h b/src/decode-tcp.h
index 644a2b2f1f..46bfee771f 100644
--- a/src/decode-tcp.h
+++ b/src/decode-tcp.h
@@ -92,7 +92,6 @@
 
 #define TCP_HAS_WSCALE(p)                   ((p)->tcpvars.ws.type == TCP_OPT_WS)
 #define TCP_HAS_SACK(p)                     ((p)->tcpvars.sack.type == TCP_OPT_SACK)
-#define TCP_HAS_SACKOK(p)                   ((p)->tcpvars.sackok.type == TCP_OPT_SACKOK)
 #define TCP_HAS_TS(p)                       ((p)->tcpvars.ts_set)
 #define TCP_HAS_MSS(p)                      ((p)->tcpvars.mss.type == TCP_OPT_MSS)
 #define TCP_HAS_TFO(p)                      ((p)->tcpvars.tfo.type == TCP_OPT_TFO)
@@ -102,7 +101,7 @@
                                                 (((*(uint8_t *)(p)->tcpvars.ws.data) <= TCP_WSCALE_MAX) ? \
                                                   (*(uint8_t *)((p)->tcpvars.ws.data)) : 0) : 0)
 
-#define TCP_GET_SACKOK(p)                    (TCP_HAS_SACKOK((p)) ? 1 : 0)
+#define TCP_GET_SACKOK(p)                    (p)->tcpvars.sack_ok
 #define TCP_GET_SACK_PTR(p)                  TCP_HAS_SACK((p)) ? (p)->tcpvars.sack.data : NULL
 #define TCP_GET_SACK_CNT(p)                  (TCP_HAS_SACK((p)) ? (((p)->tcpvars.sack.len - 2) / 8) : 0)
 #define TCP_GET_MSS(p)                       SCNtohs(*(uint16_t *)((p)->tcpvars.mss.data))
@@ -158,11 +157,11 @@ typedef struct TCPVars_
     bool md5_option_present;
     bool ao_option_present;
     bool ts_set;
+    bool sack_ok;
     uint32_t ts_val;    /* host-order */
     uint32_t ts_ecr;    /* host-order */
     uint16_t stream_pkt_flags;
     TCPOpt sack;
-    TCPOpt sackok;
     TCPOpt ws;
     TCPOpt mss;
     TCPOpt tfo;         /* tcp fast open */
diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index c6038f3007..316f47ef83 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -1305,7 +1305,7 @@ static int StreamTcpPacketStateNone(
             ssn->client.last_ts = 0;
         }
 
-        if (TCP_GET_SACKOK(p) == 1) {
+        if (TCP_GET_SACKOK(p)) {
             ssn->flags |= STREAMTCP_FLAG_SACKOK;
             SCLogDebug("ssn %p: SYN/ACK with SACK permitted, assuming "
                     "SACK permitted for both sides", ssn);
@@ -1357,7 +1357,7 @@ static int StreamTcpPacketStateNone(
             ssn->server.wscale = TCP_GET_WSCALE(p);
         }
 
-        if (TCP_GET_SACKOK(p) == 1) {
+        if (TCP_GET_SACKOK(p)) {
             ssn->flags |= STREAMTCP_FLAG_CLIENT_SACKOK;
             SCLogDebug("ssn %p: SACK permitted on SYN packet", ssn);
         }
@@ -1490,7 +1490,7 @@ static inline void StreamTcp3whsSynAckToStateQueue(Packet *p, TcpStateQueue *q)
     q->ack = TCP_GET_ACK(p);
     q->pkt_ts = SCTIME_SECS(p->ts);
 
-    if (TCP_GET_SACKOK(p) == 1)
+    if (TCP_GET_SACKOK(p))
         q->flags |= STREAMTCP_QUEUE_FLAG_SACK;
 
     if (TCP_HAS_WSCALE(p)) {
@@ -1751,7 +1751,7 @@ static void TcpStateQueueInitFromPktSyn(const Packet *p, TcpStateQueue *q)
     q->win = TCP_GET_WINDOW(p);
     q->pkt_ts = SCTIME_SECS(p->ts);
 
-    if (TCP_GET_SACKOK(p) == 1) {
+    if (TCP_GET_SACKOK(p)) {
         q->flags |= STREAMTCP_QUEUE_FLAG_SACK;
     }
     if (TCP_HAS_WSCALE(p)) {
@@ -1783,7 +1783,7 @@ static void TcpStateQueueInitFromPktSynAck(const Packet *p, TcpStateQueue *q)
     q->win = TCP_GET_WINDOW(p);
     q->pkt_ts = SCTIME_SECS(p->ts);
 
-    if (TCP_GET_SACKOK(p) == 1) {
+    if (TCP_GET_SACKOK(p)) {
         q->flags |= STREAMTCP_QUEUE_FLAG_SACK;
     }
     if (TCP_HAS_WSCALE(p)) {
@@ -2039,7 +2039,7 @@ static int StreamTcpPacketStateSynSent(
             ssn->server.wscale = 0;
         }
 
-        if ((ssn->flags & STREAMTCP_FLAG_CLIENT_SACKOK) && TCP_GET_SACKOK(p) == 1) {
+        if ((ssn->flags & STREAMTCP_FLAG_CLIENT_SACKOK) && TCP_GET_SACKOK(p)) {
             ssn->flags |= STREAMTCP_FLAG_SACKOK;
             SCLogDebug("ssn %p: SACK permitted for 4WHS session", ssn);
         }
@@ -2137,7 +2137,7 @@ static int StreamTcpPacketStateSynSent(
                 ssn->server.wscale = 0;
             }
 
-            if (TCP_GET_SACKOK(p) == 1) {
+            if (TCP_GET_SACKOK(p)) {
                 ssn->flags |= STREAMTCP_FLAG_CLIENT_SACKOK;
             } else {
                 ssn->flags &= ~STREAMTCP_FLAG_CLIENT_SACKOK;