Skip to content

Commit 71cea17

Browse files
edumazetdavem330
edumazet
authored and
davem330
committed
tcp: md5: remove spinlock usage in fast path
TCP md5 code uses per cpu variables but protects access to them with a shared spinlock, which is a contention point. [ tcp_md5sig_pool_lock is locked twice per incoming packet ] Makes things much simpler, by allocating crypto structures once, first time a socket needs md5 keys, and not deallocating them as they are really small. Next step would be to allow crypto allocations being done in a NUMA aware way. Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 168fc21 commit 71cea17

File tree

4 files changed

+29
-93
lines changed

4 files changed

+29
-93
lines changed

‎include/net/tcp.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,11 +1283,13 @@ static inline struct tcp_md5sig_key *tcp_md5_do_lookup(struct sock *sk,
12831283
#define tcp_twsk_md5_key(twsk) NULL
12841284
#endif
12851285

1286-
extern struct tcp_md5sig_pool __percpu *tcp_alloc_md5sig_pool(struct sock *);
1287-
extern void tcp_free_md5sig_pool(void);
1286+
extern bool tcp_alloc_md5sig_pool(void);
12881287

12891288
extern struct tcp_md5sig_pool *tcp_get_md5sig_pool(void);
1290-
extern void tcp_put_md5sig_pool(void);
1289+
static inline void tcp_put_md5sig_pool(void)
1290+
{
1291+
local_bh_enable();
1292+
}
12911293

12921294
extern int tcp_md5_hash_header(struct tcp_md5sig_pool *, const struct tcphdr *);
12931295
extern int tcp_md5_hash_skb_data(struct tcp_md5sig_pool *, const struct sk_buff *,

‎net/ipv4/tcp.c

Lines changed: 21 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -3095,9 +3095,8 @@ int tcp_gro_complete(struct sk_buff *skb)
30953095
EXPORT_SYMBOL(tcp_gro_complete);
30963096

30973097
#ifdef CONFIG_TCP_MD5SIG
3098-
static unsigned long tcp_md5sig_users;
3099-
static struct tcp_md5sig_pool __percpu *tcp_md5sig_pool;
3100-
static DEFINE_SPINLOCK(tcp_md5sig_pool_lock);
3098+
static struct tcp_md5sig_pool __percpu *tcp_md5sig_pool __read_mostly;
3099+
static DEFINE_MUTEX(tcp_md5sig_mutex);
31013100

31023101
static void __tcp_free_md5sig_pool(struct tcp_md5sig_pool __percpu *pool)
31033102
{
@@ -3112,30 +3111,14 @@ static void __tcp_free_md5sig_pool(struct tcp_md5sig_pool __percpu *pool)
31123111
free_percpu(pool);
31133112
}
31143113

3115-
void tcp_free_md5sig_pool(void)
3116-
{
3117-
struct tcp_md5sig_pool __percpu *pool = NULL;
3118-
3119-
spin_lock_bh(&tcp_md5sig_pool_lock);
3120-
if (--tcp_md5sig_users == 0) {
3121-
pool = tcp_md5sig_pool;
3122-
tcp_md5sig_pool = NULL;
3123-
}
3124-
spin_unlock_bh(&tcp_md5sig_pool_lock);
3125-
if (pool)
3126-
__tcp_free_md5sig_pool(pool);
3127-
}
3128-
EXPORT_SYMBOL(tcp_free_md5sig_pool);
3129-
3130-
static struct tcp_md5sig_pool __percpu *
3131-
__tcp_alloc_md5sig_pool(struct sock *sk)
3114+
static void __tcp_alloc_md5sig_pool(void)
31323115
{
31333116
int cpu;
31343117
struct tcp_md5sig_pool __percpu *pool;
31353118

31363119
pool = alloc_percpu(struct tcp_md5sig_pool);
31373120
if (!pool)
3138-
return NULL;
3121+
return;
31393122

31403123
for_each_possible_cpu(cpu) {
31413124
struct crypto_hash *hash;
@@ -3146,53 +3129,27 @@ __tcp_alloc_md5sig_pool(struct sock *sk)
31463129

31473130
per_cpu_ptr(pool, cpu)->md5_desc.tfm = hash;
31483131
}
3149-
return pool;
3132+
/* before setting tcp_md5sig_pool, we must commit all writes
3133+
* to memory. See ACCESS_ONCE() in tcp_get_md5sig_pool()
3134+
*/
3135+
smp_wmb();
3136+
tcp_md5sig_pool = pool;
3137+
return;
31503138
out_free:
31513139
__tcp_free_md5sig_pool(pool);
3152-
return NULL;
31533140
}
31543141

3155-
struct tcp_md5sig_pool __percpu *tcp_alloc_md5sig_pool(struct sock *sk)
3142+
bool tcp_alloc_md5sig_pool(void)
31563143
{
3157-
struct tcp_md5sig_pool __percpu *pool;
3158-
bool alloc = false;
3159-
3160-
retry:
3161-
spin_lock_bh(&tcp_md5sig_pool_lock);
3162-
pool = tcp_md5sig_pool;
3163-
if (tcp_md5sig_users++ == 0) {
3164-
alloc = true;
3165-
spin_unlock_bh(&tcp_md5sig_pool_lock);
3166-
} else if (!pool) {
3167-
tcp_md5sig_users--;
3168-
spin_unlock_bh(&tcp_md5sig_pool_lock);
3169-
cpu_relax();
3170-
goto retry;
3171-
} else
3172-
spin_unlock_bh(&tcp_md5sig_pool_lock);
3173-
3174-
if (alloc) {
3175-
/* we cannot hold spinlock here because this may sleep. */
3176-
struct tcp_md5sig_pool __percpu *p;
3177-
3178-
p = __tcp_alloc_md5sig_pool(sk);
3179-
spin_lock_bh(&tcp_md5sig_pool_lock);
3180-
if (!p) {
3181-
tcp_md5sig_users--;
3182-
spin_unlock_bh(&tcp_md5sig_pool_lock);
3183-
return NULL;
3184-
}
3185-
pool = tcp_md5sig_pool;
3186-
if (pool) {
3187-
/* oops, it has already been assigned. */
3188-
spin_unlock_bh(&tcp_md5sig_pool_lock);
3189-
__tcp_free_md5sig_pool(p);
3190-
} else {
3191-
tcp_md5sig_pool = pool = p;
3192-
spin_unlock_bh(&tcp_md5sig_pool_lock);
3193-
}
3144+
if (unlikely(!tcp_md5sig_pool)) {
3145+
mutex_lock(&tcp_md5sig_mutex);
3146+
3147+
if (!tcp_md5sig_pool)
3148+
__tcp_alloc_md5sig_pool();
3149+
3150+
mutex_unlock(&tcp_md5sig_mutex);
31943151
}
3195-
return pool;
3152+
return tcp_md5sig_pool != NULL;
31963153
}
31973154
EXPORT_SYMBOL(tcp_alloc_md5sig_pool);
31983155

@@ -3209,28 +3166,15 @@ struct tcp_md5sig_pool *tcp_get_md5sig_pool(void)
32093166
struct tcp_md5sig_pool __percpu *p;
32103167

32113168
local_bh_disable();
3212-
3213-
spin_lock(&tcp_md5sig_pool_lock);
3214-
p = tcp_md5sig_pool;
3215-
if (p)
3216-
tcp_md5sig_users++;
3217-
spin_unlock(&tcp_md5sig_pool_lock);
3218-
3169+
p = ACCESS_ONCE(tcp_md5sig_pool);
32193170
if (p)
3220-
return this_cpu_ptr(p);
3171+
return __this_cpu_ptr(p);
32213172

32223173
local_bh_enable();
32233174
return NULL;
32243175
}
32253176
EXPORT_SYMBOL(tcp_get_md5sig_pool);
32263177

3227-
void tcp_put_md5sig_pool(void)
3228-
{
3229-
local_bh_enable();
3230-
tcp_free_md5sig_pool();
3231-
}
3232-
EXPORT_SYMBOL(tcp_put_md5sig_pool);
3233-
32343178
int tcp_md5_hash_header(struct tcp_md5sig_pool *hp,
32353179
const struct tcphdr *th)
32363180
{

‎net/ipv4/tcp_ipv4.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,7 @@ int tcp_md5_do_add(struct sock *sk, const union tcp_md5_addr *addr,
10261026
key = sock_kmalloc(sk, sizeof(*key), gfp);
10271027
if (!key)
10281028
return -ENOMEM;
1029-
if (hlist_empty(&md5sig->head) && !tcp_alloc_md5sig_pool(sk)) {
1029+
if (!tcp_alloc_md5sig_pool()) {
10301030
sock_kfree_s(sk, key, sizeof(*key));
10311031
return -ENOMEM;
10321032
}
@@ -1044,20 +1044,14 @@ EXPORT_SYMBOL(tcp_md5_do_add);
10441044

10451045
int tcp_md5_do_del(struct sock *sk, const union tcp_md5_addr *addr, int family)
10461046
{
1047-
struct tcp_sock *tp = tcp_sk(sk);
10481047
struct tcp_md5sig_key *key;
1049-
struct tcp_md5sig_info *md5sig;
10501048

10511049
key = tcp_md5_do_lookup(sk, (union tcp_md5_addr *)&addr, AF_INET);
10521050
if (!key)
10531051
return -ENOENT;
10541052
hlist_del_rcu(&key->node);
10551053
atomic_sub(sizeof(*key), &sk->sk_omem_alloc);
10561054
kfree_rcu(key, rcu);
1057-
md5sig = rcu_dereference_protected(tp->md5sig_info,
1058-
sock_owned_by_user(sk));
1059-
if (hlist_empty(&md5sig->head))
1060-
tcp_free_md5sig_pool();
10611055
return 0;
10621056
}
10631057
EXPORT_SYMBOL(tcp_md5_do_del);
@@ -1071,8 +1065,6 @@ static void tcp_clear_md5_list(struct sock *sk)
10711065

10721066
md5sig = rcu_dereference_protected(tp->md5sig_info, 1);
10731067

1074-
if (!hlist_empty(&md5sig->head))
1075-
tcp_free_md5sig_pool();
10761068
hlist_for_each_entry_safe(key, n, &md5sig->head, node) {
10771069
hlist_del_rcu(&key->node);
10781070
atomic_sub(sizeof(*key), &sk->sk_omem_alloc);

‎net/ipv4/tcp_minisocks.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
317317
key = tp->af_specific->md5_lookup(sk, sk);
318318
if (key != NULL) {
319319
tcptw->tw_md5_key = kmemdup(key, sizeof(*key), GFP_ATOMIC);
320-
if (tcptw->tw_md5_key && tcp_alloc_md5sig_pool(sk) == NULL)
320+
if (tcptw->tw_md5_key && !tcp_alloc_md5sig_pool())
321321
BUG();
322322
}
323323
} while (0);
@@ -358,10 +358,8 @@ void tcp_twsk_destructor(struct sock *sk)
358358
#ifdef CONFIG_TCP_MD5SIG
359359
struct tcp_timewait_sock *twsk = tcp_twsk(sk);
360360

361-
if (twsk->tw_md5_key) {
362-
tcp_free_md5sig_pool();
361+
if (twsk->tw_md5_key)
363362
kfree_rcu(twsk->tw_md5_key, rcu);
364-
}
365363
#endif
366364
}
367365
EXPORT_SYMBOL_GPL(tcp_twsk_destructor);

0 commit comments

Comments
 (0)