Hi, I'm sorry for lagging behind, this one felt outside my radar.
On Mon, 2018-10-22 at 12:06 -0400, Willem de Bruijn wrote: > @@ -2431,7 +2435,9 @@ int udp_lib_setsockopt(struct sock *sk, int level, int > optname, > > /* FALLTHROUGH */ > > case UDP_ENCAP_L2TPINUDP: > > up->encap_type = val; > > - udp_encap_enable(); > > + if (!up->encap_enabled) > > + udp_encap_enable(); > > + up->encap_enabled = 1; > > nit: no need for the branch: udp_encap_enable already has a branch and > is static inline. Uhm... I think it's needed, so that we call udp_encap_enable() at most once per socket. If up->encap_enabled we also call static_key_disable at socket destruction time (once per socket, again) and hopefully we don't "leak" static_key_enable invocation. > Perhaps it makes sense to convert that to take the udp_sock and handle > the state change within, to avoid having to open code at multiple > locations. Possibly calling directly udp_tunnel_encap_enable()? that additionally cope with ipv6, which is not needed here, but should not hurt. Cheers, Paolo
