On Tue, Apr 24, 2018 at 7:41 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > > > 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. >
Could you pls. explain the issue to me ? > I am not that happy. > > Please revert this patch. > OK. I will revert it. > Thank you.