Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions felix/bpf-gpl/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ extern const volatile struct cali_tc_preamble_globals __globals;
#define INGRESS_PACKET_RATE_CONFIGURED (GLOBAL_FLAGS & CALI_GLOBALS_INGRESS_PACKET_RATE_CONFIGURED)
#define EGRESS_PACKET_RATE_CONFIGURED (GLOBAL_FLAGS & CALI_GLOBALS_EGRESS_PACKET_RATE_CONFIGURED)
#define WORKLOAD_SRC_SPOOFING_CONFIGURED (GLOBAL_FLAGS & CALI_GLOBALS_WORKLOAD_SRC_SPOOFING_CONFIGURED)
#define INGRESS_CONN_LIMIT_CONFIGURED (GLOBAL_FLAGS & CALI_GLOBALS_INGRESS_CONN_LIMIT_CONFIGURED)
#define EGRESS_CONN_LIMIT_CONFIGURED (GLOBAL_FLAGS & CALI_GLOBALS_EGRESS_CONN_LIMIT_CONFIGURED)

#define MAP_VERSIONED(name, ver) name##ver
#define map_symbol(name, ver) MAP_VERSIONED(name, ver)
Expand Down
70 changes: 69 additions & 1 deletion felix/bpf-gpl/conntrack.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "icmp.h"
#include "types.h"
#include "rpf.h"
#include "qos.h"

#ifdef IPVER6
#define IPPROTO_ICMP_46 IPPROTO_ICMPV6
Expand Down Expand Up @@ -574,6 +575,47 @@ static CALI_BPF_INLINE bool tcp_recycled(bool syn, struct calico_ct_value *v)
return syn && (a->fin_seen || a->rst_seen) && (b->fin_seen || b->rst_seen);
}

/* qos_connlimit_decrement_for_ct decrements the per-pod connlimit counter(s)
* for a CT entry that is closing or being purged. Reads the entry's
* CONNLIMIT_* flags and the per-leg ifindex fields to pick the right
* (ifindex, direction) pair(s). Idempotent: skips entries that already carry
* CONNLIMIT_DEC, and sets DEC before decrementing so a concurrent decrement
* on another CPU sees the flag and bails.
*
* Called from the packet path on FIN-FIN / RST close, and from
* conntrack_cleanup when an idle entry is purged with no close packet.
*/
static CALI_BPF_INLINE void qos_connlimit_decrement_for_ct(struct calico_ct_value *v)
{
__u32 cl_flags = ct_value_get_flags(v);
if (cl_flags & CALI_CT_FLAG_CONNLIMIT_DEC) {
return;
}
if (!(cl_flags & (CALI_CT_FLAG_CONNLIMIT_INGRESS | CALI_CT_FLAG_CONNLIMIT_EGRESS))) {
return;
}
ct_value_set_flags(v, CALI_CT_FLAG_CONNLIMIT_DEC);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Security — qos_connlimit_decrement_for_ct sets CALI_CT_FLAG_CONNLIMIT_DEC then decrements without synchronization:

  • Risk: Two CPUs handling close events for the same flow can race: both read flags without DEC, both set DEC, and both call qos_connlimit_decrement(), resulting in a double-decrement (under-enforcement). Similarly, first‑SYN handling (see below) can race to double-increment. The periodic scanner eventually corrects drift, but within the scan window (~30–60s), limits can be under‑ or over‑enforced (allowing more connections than configured or causing spurious denials).
  • Mitigation:
    • Make DEC setting + decrement atomic per-CT entry. Options include adding a bpf_spin_lock to the conntrack value struct and using it around flag checks/updates, or using an atomic bit test-and-set helper if available. If modifying the CT value is too costly, consider a per-flow dedupe (e.g., a small LRU set keyed by CT key hash) to gate decrements to once-per-flow close.
    • Tighten scanner cadence when connlimits are configured to reduce the correction window.
    • Add FV that stresses concurrent close on the same flow to guard against regressions.


if (cl_flags & CALI_CT_FLAG_CONNLIMIT_INGRESS) {
/* Ingress: pod is the responder (non-opener). */
__u32 pod_ifindex = v->a_to_b.opener
? v->b_to_a.ifindex
: v->a_to_b.ifindex;
if (pod_ifindex != CT_INVALID_IFINDEX) {
qos_connlimit_decrement(pod_ifindex, 1);
}
}
if (cl_flags & CALI_CT_FLAG_CONNLIMIT_EGRESS) {
/* Egress: pod is the opener. */
__u32 pod_ifindex = v->a_to_b.opener
? v->a_to_b.ifindex
: v->b_to_a.ifindex;
if (pod_ifindex != CT_INVALID_IFINDEX) {
qos_connlimit_decrement(pod_ifindex, 0);
}
}
}

