On 04/23/2018 09:39 PM, Yafang Shao wrote: > On Tue, Apr 24, 2018 at 12:09 AM, Eric Dumazet <eric.duma...@gmail.com> wrote: >> >> >> On 04/23/2018 08:58 AM, David Miller wrote: >>> From: Yafang Shao <laoar.s...@gmail.com> >>> Date: Sun, 22 Apr 2018 21:50:04 +0800 >>> >>>> With sk_cookie we can identify a socket, that is very helpful for >>>> traceing and statistic, i.e. tcp tracepiont and ebpf. >>>> So we'd better init it by default for inet socket. >>>> When using it, we just need call atomic64_read(&sk->sk_cookie). >>>> >>>> Signed-off-by: Yafang Shao <laoar.s...@gmail.com> >>> >>> Applied, thank you. >>> >> >> This is adding yet another atomic_inc on a global cache line. >> > > That's a trade-off. > >> Most applications do not need the cookie being ever set. >> >> The existing mechanism was fine. Set it on demand. > > There are some drawback in the existing mechanism. > - we have to set the net->cookie_gen and then sk->sk_cookie when we > want to get the sk_cookie, that's a little expensive as well.
Same cost. > After that change, sock_gen_cookie() could be replaced by > atomic64_read(&sk->sk_cookie) in most places. Same cost than the helper. > > - If the application want to get the sk_cookie, it must set it first. > What if the application don't have the permision to write? > Furthermore, maybe it is a security concern ? Maybe ? Please elaborate. Your patch destroys SYNFLOOD behavior. I have spent months of work solving the SYNFLOOD behavior, your patch crushes it. I am not that happy. Please revert this patch. Thank you.
