Thread (41 messages) flat view 41 messages, 4 authors, 2018-10-25

Re: [RFC PATCH v2 01/10] udp: implement complete book-keeping for encap_needed

From: Paolo Abeni <pabeni@redhat.com>
Date: 2018-10-25 21:32:50

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:
quoted hunk ↗ jump to hunk
@@ -2431,7 +2435,9 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
quoted
                        /* 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help