Re: [MPTCP] [PATCH v2 14/25] tcp_extra_options: Check static branch before _parse

On Tue, 28 Nov 2017, Mat Martineau wrote: > > On Mon, 27 Nov 2017, Christoph Paasch wrote: > > > Signed-off-by: Christoph Paasch <cpaasch(a)apple.com&gt; > > --- > > net/ipv4/tcp_input.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > > index 4e84f299c96f..7acc1f46641c 100644 > > --- a/net/ipv4/tcp_input.c > > +++ b/net/ipv4/tcp_input.c > > @@ -3799,9 +3799,11 @@ void tcp_parse_options(const struct net *net, > > break; > > > > default: > > - tcp_extra_options_parse(opcode, opsize, ptr, > > - skb, opt_rx, > > - tcp_to_sk(tp)); > > + if (static_branch_unlikely(&tcp_extra_options_enabled)) > > Since this case in the switch statement is only executed for > unrecognized options, I had skipped the static branch in the initial > implementation. It is more consistent to use the static branch, and > there isn't much use in making the function call just to try to iterate > over an empty list. > > > + tcp_extra_options_parse(opcode, opsize, > > + ptr, skb, > > + opt_rx, > > + tcp_to_sk(tp)); > > break; > > > > } > > -- > > 2.15.0 > > > > I sent my reply and then realized I had one more point to make. We might need to take another look at the use of static branching by the extra options framework. It's one thing to inc/dec tcp_extra_options_enabled when modules are added or removed, but we don't want to be rapidly changing static branches as MPTCP sockets come and go. Does tcp_extra_options_enabled still make sense for per-socket extra_option lists?