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. > { > - 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 > +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
