Re: [bpf PATCH v4 3/4] bpf: sockhash fix omitted bucket lock in sock_close

2018-06-29 Thread John Fastabend
On 06/29/2018 06:45 AM, Daniel Borkmann wrote: > On 06/25/2018 05:34 PM, John Fastabend wrote: > [...] >> @@ -2302,9 +2347,12 @@ static int sock_hash_ctx_update_elem(struct >> bpf_sock_ops_kern *skops, >> goto bucket_err; >> } >> >> -e->hash_link = l_new; >> -e->htab =

Re: [bpf PATCH v4 3/4] bpf: sockhash fix omitted bucket lock in sock_close

2018-06-29 Thread Daniel Borkmann
On 06/25/2018 05:34 PM, John Fastabend wrote: [...] > @@ -2302,9 +2347,12 @@ static int sock_hash_ctx_update_elem(struct > bpf_sock_ops_kern *skops, > goto bucket_err; > } > > - e->hash_link = l_new; > - e->htab = container_of(map, struct bpf_htab, map); > +

Re: [bpf PATCH v4 3/4] bpf: sockhash fix omitted bucket lock in sock_close

2018-06-25 Thread Martin KaFai Lau
On Mon, Jun 25, 2018 at 08:34:17AM -0700, John Fastabend wrote: > First in tcp_close, reduce scope of sk_callback_lock() the lock is > only needed for protecting maps list the ingress and cork > lists are protected by sock lock. Having the lock in wider scope is > harmless but may confuse the

[bpf PATCH v4 3/4] bpf: sockhash fix omitted bucket lock in sock_close

2018-06-25 Thread John Fastabend
First in tcp_close, reduce scope of sk_callback_lock() the lock is only needed for protecting maps list the ingress and cork lists are protected by sock lock. Having the lock in wider scope is harmless but may confuse the reader who may infer it is in fact needed. Next, in sock_hash_delete_elem()