Re: [PATCH mptcp-next 1/2] mptcp: optimize out option generation

[Paolo Abeni <pabeni@redhat.com>]

On Mon, 2021-07-26 at 11:21 +0800, Geliang Tang wrote:
> Hi Paolo,
> 
> Thanks for this patchset, and I have some small comments.
> 
> Paolo Abeni <pabeni@redhat.com> 于2021年7月24日周六 上午1:06写道：
> > Currently we have several protocol contraint on MPTCP option
> > generation (e.g. MPC and MPJ subopt are mutually exclusive)
> > and some additionall ones required by our implementation
> > (e.g. almost all ADD_ADDR variant are mutually exclusive with
> > everything else).
> > 
> > We can leverage the above to optimize the out option generation:
> > we check DSS/MPC/MPJ presencence in a mutually exclusive way,
> > avoiding many unneeded conditionals in the common case.
> > 
> > Additionally extend the existing constraint on ADD_ADDR opt on
> > all subvariant, so that it become fully mutually exclusive with
> > the above and we can skip another conditional statement the common
> > case.
> > 
> > This change is also need by the next patch.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > --
> > side node: we could probably optimize the conditionals in
> > mptcp_established_options()
> > ---
> >  net/mptcp/options.c  | 225 ++++++++++++++++++++++---------------------
> >  net/mptcp/pm.c       |   9 +-
> >  net/mptcp/protocol.h |   1 +
> >  3 files changed, 122 insertions(+), 113 deletions(-)
> > 
> > diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> > index eafdb9408f3a..38f324667761 100644
> > --- a/net/mptcp/options.c
> > +++ b/net/mptcp/options.c
> > @@ -592,6 +592,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> >                 dss_size = map_size;
> >                 if (skb && snd_data_fin_enable)
> >                         mptcp_write_data_fin(subflow, skb, &opts->ext_copy);
> > +               opts->suboptions = OPTION_MPTCP_DSS;
> >                 ret = true;
> >         }
> > 
> > @@ -615,6 +616,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> >                 opts->ext_copy.ack64 = 0;
> >         }
> >         opts->ext_copy.use_ack = 1;
> > +       opts->suboptions = OPTION_MPTCP_DSS;
> >         WRITE_ONCE(msk->old_wspace, __mptcp_space((struct sock *)msk));
> > 
> >         /* Add kind/length/subtype/flag overhead if mapping is not populated */
> > @@ -663,9 +665,9 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> 
> I think no need to modify the code in mptcp_established_options_add_addr.

Uhmmm... I just noted with need an additional change in there: avoid
clearing 'ext_copy'.

/P