static CALI_BPF_INLINE struct calico_ct_result calico_ct_lookup(struct cali_tc_ctx *ctx)
{
struct ct_lookup_ctx ct_lookup_ctx = {
Expand Down Expand Up @@ -997,6 +1039,14 @@ static CALI_BPF_INLINE struct calico_ct_result calico_ct_lookup(struct cali_tc_c
}
}
ct_tcp_entry_update(ctx, tcp_header, src_to_dst, dst_to_src);

/* Decrement connlimit counter when a TCP connection closes
* (both FINs seen or RST). The helper sets CONNLIMIT_DEC
* before decrementing so concurrent paths bail.
*/
if ((src_to_dst->fin_seen && dst_to_src->fin_seen) || tcp_header->rst) {
qos_connlimit_decrement_for_ct(v);
}
}

__u32 ifindex = CALI_F_TO_HOST ? ctx->skb->ifindex : skb_ingress_ifindex(ctx->skb);
Expand Down Expand Up @@ -1097,8 +1147,26 @@ static CALI_BPF_INLINE struct calico_ct_result calico_ct_lookup(struct cali_tc_c
if (syn) {
CALI_CT_DEBUG("packet is SYN");
ct_result_set_flag(result.rc, CT_RES_SYN);
}

/* For ingress connection limit: mark the CT entry when a SYN is
* first seen at a WEP ingress. The CONNLIMIT_INGRESS flag on the
* CT value prevents double-counting on SYN retransmissions.
* CT_RES_CONNLIMIT_FIRST_SYN is set in the result ONLY on the
* first SYN, so the accepted_entrypoint knows to increment.
*/
if (CALI_F_TO_WEP && INGRESS_CONN_LIMIT_CONFIGURED) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Security — First‑SYN marking for ingress (sets CALI_CT_FLAG_CONNLIMIT_INGRESS and signals CT_RES_CONNLIMIT_FIRST_SYN):

  • Risk: This write is also unsynchronized. Parallel handling of SYN/SYN-ACK (or rapid retransmits) on multiple CPUs can race before the flag is visible, causing multiple “first‑SYN” observations and therefore multiple increments. The result is transient over‑enforcement (fewer connections allowed) until the scanner corrects the count.
  • Mitigation:
    • As above, add synchronization around the CT flag update (per‑CT spinlock or atomic bit test-and-set) to ensure single‑writer semantics.
    • Alternatively, derive the acceptance-side increment solely from a single well-defined entrypoint (e.g., only from calico_tc_skb_new_flow_entrypoint on the opener side) and avoid relying on unsynchronized shared CT flags to gate first‑time increments.

if (!(ct_value_get_flags(v) & CALI_CT_FLAG_CONNLIMIT_INGRESS)) {
ct_value_set_flags(v, CALI_CT_FLAG_CONNLIMIT_INGRESS);
ct_result_set_flag(result.rc, CT_RES_CONNLIMIT_FIRST_SYN);
}
/* Propagate REJECTED flag so accepted_entrypoint can
* unconditionally reject retransmissions of previously
* rejected connections. */
if (ct_value_get_flags(v) & CALI_CT_FLAG_CONNLIMIT_REJECTED) {
result.flags |= CALI_CT_FLAG_CONNLIMIT_REJECTED;
}
}
}

CALI_CT_DEBUG("result: 0x%x", result.rc);

