From: Alexander Bluhm Subject: TCP sync cache sysctl MP safe To: tech@openbsd.org Date: Sun, 29 Dec 2024 01:59:50 +0100 Hi, The sysctl values for the TCP syn cache are either atomic or can be protected by sync cache mutex. This allows to remove net lock for two TCP sysctl. ok? bluhm Index: netinet/tcp_input.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v diff -u -p -r1.414 tcp_input.c --- netinet/tcp_input.c 28 Dec 2024 22:17:09 -0000 1.414 +++ netinet/tcp_input.c 28 Dec 2024 22:49:22 -0000 @@ -3108,16 +3108,16 @@ tcp_mss_adv(struct mbuf *m, int af) */ /* syn hash parameters */ -int tcp_syn_hash_size = TCP_SYN_HASH_SIZE; /* [N] size of hash table */ -int tcp_syn_cache_limit = /* [N] global entry limit */ +int tcp_syn_hash_size = TCP_SYN_HASH_SIZE; /* [S] size of hash table */ +int tcp_syn_cache_limit = /* [a] global entry limit */ TCP_SYN_HASH_SIZE * TCP_SYN_BUCKET_SIZE; -int tcp_syn_bucket_limit = /* [N] per bucket limit */ +int tcp_syn_bucket_limit = /* [a] per bucket limit */ 3 * TCP_SYN_BUCKET_SIZE; -int tcp_syn_use_limit = 100000; /* [N] reseed after uses */ +int tcp_syn_use_limit = 100000; /* [S] reseed after uses */ struct pool syn_cache_pool; -struct syn_cache_set tcp_syn_cache[2]; -int tcp_syn_cache_active; +struct syn_cache_set tcp_syn_cache[2]; /* [S] */ +int tcp_syn_cache_active; /* [S] */ struct mutex syn_cache_mtx = MUTEX_INITIALIZER(IPL_SOFTNET); #define SYN_HASH(sa, sp, dp, rand) \ @@ -3212,7 +3212,7 @@ syn_cache_init(void) void syn_cache_insert(struct syn_cache *sc, struct tcpcb *tp) { - struct syn_cache_set *set = &tcp_syn_cache[tcp_syn_cache_active]; + struct syn_cache_set *set; struct syn_cache_head *scp; struct syn_cache *sc2; int i; @@ -3220,6 +3220,8 @@ syn_cache_insert(struct syn_cache *sc, s NET_ASSERT_LOCKED(); MUTEX_ASSERT_LOCKED(&syn_cache_mtx); + set = &tcp_syn_cache[tcp_syn_cache_active]; + /* * If there are no entries in the hash table, reinitialize * the hash secrets. To avoid useless cache swaps and @@ -3257,7 +3259,7 @@ syn_cache_insert(struct syn_cache *sc, s * Make sure that we don't overflow the per-bucket * limit or the total cache size limit. */ - if (scp->sch_length >= tcp_syn_bucket_limit) { + if (scp->sch_length >= atomic_load_int(&tcp_syn_bucket_limit)) { tcpstat_inc(tcps_sc_bucketoverflow); /* * Someone might attack our bucket hash function. Reseed @@ -3279,7 +3281,7 @@ syn_cache_insert(struct syn_cache *sc, s #endif syn_cache_rm(sc2); syn_cache_put(sc2); - } else if (set->scs_count >= tcp_syn_cache_limit) { + } else if (set->scs_count >= atomic_load_int(&tcp_syn_cache_limit)) { struct syn_cache_head *scp2, *sce; tcpstat_inc(tcps_sc_overflowed); Index: netinet/tcp_usrreq.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_usrreq.c,v diff -u -p -r1.233 tcp_usrreq.c --- netinet/tcp_usrreq.c 19 Dec 2024 22:11:35 -0000 1.233 +++ netinet/tcp_usrreq.c 29 Dec 2024 00:51:51 -0000 @@ -1341,7 +1341,7 @@ tcp_sysctl_tcpstat(void *oldp, size_t *o set = &tcp_syn_cache[tcp_syn_cache_active]; tcpstat.tcps_sc_hash_size = set->scs_size; tcpstat.tcps_sc_entry_count = set->scs_count; - tcpstat.tcps_sc_entry_limit = tcp_syn_cache_limit; + tcpstat.tcps_sc_entry_limit = atomic_load_int(&tcp_syn_cache_limit); tcpstat.tcps_sc_bucket_maxlen = 0; for (i = 0; i < set->scs_size; i++) { if (tcpstat.tcps_sc_bucket_maxlen < @@ -1349,7 +1349,7 @@ tcp_sysctl_tcpstat(void *oldp, size_t *o tcpstat.tcps_sc_bucket_maxlen = set->scs_buckethead[i].sch_length; } - tcpstat.tcps_sc_bucket_limit = tcp_syn_bucket_limit; + tcpstat.tcps_sc_bucket_limit = atomic_load_int(&tcp_syn_bucket_limit); tcpstat.tcps_sc_uses_left = set->scs_use; mtx_leave(&syn_cache_mtx); @@ -1364,7 +1364,7 @@ int tcp_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, size_t newlen) { - int error, nval; + int error, oval, nval; /* All sysctl names at this level are terminal. */ if (namelen != 1) @@ -1457,30 +1457,29 @@ tcp_sysctl(int *name, u_int namelen, voi return (tcp_sysctl_tcpstat(oldp, oldlenp, newp)); case TCPCTL_SYN_USE_LIMIT: - NET_LOCK(); + oval = nval = atomic_load_int(&tcp_syn_use_limit); error = sysctl_int_bounded(oldp, oldlenp, newp, newlen, - &tcp_syn_use_limit, 0, INT_MAX); - if (!error && newp != NULL) { + &nval, 0, INT_MAX); + if (!error && oval != nval) { /* * Global tcp_syn_use_limit is used when reseeding a * new cache. Also update the value in active cache. */ mtx_enter(&syn_cache_mtx); - if (tcp_syn_cache[0].scs_use > tcp_syn_use_limit) - tcp_syn_cache[0].scs_use = tcp_syn_use_limit; - if (tcp_syn_cache[1].scs_use > tcp_syn_use_limit) - tcp_syn_cache[1].scs_use = tcp_syn_use_limit; + if (tcp_syn_cache[0].scs_use > nval) + tcp_syn_cache[0].scs_use = nval; + if (tcp_syn_cache[1].scs_use > nval) + tcp_syn_cache[1].scs_use = nval; + tcp_syn_use_limit = nval; mtx_leave(&syn_cache_mtx); } - NET_UNLOCK(); return (error); case TCPCTL_SYN_HASH_SIZE: - NET_LOCK(); - nval = tcp_syn_hash_size; + oval = nval = atomic_load_int(&tcp_syn_hash_size); error = sysctl_int_bounded(oldp, oldlenp, newp, newlen, &nval, 1, 100000); - if (!error && nval != tcp_syn_hash_size) { + if (!error && oval != nval) { /* * If global hash size has been changed, * switch sets as soon as possible. Then @@ -1494,7 +1493,6 @@ tcp_sysctl(int *name, u_int namelen, voi tcp_syn_hash_size = nval; mtx_leave(&syn_cache_mtx); } - NET_UNLOCK(); return (error); default: