Paolo Valerio <[email protected]> writes:
> On 30 Jul 2026 at 01:49:19 PM, Aaron Conole <[email protected]> wrote:
>
>> Refactored TCP module to use a new ct private storage area rather than
>> an compatible extended conn struct so that future modules will have
>> access to the TCP state details. This will be needed when getting the
>> actual tcp state of the connection for offload.
>>
>> Assisted-by: Claude Sonnet 4.6 <[email protected]>
>> Signed-off-by: Aaron Conole <[email protected]>
>> ---
>> lib/automake.mk | 1 +
>> lib/conntrack-private.h | 26 +++++++-----
>> lib/conntrack-tcp.c | 77 ++++++++++++++++++-----------------
>> lib/conntrack-tcp.h | 59 +++++++++++++++++++++++++++
>> lib/conntrack.c | 2 +
>> utilities/checkpatch_dict.txt | 1 +
>> 6 files changed, 117 insertions(+), 49 deletions(-)
>> create mode 100644 lib/conntrack-tcp.h
>>
>> diff --git a/lib/automake.mk b/lib/automake.mk
>> index 0a86985e35..8051362354 100644
>> --- a/lib/automake.mk
>> +++ b/lib/automake.mk
>> @@ -50,6 +50,7 @@ lib_libopenvswitch_la_SOURCES = \
>> lib/conntrack-icmp.c \
>> lib/conntrack-private.h \
>> lib/conntrack-tcp.c \
>> + lib/conntrack-tcp.h \
>> lib/conntrack-tftp.c \
>> lib/conntrack-tp.c \
>> lib/conntrack-tp.h \
>> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
>> index f1e8568bd5..6dd6c0ab1c 100644
>> --- a/lib/conntrack-private.h
>> +++ b/lib/conntrack-private.h
>> @@ -162,6 +162,17 @@ struct conn {
>> void *private[CT_CONN_PRIVATE_MAX];
>> };
>>
>> +/* Initialize the mutex and reclaimed flag of a freshly allocated conn.
>> + * Must be called before conn->lock is acquired or before the conn is made
>> + * visible to other threads. Each L4 new_conn callback is responsible for
>> + * calling this on the conn it allocates. */
>> +static inline void
>> +conn_init(struct conn *conn)
>> +{
>> + ovs_mutex_init_adaptive(&conn->lock);
>> + atomic_flag_clear(&conn->reclaimed);
>> +}
>> +
>> enum ct_update_res {
>> CT_UPDATE_INVALID,
>> CT_UPDATE_VALID,
>> @@ -285,17 +296,6 @@ struct ct_l4_proto {
>> struct ct_dpif_protoinfo *);
>> };
>>
>> -/* Initialize the mutex and reclaimed flag of a freshly allocated conn.
>> - * Must be called before conn->lock is acquired or before the conn is made
>> - * visible to other threads. Each L4 new_conn callback is responsible for
>> - * calling this on the conn it allocates. */
>> -static inline void
>> -conn_init(struct conn *conn)
>> -{
>> - ovs_mutex_init_adaptive(&conn->lock);
>> - atomic_flag_clear(&conn->reclaimed);
>> -}
>> -
>> /* Transient lookup context built for each packet; private to conntrack.c
>> and
>> * the ALG helper modules. */
>> struct conn_lookup_ctx {
>> @@ -376,6 +376,10 @@ void expectation_create(struct conntrack *ct, ovs_be16
>> dst_port,
>> void conntrack_ftp_init(void);
>> void conntrack_tftp_init(void);
>>
>> +/* Shared private slot ID for all L4 protocol state; valid after the
>> + * corresponding L4 module init (e.g. conntrack_tcp_init()) is called. */
>> +extern ct_private_id_t conntrack_l4_private_id;
>> +
>> /* conn_private_get() / conn_private_set()
>> *
>> * Fast-path accessors for per-connection private storage slots.
>> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
>> index cd857cd334..5ab300b661 100644
>> --- a/lib/conntrack-tcp.c
>> +++ b/lib/conntrack-tcp.c
>> @@ -3,6 +3,7 @@
>> * Copyright (c) 2002 - 2008 Henning Brauer
>> * Copyright (c) 2012 Gleb Smirnoff <[email protected]>
>> * Copyright (c) 2015, 2016 Nicira, Inc.
>> + * Copyright (c) 2026 Red Hat, Inc.
>> * All rights reserved.
>> *
>> * Redistribution and use in source and binary forms, with or without
>> @@ -39,6 +40,7 @@
>> #include <config.h>
>>
>> #include "conntrack-private.h"
>> +#include "conntrack-tcp.h"
>> #include "conntrack-tp.h"
>> #include "coverage.h"
>> #include "ct-dpif.h"
>> @@ -49,18 +51,7 @@ COVERAGE_DEFINE(conntrack_tcp_seq_chk_bypass);
>> COVERAGE_DEFINE(conntrack_tcp_seq_chk_failed);
>> COVERAGE_DEFINE(conntrack_invalid_tcp_flags);
>>
>> -struct tcp_peer {
>> - uint32_t seqlo; /* Max sequence number sent
>> */
>> - uint32_t seqhi; /* Max the other end ACKd + win
>> */
>> - uint16_t max_win; /* largest window (pre scaling)
>> */
>> - uint8_t wscale; /* window scaling factor
>> */
>> - enum ct_dpif_tcp_state state;
>> -};
>> -
>> -struct conn_tcp {
>> - struct conn up;
>> - struct tcp_peer peer[2]; /* 'conn' lock protected. */
>> -};
>> +ct_private_id_t conntrack_l4_private_id = CT_PRIVATE_ID_INVALID;
>>
>> enum {
>> TCPOPT_EOL,
>> @@ -79,12 +70,6 @@ enum {
>> #define SEQ_MIN(a, b) INT_MOD_MIN(a, b)
>> #define SEQ_MAX(a, b) INT_MOD_MAX(a, b)
>>
>> -static struct conn_tcp*
>> -conn_tcp_cast(const struct conn* conn)
>> -{
>> - return CONTAINER_OF(conn, struct conn_tcp, up);
>> -}
>> -
>> /* pf does this in in pf_normalize_tcp(), and it is called only if scrub
>> * is enabled. We're not scrubbing, but this check seems reasonable. */
>> static bool
>> @@ -113,9 +98,6 @@ tcp_invalid_flags(uint16_t flags)
>> }
>>
>> #define TCP_MAX_WSCALE 14
>> -#define CT_WSCALE_FLAG 0x80
>> -#define CT_WSCALE_UNKNOWN 0x40
>> -#define CT_WSCALE_MASK 0xf
>>
>> static uint8_t
>> tcp_get_wscale(const struct tcp_header *tcp)
>> @@ -163,8 +145,12 @@ tcp_bypass_seq_chk(struct conntrack *ct)
>> static enum ct_update_res
>> tcp_conn_update(struct conntrack *ct, struct conn *conn_,
>> struct dp_packet *pkt, bool reply, long long now)
>> + OVS_REQUIRES(conn_->lock)
>
> This and protoinfo are called indirectly. I'm not sure the lock
> requirement is enforced through indirect call. Maybe they are, but worth
> a double check.
I will double check - IIRC, they were checked with some clang version (I
hit some build failures)
>> {
>> - struct conn_tcp *conn = conn_tcp_cast(conn_);
>> + struct conn_tcp_state *conn = conn_tcp_state_get(conn_);
>> + if (!conn) {
>> + return CT_UPDATE_INVALID;
>> + }
>> struct tcp_header *tcp = dp_packet_l4(pkt);
>> /* The peer that sent 'pkt' */
>> struct tcp_peer *src = &conn->peer[reply ? 1 : 0];
>> @@ -189,7 +175,7 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
>> return CT_UPDATE_NEW;
>> } else if (src->state <= CT_DPIF_TCPS_SYN_SENT) {
>> src->state = CT_DPIF_TCPS_SYN_SENT;
>> - conn_update_expiration(ct, &conn->up, CT_TM_TCP_FIRST_PACKET,
>> now);
>> + conn_update_expiration(ct, conn_, CT_TM_TCP_FIRST_PACKET, now);
>> return CT_UPDATE_VALID_NEW;
>> }
>> }
>> @@ -340,18 +326,18 @@ tcp_conn_update(struct conntrack *ct, struct conn
>> *conn_,
>>
>> if (src->state >= CT_DPIF_TCPS_FIN_WAIT_2
>> && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
>> - conn_update_expiration(ct, &conn->up, CT_TM_TCP_CLOSED, now);
>> + conn_update_expiration(ct, conn_, CT_TM_TCP_CLOSED, now);
>> } else if (src->state >= CT_DPIF_TCPS_CLOSING
>> && dst->state >= CT_DPIF_TCPS_CLOSING) {
>> - conn_update_expiration(ct, &conn->up, CT_TM_TCP_FIN_WAIT, now);
>> + conn_update_expiration(ct, conn_, CT_TM_TCP_FIN_WAIT, now);
>> } else if (src->state < CT_DPIF_TCPS_ESTABLISHED
>> || dst->state < CT_DPIF_TCPS_ESTABLISHED) {
>> - conn_update_expiration(ct, &conn->up, CT_TM_TCP_OPENING, now);
>> + conn_update_expiration(ct, conn_, CT_TM_TCP_OPENING, now);
>> } else if (src->state >= CT_DPIF_TCPS_CLOSING
>> || dst->state >= CT_DPIF_TCPS_CLOSING) {
>> - conn_update_expiration(ct, &conn->up, CT_TM_TCP_CLOSING, now);
>> + conn_update_expiration(ct, conn_, CT_TM_TCP_CLOSING, now);
>> } else {
>> - conn_update_expiration(ct, &conn->up, CT_TM_TCP_ESTABLISHED,
>> now);
>> + conn_update_expiration(ct, conn_, CT_TM_TCP_ESTABLISHED, now);
>> }
>> } else if ((dst->state < CT_DPIF_TCPS_SYN_SENT
>> || dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
>> @@ -439,15 +425,14 @@ static struct conn *
>> tcp_new_conn(struct conntrack *ct, struct dp_packet *pkt, long long now,
>> uint32_t tp_id)
>> {
>> - struct conn_tcp* newconn = NULL;
>> struct tcp_header *tcp = dp_packet_l4(pkt);
>> + struct conn_tcp_state *tcp_state;
>> struct tcp_peer *src, *dst;
>> uint16_t tcp_flags = TCP_FLAGS(tcp->tcp_ctl);
>>
>> - newconn = xzalloc(sizeof *newconn);
>> -
>> - src = &newconn->peer[0];
>> - dst = &newconn->peer[1];
>> + tcp_state = xzalloc(sizeof *tcp_state);
>> + src = &tcp_state->peer[0];
>> + dst = &tcp_state->peer[1];
>>
>> src->seqlo = ntohl(get_16aligned_be32(&tcp->tcp_seq));
>> src->seqhi = src->seqlo + dp_packet_get_tcp_payload_length(pkt) + 1;
>> @@ -473,11 +458,15 @@ tcp_new_conn(struct conntrack *ct, struct dp_packet
>> *pkt, long long now,
>> src->state = CT_DPIF_TCPS_SYN_SENT;
>> dst->state = CT_DPIF_TCPS_CLOSED;
>>
>> - newconn->up.tp_id = tp_id;
>> - conn_init(&newconn->up);
>> - conn_init_expiration(ct, &newconn->up, CT_TM_TCP_FIRST_PACKET, now);
>> + struct conn *newconn = xzalloc(sizeof *newconn);
>> + newconn->tp_id = tp_id;
>> + conn_init(newconn);
>> + ovs_mutex_lock(&newconn->lock);
>> + conn_private_set(newconn, conntrack_l4_private_id, tcp_state);
>> + ovs_mutex_unlock(&newconn->lock);
>> + conn_init_expiration(ct, newconn, CT_TM_TCP_FIRST_PACKET, now);
>>
>> - return &newconn->up;
>> + return newconn;
>> }
>>
>> static uint8_t
>> @@ -499,8 +488,12 @@ tcp_peer_to_protoinfo_flags(const struct tcp_peer *peer)
>> static void
>> tcp_conn_get_protoinfo(const struct conn *conn_,
>> struct ct_dpif_protoinfo *protoinfo)
>> + OVS_REQUIRES(conn_->lock)
>> {
>> - const struct conn_tcp *conn = conn_tcp_cast(conn_);
>> + const struct conn_tcp_state *conn = conn_tcp_state_get(conn_);
>> + if (!conn) {
>> + return;
>> + }
>>
>> protoinfo->proto = IPPROTO_TCP;
>> protoinfo->tcp.state_orig = conn->peer[0].state;
>> @@ -519,3 +512,11 @@ struct ct_l4_proto ct_proto_tcp = {
>> .conn_update = tcp_conn_update,
>> .conn_get_protoinfo = tcp_conn_get_protoinfo,
>> };
>> +
>> +
>
> nit: extra new line
d'oh - will fix.
>> +void
>> +conntrack_tcp_init(void)
>> +{
>> + conntrack_l4_private_id = conn_private_id_alloc(free);
>> + ovs_assert(conntrack_l4_private_id != CT_PRIVATE_ID_INVALID);
>> +}
>> diff --git a/lib/conntrack-tcp.h b/lib/conntrack-tcp.h
>> new file mode 100644
>> index 0000000000..a80b762a09
>> --- /dev/null
>> +++ b/lib/conntrack-tcp.h
>> @@ -0,0 +1,59 @@
>> +/*
>> + * Copyright (c) 2026 Red Hat, Inc.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at:
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#ifndef CONNTRACK_TCP_H
>> +#define CONNTRACK_TCP_H
>> +
>> +#include "conntrack-private.h"
>> +#include "ct-dpif.h"
>> +
>> +/* wscale field flags stored in tcp_peer.wscale. */
>> +#define CT_WSCALE_FLAG 0x80 /* Negotiated window scaling is in use. */
>> +#define CT_WSCALE_UNKNOWN 0x40 /* Scale factor not yet known. */
>> +#define CT_WSCALE_MASK 0x0f /* Actual scale factor (0-14). */
>> +
>> +/* Per-direction TCP state tracked by the conntrack TCP module. */
>> +struct tcp_peer {
>> + uint32_t seqlo; /* Max sequence number sent. */
>> + uint32_t seqhi; /* Max the other end ACKed + win. */
>> + uint16_t max_win; /* Largest window (pre-scaling). */
>> + uint8_t wscale; /* Window scaling factor + flags. */
>> + enum ct_dpif_tcp_state state;
>> +};
>> +
>> +/* TCP-specific connection state stored in the conntrack private data slot.
>> + * Access via conn_tcp_state_get(). */
>> +struct conn_tcp_state {
>> + struct tcp_peer peer[2]; /* peer[0]=original, peer[1]=reply. */
>> +};
>> +
>> +/* Must be called once at module initialization before any connections are
>> + * created (called internally by conntrack_init()). */
>> +void conntrack_tcp_init(void);
>> +
>> +/* Returns the TCP state for 'conn', or NULL if not a TCP connection or
>> + * conntrack_tcp_init() has not been called. */
>> +static inline struct conn_tcp_state *
>> +conn_tcp_state_get(const struct conn *conn)
>> + OVS_REQUIRES(conn->lock)
>> +{
>> + if (conntrack_l4_private_id == CT_PRIVATE_ID_INVALID) {
>> + return NULL;
>> + }
>> + return conn_private_get(conn, conntrack_l4_private_id);
>> +}
>> +
>> +#endif /* CONNTRACK_TCP_H */
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index 9d726e5524..d183d4920b 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -24,6 +24,7 @@
>>
>> #include "conntrack.h"
>> #include "conntrack-private.h"
>> +#include "conntrack-tcp.h"
>> #include "conntrack-tp.h"
>> #include "coverage.h"
>> #include "crc32c.h"
>> @@ -223,6 +224,7 @@ conntrack_init(void)
>> l4_protos[IPPROTO_ICMP] = &ct_proto_icmp4;
>> l4_protos[IPPROTO_ICMPV6] = &ct_proto_icmp6;
>>
>> + conntrack_tcp_init();
>> conntrack_ftp_init();
>> conntrack_tftp_init();
>>
>> diff --git a/utilities/checkpatch_dict.txt b/utilities/checkpatch_dict.txt
>> index 027da639a8..53119bc3c7 100644
>> --- a/utilities/checkpatch_dict.txt
>> +++ b/utilities/checkpatch_dict.txt
>> @@ -335,6 +335,7 @@ wc
>> wget
>> whcan
>> wildcard
>> +wscale
>> x86_64
>> xbundle
>> xbundles
>> --
>> 2.51.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