Expand Down
6 changes: 6 additions & 0 deletions felix/bpf-gpl/conntrack_cleanup.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ static long process_ccq_entry(void *map, struct calico_ct_key *key, struct cali_
if (!value->rev_key.protocol) {
actual_ct_value = cali_ct_lookup_elem(key);
if (actual_ct_value && (actual_ct_value->last_seen == value->last_seen)) {
// Decrement the per-pod connlimit counter if this entry
// carried one of the CONNLIMIT_* flags and the packet
// path didn't already decrement.
qos_connlimit_decrement_for_ct(actual_ct_value);
if (!cali_ct_delete_elem(key)) {
ictx->num_cleaned++;
}
Expand All @@ -58,6 +62,8 @@ static long process_ccq_entry(void *map, struct calico_ct_key *key, struct cali_
}
struct calico_ct_value *rev_ct_value = cali_ct_lookup_elem(rev_key);
if (rev_ct_value && (rev_ct_value->last_seen == value->rev_last_seen)) {
// The reverse leg holds the connlimit flags + ifindex.
qos_connlimit_decrement_for_ct(rev_ct_value);
if (!cali_ct_delete_elem(rev_key)) {
ictx->num_cleaned++;
}
Expand Down
12 changes: 12 additions & 0 deletions felix/bpf-gpl/conntrack_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ enum cali_ct_type {
#define CALI_CT_FLAG_SET_DSCP 0x8000 /* marks connections that needs to set DSCP */
#define CALI_CT_FLAG_MAGLEV 0X10000 /* marks Maglev connections. Allows packets of an existing to arrive via a different tunnel after failover. */
#define CALI_CT_FLAG_SEND_RESET 0x20000 /* marks connections where we should send a TCP RST on behalf of the workload */
#define CALI_CT_FLAG_CONNLIMIT_INGRESS 0x40000 /* marks connections counted against an ingress connection limit */
#define CALI_CT_FLAG_CONNLIMIT_REJECTED 0x80000 /* marks connections rejected by the connection limit */
#define CALI_CT_FLAG_CONNLIMIT_EGRESS 0x100000 /* marks connections counted against an egress connection limit */
#define CALI_CT_FLAG_CONNLIMIT_DEC 0x200000 /* marks connections already decremented from connlimit counter */

struct calico_ct_leg {
__u64 bytes;
Expand Down Expand Up @@ -134,6 +138,13 @@ static CALI_BPF_INLINE void __xxx_compile_asserts(void) {
ret; \
})

#define ct_value_clear_flags(v, f) do { \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casey Casey — Looks like ct_value_clear_flags() is added but not used anywhere in this diff. Can this be removed for now? Dead helpers tend to invite bit-rot — we can add it back when we actually need clears.

(v)->flags &= ~((f) & 0xff); \
(v)->flags2 &= ~(((f) >> 8) & 0xff); \
(v)->flags3 &= ~(((f) >> 16) & 0xff); \
(v)->flags4 &= ~(((f) >> 24) & 0xff); \
} while(0)

struct ct_lookup_ctx {
__u8 proto;
DECLARE_IP_ADDR(src);
Expand Down Expand Up @@ -219,6 +230,7 @@ enum calico_ct_result_type {
#define CT_RES_SYN 0x1000
#define CT_RES_CONFIRMED 0x2000
#define CT_RES_TO_WORKLOAD 0x4000
#define CT_RES_CONNLIMIT_FIRST_SYN 0x8000

#define ct_result_rc(rc) ((rc) & 0xff)
#define ct_result_flags(rc) ((rc) & ~0xff)
Expand Down
2 changes: 2 additions & 0 deletions felix/bpf-gpl/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ enum cali_globals_flags {
CALI_GLOBALS_EGRESS_PACKET_RATE_CONFIGURED = 0x00008000,
CALI_GLOBALS_UDP_GSO_LINEARIZE = 0x00010000,
CALI_GLOBALS_WORKLOAD_SRC_SPOOFING_CONFIGURED = 0x00020000,
CALI_GLOBALS_INGRESS_CONN_LIMIT_CONFIGURED = 0x00040000,
CALI_GLOBALS_EGRESS_CONN_LIMIT_CONFIGURED = 0x00080000,
};

struct cali_ctlb_globals {
Expand Down
86 changes: 83 additions & 3 deletions felix/bpf-gpl/qos.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ struct calico_qos_key {

struct calico_qos_val {
struct bpf_spin_lock lock;
// config
// packet rate config
__s16 packet_rate;
__s16 packet_burst;
// state
// packet rate state
__s16 packet_rate_tokens;
__s16 padding[3]; // alignment
__u64 packet_rate_last_update;
// connection limit
__s32 max_connections; // 0 = no limit, >0 = limit
__s32 current_count; // maintained by BPF (increment on SYN) and scanner (recount)
};

// 2*IFACE_STATE_MAP_SIZE because it will potentially have 2 entries for each interface (ingress/egress)
CALI_MAP(cali_qos,,
CALI_MAP(cali_qos, 2,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧪 Maintainability & Tests — The QoS BPF map version was bumped to 2 (good), and new fields were added. The header comment still only mentions “packet rate” and does not call out the new connection-limit fields or the map version.

  • Expand the comment above calico_qos_val and the map definition to describe max_connections/current_count semantics (0=disabled), locking, and to note that this is version 2 of cali_qos to aid future readers syncing userspace/BPF layouts.

BPF_MAP_TYPE_HASH,
struct calico_qos_key, struct calico_qos_val,
2*IFACE_STATE_MAP_SIZE, BPF_F_NO_PREALLOC)
Expand Down Expand Up @@ -151,4 +154,81 @@ static CALI_BPF_INLINE bool qos_dscp_set(struct cali_tc_ctx *ctx, __s8 dscp)
return true;
}

/* qos_connlimit_check_and_increment atomically checks the connection limit for
* the current interface and direction using the cali_qos map. If below the
* limit, increments the counter and returns 0 (allow). If at or above the
* limit, returns -1 (reject). Returns 0 if no limit is configured for this
* interface+direction (no map entry or max_connections <= 0).
*
* Only called for TCP SYN on WEP interfaces.
*/
static CALI_BPF_INLINE int qos_connlimit_check_and_increment(struct cali_tc_ctx *ctx)
{
struct calico_qos_key key = {
.ifindex = ctx->skb->ifindex,
#if CALI_F_INGRESS
.ingress = 1,
#else // CALI_F_EGRESS
.ingress = 0,
#endif
};

struct calico_qos_val *qos = cali_qos_lookup_elem(&key);
if (!qos || qos->max_connections <= 0) {
return 0;
}

int rc = 0;

bpf_spin_lock(&qos->lock);
if (qos->current_count >= qos->max_connections) {
rc = -1;
} else {
qos->current_count++;
}
bpf_spin_unlock(&qos->lock);

if (rc < 0) {
CALI_DEBUG("connlimit: over limit (%d >= %d), rejecting",
qos->current_count, qos->max_connections);
} else {
CALI_DEBUG("connlimit: under limit (%d/%d), allowing",
qos->current_count, qos->max_connections);
}

return rc;
}

/* qos_connlimit_decrement decrements the connection limit counter for the
* given interface and direction. Called when a TCP connection closes (both FINs
* seen or RST) and from the cleanup program when a counted CT entry expires.
* Takes an explicit ifindex+direction because the decrement may run from any
* BPF program (from_hep, from_wep, conntrack_cleanup, ...), not just the pod's
* own WEP program.
*/
static CALI_BPF_INLINE void qos_connlimit_decrement(__u32 ifindex, __u32 direction)
{
struct calico_qos_key key = {
.ifindex = ifindex,
.ingress = direction,
};

struct calico_qos_val *qos = cali_qos_lookup_elem(&key);
if (!qos || qos->max_connections <= 0) {
return;
}

bpf_spin_lock(&qos->lock);
if (qos->current_count > 0) {
qos->current_count--;
}
bpf_spin_unlock(&qos->lock);

/* CALI_DEBUG_NO_FLAG (not CALI_DEBUG) — the TC programs' CALI_LOG
* override reads ctx->globals which we don't have here.
*/
CALI_DEBUG_NO_FLAG("connlimit: decremented count to %d/%d",
qos->current_count, qos->max_connections);
}

#endif /* __CALI_QOS_H__ */
65 changes: 65 additions & 0 deletions felix/bpf-gpl/tc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,52 @@ int calico_tc_skb_accepted_entrypoint(struct __sk_buff *skb)
skb_log(ctx, true);
}

/* Ingress connection limit for TCP SYN arriving at a WEP.
* On the first SYN (CT_RES_CONNLIMIT_FIRST_SYN), atomically check the
* limit and increment the counter in the QoS map. On SYN retransmissions,
* always reject if the CT entry was previously rejected (CONNLIMIT_REJECTED
* flag), otherwise allow (the connection was already accepted and counted).
* The Go-side CT scanner periodically recounts and corrects drift.
*/
if (CALI_F_TO_WEP && !policy_skipped &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Security — The ingress connection limit check is gated on “!policy_skipped”:

  • Risk: If policy_skipped is true for traffic to a WEP (e.g., due to a fast-path or a “skip policy” condition), the configured connection limit is not enforced, allowing unlimited new TCP connections despite the QoS limit. This is a policy enforcement bypass that weakens isolation.
  • Mitigation: Ensure that connection limits are enforced independent of policy_skipped (unless there’s a deliberate and documented exception). Either:
    • Remove the policy_skipped clause from this check, or
    • Guarantee that policy_skipped is never set for traffic to workloads with configured conn limits (and document that invariant), and add a unit/FV test to prevent regressions.

ctx->state->ip_proto == IPPROTO_TCP &&
ct_result_is_syn(ctx->state->ct_result.rc)) {
bool reject = false;
if (ctx->state->ct_result.rc & CT_RES_CONNLIMIT_FIRST_SYN) {
/* First SYN: check and increment. */
reject = qos_connlimit_check_and_increment(ctx) < 0;
} else if (ctx->state->ct_result.flags & CALI_CT_FLAG_CONNLIMIT_REJECTED) {
/* SYN retransmission of a previously rejected connection. */
CALI_DEBUG("connlimit: retransmission of rejected connection");
reject = true;
}
if (reject) {
CALI_DEBUG("Ingress connection limit exceeded, rejecting with TCP RST");
/* Mark the CT entry so retransmissions are unconditionally
* rejected without re-checking the counter (which may have
* changed due to scanner recounts). */
{
bool sltd = src_lt_dest(&ctx->state->ip_src,
&ctx->state->ip_dst,
ctx->state->sport,
ctx->state->dport);
struct calico_ct_key ck;
fill_ct_key(&ck, sltd, ctx->state->ip_proto,
&ctx->state->ip_src, &ctx->state->ip_dst,
ctx->state->sport, ctx->state->dport);
struct calico_ct_value *cv = cali_ct_lookup_elem(&ck);
if (cv) {
ct_value_set_flags(cv, CALI_CT_FLAG_CONNLIMIT_REJECTED);
}
}
if (!skb_refresh_validate_ptrs(ctx, TCP_SIZE)) {
ctx->state->ct_result.ifindex_fwd = CT_INVALID_IFINDEX;
CALI_JUMP_TO(ctx, PROG_INDEX_TCP_RST);
}
goto deny;
}
}

if ((CALI_F_FROM_WEP || CALI_F_TO_HEP) && qos_dscp_needs_update(ctx) && !qos_dscp_set(ctx, EGRESS_DSCP)) {
goto deny;
}
Expand Down Expand Up @@ -1475,6 +1521,22 @@ int calico_tc_skb_new_flow_entrypoint(struct __sk_buff *skb)
CALI_DEBUG("Allowed by policy: ACCEPT");
}

/* Check egress connection limit for new TCP connections from WEP.
* Atomically check the limit and increment the counter in the QoS map.
* The Go-side CT scanner periodically recounts and corrects drift.
*/
if (CALI_F_FROM_WEP && state->ip_proto == IPPROTO_TCP &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nell Nell — Do we want to gate this on the global “egress connlimit configured” flag to avoid a QoS map lookup when the feature is off? Something like:

Suggested change
if (CALI_F_FROM_WEP && state->ip_proto == IPPROTO_TCP &&
if (CALI_F_FROM_WEP && state->ip_proto == IPPROTO_TCP &&
EGRESS_CONN_LIMIT_CONFIGURED &&
!(state->flags & CALI_ST_SUPPRESS_CT_STATE)) {

(We do that for marking the CT ctx a few lines below; feels consistent.)

!(state->flags & CALI_ST_SUPPRESS_CT_STATE)) {
if (qos_connlimit_check_and_increment(ctx) < 0) {
CALI_DEBUG("Egress connection limit exceeded, rejecting with TCP RST");
if (!skb_refresh_validate_ptrs(ctx, TCP_SIZE)) {
ctx->state->ct_result.ifindex_fwd = CT_INVALID_IFINDEX;
CALI_JUMP_TO(ctx, PROG_INDEX_TCP_RST);
}
goto deny;
}
}

if (CALI_F_FROM_WEP &&
CALI_DROP_WORKLOAD_TO_HOST &&
cali_rt_flags_local_host(
Expand Down Expand Up @@ -1512,6 +1574,9 @@ int calico_tc_skb_new_flow_entrypoint(struct __sk_buff *skb)
if (state->flags & CALI_ST_SKIP_REDIR_PEER) {
ct_ctx_nat->flags |= CALI_CT_FLAG_SKIP_REDIR_PEER;
}
if (CALI_F_FROM_WEP && state->ip_proto == IPPROTO_TCP && EGRESS_CONN_LIMIT_CONFIGURED) {
ct_ctx_nat->flags |= CALI_CT_FLAG_CONNLIMIT_EGRESS;
}
if (CALI_F_TO_WEP) {
if (!(ctx->skb->mark & CALI_SKB_MARK_SEEN)) {
/* If the packet wasn't seen, must come from host. There is no
Expand Down
Loading
Loading